This is an archive of the discontinued Mercurial Phabricator instance.

Differential D771

remotefilelog: add seen set to annotate algorithm
ClosedPublic

Authored by durham on Sep 21 2017, 6:23 PM.
Tags
None
Subscribers
quark

Details

Reviewers
quark
Group Reviewers
Restricted Project
Commits
rFBHGXad5622aae1ba: remotefilelog: add seen set to annotate algorithm
Summary

The remotefilelog annotate logic was not keeping track of which commits it had
seen when doing the bfs ancestor walk. For very mergy histories this resulted in
a ton of duplicate work.

Diff Detail

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

Event Timeline

durham created this revision.Sep 21 2017, 6:23 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 21 2017, 6:23 PM
quark added a subscriber: quark.Sep 21 2017, 6:26 PM
quark added inline comments.
remotefilelog/remotefilectx.py
384

Could we check and set seen here? It looks simpler.

durham added inline comments.Sep 21 2017, 6:36 PM
remotefilelog/remotefilectx.py
384

If we check it here we can end up queueing things we don't need to queue. In a theoretical bfs that could end up queueing a lot of extra things. That's unlikely in a commit graph, but I figured might as well be efficient.

quark accepted this revision.Sep 21 2017, 6:56 PM
This revision is now accepted and ready to land.Sep 21 2017, 6:56 PM
Closed by commit rFBHGXad5622aae1ba: remotefilelog: add seen set to annotate algorithm (authored by durham). · Explain WhySep 22 2017, 11:32 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mremotefilelog/remotefilectx.py (7 lines)

Diff 2021

remotefilelog/remotefilectx.py

changeid=introrev, changeid=introrev,
fileid=self._fileid, fileid=self._fileid,
filelog=self._filelog, filelog=self._filelog,
ancestormap=self._ancestormap) ancestormap=self._ancestormap)
# like self.ancestors, but append to "fetch" and skip visiting parents # like self.ancestors, but append to "fetch" and skip visiting parents
# of nodes in "prefetchskip". # of nodes in "prefetchskip".
fetch = [] fetch = []
seen = set()
queue = collections.deque((introctx,)) queue = collections.deque((introctx,))
seen.add(introctx.node())
while queue: while queue:
current = queue.pop() current = queue.pop()
quarkUnsubmitted
Not Done

Could we check and set seen here? It looks simpler.

quark: Could we check and set `seen` here? It looks simpler.
durhamAuthorUnsubmitted
Not Done

If we check it here we can end up queueing things we don't need to queue. In a theoretical bfs that could end up queueing a lot of extra things. That's unlikely in a commit graph, but I figured might as well be efficient.

durham: If we check it here we can end up queueing things we don't need to queue. In a theoretical bfs…
if current.filenode() != self.filenode(): if current.filenode() != self.filenode():
# this is a "joint point". fastannotate needs contents of # this is a "joint point". fastannotate needs contents of
# "joint point"s to calculate diffs for side branches. # "joint point"s to calculate diffs for side branches.
fetch.append((current.path(), hex(current.filenode()))) fetch.append((current.path(), hex(current.filenode())))
if prefetchskip and current in prefetchskip: if prefetchskip and current in prefetchskip:
continue continue
map(queue.append, current.parents()) for parent in current.parents():
if parent.node() not in seen:
seen.add(parent.node())
queue.append(parent)
self._repo.ui.debug('remotefilelog: prefetching %d files ' self._repo.ui.debug('remotefilelog: prefetching %d files '
'for annotate\n' % len(fetch)) 'for annotate\n' % len(fetch))
if fetch: if fetch:
self._repo.fileservice.prefetch(fetch) self._repo.fileservice.prefetch(fetch)
return super(remotefilectx, self).annotate(follow, linenumber, return super(remotefilectx, self).annotate(follow, linenumber,
skiprevs=skiprevs, skiprevs=skiprevs,
diffopts=diffopts) diffopts=diffopts)