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

This is an archive of the discontinued Mercurial Phabricator instance.

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

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

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)?

This revision was automatically updated to reflect the committed changes.
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.