This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4803

storageutil: extract most of emitrevisions() to standalone function
ClosedPublic

Authored by indygreg on Sep 28 2018, 8:18 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG842ffcf1d42f: storageutil: extract most of emitrevisions() to standalone function
Summary

As part of implementing a storage backend, I found myself copying
most of revlog.emitrevisions(). This code is highly nuanced and it
bothered me greatly to be copying such low-level code.

This commit extracts the bulk of revlog.emitrevisions() into a
new standalone function. In order to make the function generally
usable, all "self" function calls that aren't exposed on the
ifilestorage interface are passed in via callable arguments.

No meaningful behavior should have changed as part of the port.

Upcoming commits will tweak behavior to make the code more
generically usable.

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

indygreg created this revision.Sep 28 2018, 8:18 PM
durin42 requested changes to this revision.Oct 3 2018, 10:47 AM
durin42 added a subscriber: durin42.
durin42 added inline comments.
mercurial/utils/storageutil.py
266

This is a huge step backwards for remotefilelog and friends: it's all defined in terms of revs, whereas emitrevisions (as copypasta-inducing as it was) was in terms of nodes. We need to do better.

This revision now requires changes to proceed.Oct 3 2018, 10:47 AM
durin42 added a comment.Oct 3 2018, 11:03 AM

Oh, I just got to D4805. I think the helper is still of somewhat limited utility and truly novel storage engines will end up totally reimplementing emitrevisions, but that's fine.

Closed by commit rHG842ffcf1d42f: storageutil: extract most of emitrevisions() to standalone function (authored by indygreg). · Explain WhyOct 3 2018, 11:05 AM
This revision was automatically updated to reflect the committed changes.
indygreg added a comment.Oct 3 2018, 12:21 PM

FWIW I would like to rewrite all of filelog (and this API) to be in terms of nodes. My hands are somewhat tied with this function due to how all the functions are implemented in terms of revs today. We'll get there eventually...

Revision Contents
Changeset List

PathPackages
Mmercurial/revlog.py (106 lines)
Mmercurial/utils/storageutil.py (146 lines)

Diff 11617

mercurial/revlog.py

if nodesorder not in ('nodes', 'storage', None): if nodesorder not in ('nodes', 'storage', None):
raise error.ProgrammingError('unhandled value for nodesorder: %s' % raise error.ProgrammingError('unhandled value for nodesorder: %s' %
nodesorder) nodesorder)
if nodesorder is None and not self._generaldelta: if nodesorder is None and not self._generaldelta:
nodesorder = 'storage' nodesorder = 'storage'
frev = self.rev frev = self.rev
fnode = self.node
if nodesorder == 'nodes': if nodesorder == 'nodes':
revs = [frev(n) for n in nodes] revs = [frev(n) for n in nodes]
elif nodesorder == 'storage': elif nodesorder == 'storage':
revs = sorted(frev(n) for n in nodes) revs = sorted(frev(n) for n in nodes)
else: else:
assert self._generaldelta assert self._generaldelta
revs = set(frev(n) for n in nodes) revs = set(frev(n) for n in nodes)
revs = dagop.linearize(revs, self.parentrevs) revs = dagop.linearize(revs, self.parentrevs)
prevrev = None return storageutil.emitrevisions(
self, revs, revlogrevisiondelta,
if deltaprevious or assumehaveparentrevisions: deltaparentfn=self.deltaparent,
prevrev = self.parentrevs(revs[0])[0] candeltafn=self.candelta,
rawsizefn=self.rawsize,
# Set of revs available to delta against. revdifffn=self.revdiff,
available = set() flagsfn=self.flags,
sendfulltext=not self._storedeltachains,
for rev in revs: revisiondata=revisiondata,
if rev == nullrev: assumehaveparentrevisions=assumehaveparentrevisions,
continue deltaprevious=deltaprevious)
node = fnode(rev)
deltaparentrev = self.deltaparent(rev)
p1rev, p2rev = self.parentrevs(rev)
# Forced delta against previous mode.
if deltaprevious:
baserev = prevrev
# Revlog is configured to use full snapshots. Stick to that.
elif not self._storedeltachains:
baserev = nullrev
# There is a delta in storage. We try to use that because it
# amounts to effectively copying data from storage and is
# therefore the fastest.
elif deltaparentrev != nullrev:
# Base revision was already emitted in this group. We can
# always safely use the delta.
if deltaparentrev in available:
baserev = deltaparentrev
# Base revision is a parent that hasn't been emitted already.
# Use it if we can assume the receiver has the parent revision.
elif (assumehaveparentrevisions
and deltaparentrev in (p1rev, p2rev)):
baserev = deltaparentrev
# No guarantee the receiver has the delta parent. Send delta
# against last revision (if possible), which in the common case
# should be similar enough to this revision that the delta is
# reasonable.
elif prevrev is not None:
baserev = prevrev
else:
baserev = nullrev
# Storage has a fulltext revision.
# Let's use the previous revision, which is as good a guess as any.
# There is definitely room to improve this logic.
elif prevrev is not None:
baserev = prevrev
else:
baserev = nullrev
# But we can't actually use our chosen delta base for whatever
# reason. Reset to fulltext.
if baserev != nullrev and not self.candelta(baserev, rev):
baserev = nullrev
revision = None
delta = None
baserevisionsize = None
if revisiondata:
if self.iscensored(baserev) or self.iscensored(rev):
try:
revision = self.revision(node, raw=True)
except error.CensoredNodeError as e:
revision = e.tombstone
if baserev != nullrev:
baserevisionsize = self.rawsize(baserev)
elif baserev == nullrev and not deltaprevious:
revision = self.revision(node, raw=True)
available.add(rev)
else:
delta = self.revdiff(baserev, rev)
available.add(rev)
yield revlogrevisiondelta(
node=node,
p1node=fnode(p1rev),
p2node=fnode(p2rev),
basenode=fnode(baserev),
flags=self.flags(rev),
baserevisionsize=baserevisionsize,
revision=revision,
delta=delta)
prevrev = rev
DELTAREUSEALWAYS = 'always' DELTAREUSEALWAYS = 'always'
DELTAREUSESAMEREVS = 'samerevs' DELTAREUSESAMEREVS = 'samerevs'
DELTAREUSENEVER = 'never' DELTAREUSENEVER = 'never'
DELTAREUSEFULLADD = 'fulladd' DELTAREUSEFULLADD = 'fulladd'
DELTAREUSEALL = {'always', 'samerevs', 'never', 'fulladd'} DELTAREUSEALL = {'always', 'samerevs', 'never', 'fulladd'}

mercurial/utils/storageutil.py

for p in parentrevsfn(strippoint): for p in parentrevsfn(strippoint):
if p != nullrev: if p != nullrev:
plinkrev = linkrevfn(p) plinkrev = linkrevfn(p)
heads[p] = plinkrev heads[p] = plinkrev
if plinkrev >= minlinkrev: if plinkrev >= minlinkrev:
futurelargelinkrevs.add(plinkrev) futurelargelinkrevs.add(plinkrev)
return strippoint, brokenrevs return strippoint, brokenrevs
def emitrevisions(store, revs, resultcls, deltaparentfn, candeltafn,
durin42Unsubmitted
Not Done

This is a huge step backwards for remotefilelog and friends: it's all defined in terms of revs, whereas emitrevisions (as copypasta-inducing as it was) was in terms of nodes. We need to do better.

durin42: This is a huge step backwards for remotefilelog and friends: it's all defined in terms of revs…
rawsizefn, revdifffn, flagsfn, sendfulltext=False,
revisiondata=False, assumehaveparentrevisions=False,
deltaprevious=False):
"""Generic implementation of ifiledata.emitrevisions().
Emitting revision data is subtly complex. This function attempts to
encapsulate all the logic for doing so in a backend-agnostic way.
``store``
Object conforming to ``ifilestorage`` interface.
``revs``
List of integer revision numbers whose data to emit.
``resultcls``
A type implementing the ``irevisiondelta`` interface that will be
constructed and returned.
``deltaparentfn``
Callable receiving a revision number and returning the revision number
of a revision that the internal delta is stored against. This delta
will be preferred over computing a new arbitrary delta.
``candeltafn``
Callable receiving a pair of revision numbers that returns a bool
indicating whether a delta between them can be produced.
``rawsizefn``
Callable receiving a revision number and returning the length of the
``store.revision(rev, raw=True)``.
``revdifffn``
Callable receiving a pair of revision numbers that returns a delta
between them.
``flagsfn``
Callable receiving a revision number and returns the integer flags
value for it.
``sendfulltext``
Whether to send fulltext revisions instead of deltas, if allowed.
``revisiondata``
``assumehaveparentrevisions``
``deltaprevious``
See ``ifiledata.emitrevisions()`` interface documentation.
"""
fnode = store.node
prevrev = None
if deltaprevious or assumehaveparentrevisions:
prevrev = store.parentrevs(revs[0])[0]
# Set of revs available to delta against.
available = set()
for rev in revs:
if rev == nullrev:
continue
node = fnode(rev)
deltaparentrev = deltaparentfn(rev)
p1rev, p2rev = store.parentrevs(rev)
# Forced delta against previous mode.
if deltaprevious:
baserev = prevrev
# We're instructed to send fulltext. Honor that.
elif sendfulltext:
baserev = nullrev
# There is a delta in storage. We try to use that because it
# amounts to effectively copying data from storage and is
# therefore the fastest.
elif deltaparentrev != nullrev:
# Base revision was already emitted in this group. We can
# always safely use the delta.
if deltaparentrev in available:
baserev = deltaparentrev
# Base revision is a parent that hasn't been emitted already.
# Use it if we can assume the receiver has the parent revision.
elif (assumehaveparentrevisions
and deltaparentrev in (p1rev, p2rev)):
baserev = deltaparentrev
# No guarantee the receiver has the delta parent. Send delta
# against last revision (if possible), which in the common case
# should be similar enough to this revision that the delta is
# reasonable.
elif prevrev is not None:
baserev = prevrev
else:
baserev = nullrev
# Storage has a fulltext revision.
# Let's use the previous revision, which is as good a guess as any.
# There is definitely room to improve this logic.
elif prevrev is not None:
baserev = prevrev
else:
baserev = nullrev
# But we can't actually use our chosen delta base for whatever
# reason. Reset to fulltext.
if baserev != nullrev and not candeltafn(baserev, rev):
baserev = nullrev
revision = None
delta = None
baserevisionsize = None
if revisiondata:
if store.iscensored(baserev) or store.iscensored(rev):
try:
revision = store.revision(node, raw=True)
except error.CensoredNodeError as e:
revision = e.tombstone
if baserev != nullrev:
baserevisionsize = rawsizefn(baserev)
elif baserev == nullrev and not deltaprevious:
revision = store.revision(node, raw=True)
available.add(rev)
else:
delta = revdifffn(baserev, rev)
available.add(rev)
yield resultcls(
node=node,
p1node=fnode(p1rev),
p2node=fnode(p2rev),
basenode=fnode(baserev),
flags=flagsfn(rev),
baserevisionsize=baserevisionsize,
revision=revision,
delta=delta)
prevrev = rev