( )⚙ D4370 treemanifest: use visitchildrenset when filtering a manifest to a matcher

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4370

treemanifest: use visitchildrenset when filtering a manifest to a matcher
ClosedPublic

Authored by spectral on Aug 24 2018, 1:36 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG154e4f84b51c: treemanifest: use visitchildrenset when filtering a manifest to a matcher
Summary

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

spectral created this revision.Aug 24 2018, 1:36 AM
spectral updated this revision to Diff 10836.Sep 7 2018, 4:07 PM
spectral updated this revision to Diff 10838.Sep 7 2018, 9:22 PM
yuja added a subscriber: yuja.Sep 11 2018, 8:26 AM

Queued, thanks.

+ def _loadchildrensetlazy(self, visit):
+ if not visit:
+ return None
+ if visit == 'all' or visit == 'this':
+ self._loadalllazy()
+ return None
+
+ todel = []
+ for k in visit:
+ kslash = k + '/'
+ ld = self._lazydirs.get(kslash)
+ if ld:
+ path, node, readsubtree = ld
+ self._dirs[kslash] = readsubtree(path, node)
+ todel.append(kslash)
+ for kslash in todel:
+ del self._lazydirs[kslash]

Any reason not to use self._loadlazy(kslash)?

Closed by commit rHG154e4f84b51c: treemanifest: use visitchildrenset when filtering a manifest to a matcher (authored by spectral). · Explain WhySep 11 2018, 8:36 AM
This revision was automatically updated to reflect the committed changes.
spectral added a comment.Sep 11 2018, 3:34 PM
In D4370#69050, @yuja wrote:

Queued, thanks.

+ def _loadchildrensetlazy(self, visit):
+ if not visit:
+ return None
+ if visit == 'all' or visit == 'this':
+ self._loadalllazy()
+ return None
+
+ todel = []
+ for k in visit:
+ kslash = k + '/'
+ ld = self._lazydirs.get(kslash)
+ if ld:
+ path, node, readsubtree = ld
+ self._dirs[kslash] = readsubtree(path, node)
+ todel.append(kslash)
+ for kslash in todel:
+ del self._lazydirs[kslash]

Any reason not to use self._loadlazy(kslash)?

Hmm, I feel like there was a previous version that iterated differently and meant I'd need a copy of the list to do this correctly.. in this version, no I don't think there's a reason to avoid it besides a potential minor speed difference; should I send a patch for that?

yuja added a comment.Sep 11 2018, 6:57 PM
> Any reason not to use `self._loadlazy(kslash)`?
Hmm, I feel like there was a previous version that iterated differently and meant I'd need a copy of the list to do this correctly.. in this version, no I don't think there's a reason to avoid it besides a potential minor speed difference; should I send a patch for that?

I don't have any preference, but please send a patch if that makes sense,
thanks.

Revision Contents
Changeset List

PathPackages
Mmercurial/manifest.py (34 lines)

Diff 10836

mercurial/manifest.py

self._dirs[k] = readsubtree(path, node) self._dirs[k] = readsubtree(path, node)
self._lazydirs = {} self._lazydirs = {}
def _loadlazy(self, d): def _loadlazy(self, d):
path, node, readsubtree = self._lazydirs[d] path, node, readsubtree = self._lazydirs[d]
self._dirs[d] = readsubtree(path, node) self._dirs[d] = readsubtree(path, node)
del self._lazydirs[d] del self._lazydirs[d]
def _loadchildrensetlazy(self, visit):
if not visit:
return None
if visit == 'all' or visit == 'this':
self._loadalllazy()
return None
todel = []
for k in visit:
kslash = k + '/'
ld = self._lazydirs.get(kslash)
if ld:
path, node, readsubtree = ld
self._dirs[kslash] = readsubtree(path, node)
todel.append(kslash)
for kslash in todel:
del self._lazydirs[kslash]
return visit
def __len__(self): def __len__(self):
self._load() self._load()
size = len(self._files) size = len(self._files)
self._loadalllazy() self._loadalllazy()
for m in self._dirs.values(): for m in self._dirs.values():
size += m.__len__() size += m.__len__()
return size return size
return self.copy() return self.copy()
return self._matches(match) return self._matches(match)
def _matches(self, match): def _matches(self, match):
'''recursively generate a new manifest filtered by the match argument. '''recursively generate a new manifest filtered by the match argument.
''' '''
visit = match.visitdir(self._dir[:-1] or '.') visit = match.visitchildrenset(self._dir[:-1] or '.')
if visit == 'all': if visit == 'all':
return self.copy() return self.copy()
ret = treemanifest(self._dir) ret = treemanifest(self._dir)
if not visit: if not visit:
return ret return ret
self._load() self._load()
for fn in self._files: for fn in self._files:
# While visitchildrenset *usually* lists only subdirs, this is
# actually up to the matcher and may have some files in the set().
# If visit == 'this', we should obviously look at the files in this
# directory; if visit is a set, and fn is in it, we should inspect
# fn (but no need to inspect things not in the set).
if visit != 'this' and fn not in visit:
continue
fullp = self._subpath(fn) fullp = self._subpath(fn)
# visitchildrenset isn't perfect, we still need to call the regular
# matcher code to further filter results.
if not match(fullp): if not match(fullp):
continue continue
ret._files[fn] = self._files[fn] ret._files[fn] = self._files[fn]
if fn in self._flags: if fn in self._flags:
ret._flags[fn] = self._flags[fn] ret._flags[fn] = self._flags[fn]
# OPT: use visitchildrenset to avoid loading everything visit = self._loadchildrensetlazy(visit)
self._loadalllazy() self._loadalllazy()
for dir, subm in self._dirs.iteritems(): for dir, subm in self._dirs.iteritems():
if visit and dir[:-1] not in visit:
continue
m = subm._matches(match) m = subm._matches(match)
if not m._isempty(): if not m._isempty():
ret._dirs[dir] = m ret._dirs[dir] = m
if not ret._isempty(): if not ret._isempty():
ret._dirty = True ret._dirty = True
return ret return ret