This is an archive of the discontinued Mercurial Phabricator instance.

Differential D663

dirstate: perform transactions with _map using single call, where possible
ClosedPublic

Authored by mbolin on Sep 9 2017, 12:30 AM.

Details

Reviewers
phillco
yuja
Group Reviewers
hg-reviewers
Commits
rHG1246acdad653: dirstate: perform transactions with _map using single call, where possible
rHG6d734a3a76e2: dirstate: perform transactions with _map using single call, where possible
Summary

This is in the same style as https://phab.mercurial-scm.org/D493.

In general, this replaces patterns such as:

f in self._map:
    entry = self._map[f]

with:

entry = self._map.get(f):
if entry is not None:
    # use entry
Test Plan

make tests

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mbolin created this revision.Sep 9 2017, 12:30 AM
phillco accepted this revision.Sep 10 2017, 9:02 PM
phillco added a subscriber: phillco.

These look good to me.

mercurial/dirstate.py
585–588

Possibly move this comment to just above line 590?

yuja requested changes to this revision.Sep 13 2017, 11:10 AM
yuja added a subscriber: yuja.
yuja added inline comments.
mercurial/dirstate.py
554

test-add.t fails here. It was self[d], not self._map[d].

This revision now requires changes to proceed.Sep 13 2017, 11:10 AM
mbolin updated this revision to Diff 1817.Sep 14 2017, 12:41 PM
mbolin added inline comments.Sep 14 2017, 12:42 PM
mercurial/dirstate.py
554

My bad: fixed without bringing the extra lookup back.

yuja accepted this revision.Sep 15 2017, 9:44 AM

Queued, thanks.

This revision is now accepted and ready to land.Sep 15 2017, 9:44 AM
Closed by commit rHG6d734a3a76e2: dirstate: perform transactions with _map using single call, where possible (authored by mbolin). · Explain WhySep 15 2017, 9:54 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/dirstate.py (47 lines)

Diff 1842

mercurial/dirstate.py

if state == 'a' or oldstate == 'r': if state == 'a' or oldstate == 'r':
scmutil.checkfilename(f) scmutil.checkfilename(f)
if f in self._dirs: if f in self._dirs:
raise error.Abort(_('directory %r already in dirstate') % f) raise error.Abort(_('directory %r already in dirstate') % f)
# shadows # shadows
for d in util.finddirs(f): for d in util.finddirs(f):
if d in self._dirs: if d in self._dirs:
break break
if d in self._map and self[d] != 'r': entry = self._map.get(d)
if entry is not None and entry[0] != 'r':
yujaUnsubmitted
Not Done

test-add.t fails here. It was self[d], not self._map[d].

yuja: test-add.t fails here. It was `self[d]`, not `self._map[d]`.
mbolinAuthorUnsubmitted
Not Done

My bad: fixed without bringing the extra lookup back.

mbolin: My bad: fixed without bringing the extra lookup back.
raise error.Abort( raise error.Abort(
_('file %r in dirstate clashes with %r') % (d, f)) _('file %r in dirstate clashes with %r') % (d, f))
if oldstate in "?r" and "_dirs" in self.__dict__: if oldstate in "?r" and "_dirs" in self.__dict__:
self._dirs.addpath(f) self._dirs.addpath(f)
self._dirty = True self._dirty = True
self._updatedfiles.add(f) self._updatedfiles.add(f)
self._map[f] = dirstatetuple(state, mode, size, mtime) self._map[f] = dirstatetuple(state, mode, size, mtime)
if state != 'n' or mtime == -1: if state != 'n' or mtime == -1:
if mtime > self._lastnormaltime: if mtime > self._lastnormaltime:
# Remember the most recent modification timeslot for status(), # Remember the most recent modification timeslot for status(),
# to make sure we won't miss future size-preserving file content # to make sure we won't miss future size-preserving file content
# modifications that happen within the same timeslot. # modifications that happen within the same timeslot.
self._lastnormaltime = mtime self._lastnormaltime = mtime
def normallookup(self, f): def normallookup(self, f):
'''Mark a file normal, but possibly dirty.''' '''Mark a file normal, but possibly dirty.'''
if self._pl[1] != nullid and f in self._map: if self._pl[1] != nullid:
# if there is a merge going on and the file was either # if there is a merge going on and the file was either
# in state 'm' (-1) or coming from other parent (-2) before # in state 'm' (-1) or coming from other parent (-2) before
# being removed, restore that state. # being removed, restore that state.
entry = self._map[f] entry = self._map.get(f)
phillcoUnsubmitted
Not Done

Possibly move this comment to just above line 590?

phillco: Possibly move this comment to just above line 590?
if entry is not None:
if entry[0] == 'r' and entry[2] in (-1, -2): if entry[0] == 'r' and entry[2] in (-1, -2):
source = self._copymap.get(f) source = self._copymap.get(f)
if entry[2] == -1: if entry[2] == -1:
self.merge(f) self.merge(f)
elif entry[2] == -2: elif entry[2] == -2:
self.otherparent(f) self.otherparent(f)
if source: if source:
self.copy(source, f) self.copy(source, f)
return return
if entry[0] == 'm' or entry[0] == 'n' and entry[2] == -2: if entry[0] == 'm' or entry[0] == 'n' and entry[2] == -2:
return return
self._addpath(f, 'n', 0, -1, -1) self._addpath(f, 'n', 0, -1, -1)
self._copymap.pop(f, None) self._copymap.pop(f, None)
if f in self._nonnormalset: if f in self._nonnormalset:
self._nonnormalset.remove(f) self._nonnormalset.remove(f)
def otherparent(self, f): def otherparent(self, f):
'''Mark as coming from the other parent, always dirty.''' '''Mark as coming from the other parent, always dirty.'''
if self._pl[1] == nullid: if self._pl[1] == nullid:
self._addpath(f, 'a', 0, -1, -1) self._addpath(f, 'a', 0, -1, -1)
self._copymap.pop(f, None) self._copymap.pop(f, None)
def remove(self, f): def remove(self, f):
'''Mark a file removed.''' '''Mark a file removed.'''
self._dirty = True self._dirty = True
self._droppath(f) self._droppath(f)
size = 0 size = 0
if self._pl[1] != nullid and f in self._map: if self._pl[1] != nullid:
entry = self._map.get(f)
if entry is not None:
# backup the previous state # backup the previous state
entry = self._map[f]
if entry[0] == 'm': # merge if entry[0] == 'm': # merge
size = -1 size = -1
elif entry[0] == 'n' and entry[2] == -2: # other parent elif entry[0] == 'n' and entry[2] == -2: # other parent
size = -2 size = -2
self._otherparentset.add(f) self._otherparentset.add(f)
self._map[f] = dirstatetuple('r', 0, size, 0) self._map[f] = dirstatetuple('r', 0, size, 0)
self._nonnormalset.add(f) self._nonnormalset.add(f)
if size == 0: if size == 0:
self._copymap.pop(f, None) self._copymap.pop(f, None)
def merge(self, f): def merge(self, f):
'''Mark a file merged.''' '''Mark a file merged.'''
if self._pl[1] == nullid: if self._pl[1] == nullid: