This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4227

repository: remove storedeltachains from ifilestorage
ClosedPublic

Authored by indygreg on Aug 9 2018, 7:12 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG0a5b20c107a6: repository: remove storedeltachains from ifilestorage
Summary

The ifilestorage interface was bootstrapped from requirements of
callers outside the storage implementation (revlogs). I believe we
even made some members public so they could be part of the interface!

Historically, the changegroup code was a gross offender when it
came to accessing low-level storage primitives. There are a handful
of members on the ifilestorage interface that are/were used only
for changegroup code.

With the recent refactor of changegroup code and the establishment
of a formal API on the storage interface for producing revision
deltas, the changegroup code is no longer accessing these low-level
primitives related to delta generation directly. Instead, things
are abstracted away in the storage implementation.

This means we can remove elements from the storage interface that
are no longer needed.

We start with "storedeltachains."

We remove it from the interface. Then we make it a private
attribute and update all references.

.. api:: storedeltachains has been dropped from ifilestorage interface

.. api:: storedeltachains on revlog classes is now _storedeltachains

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.

Revision Contents
Changeset List

Diff 10512

mercurial/changelog.py

if self._initempty: if self._initempty:
# changelogs don't benefit from generaldelta # changelogs don't benefit from generaldelta
self.version &= ~revlog.FLAG_GENERALDELTA self.version &= ~revlog.FLAG_GENERALDELTA
self._generaldelta = False self._generaldelta = False
# Delta chains for changelogs tend to be very small because entries # Delta chains for changelogs tend to be very small because entries
# tend to be small and don't delta well with each. So disable delta # tend to be small and don't delta well with each. So disable delta
# chains. # chains.
self.storedeltachains = False self._storedeltachains = False
self._realopener = opener self._realopener = opener
self._delayed = False self._delayed = False
self._delaybuf = None self._delaybuf = None
self._divert = False self._divert = False
self.filteredrevs = frozenset() self.filteredrevs = frozenset()
def tiprev(self): def tiprev(self):

mercurial/filelog.py

def __init__(self, opener, path): def __init__(self, opener, path):
self._revlog = revlog.revlog(opener, self._revlog = revlog.revlog(opener,
'/'.join(('data', path + '.i')), '/'.join(('data', path + '.i')),
censorable=True) censorable=True)
# full name of the user visible file, relative to the repository root # full name of the user visible file, relative to the repository root
self.filename = path self.filename = path
self.index = self._revlog.index self.index = self._revlog.index
self.version = self._revlog.version self.version = self._revlog.version
self.storedeltachains = self._revlog.storedeltachains
self._generaldelta = self._revlog._generaldelta self._generaldelta = self._revlog._generaldelta
def __len__(self): def __len__(self):
return len(self._revlog) return len(self._revlog)
def __iter__(self): def __iter__(self):
return self._revlog.__iter__() return self._revlog.__iter__()

mercurial/repository.py

"""Complete storage interface for a single tracked file.""" """Complete storage interface for a single tracked file."""
version = interfaceutil.Attribute( version = interfaceutil.Attribute(
"""Version number of storage. """Version number of storage.
TODO this feels revlog centric and could likely be removed. TODO this feels revlog centric and could likely be removed.
""") """)
storedeltachains = interfaceutil.Attribute(
"""Whether the store stores deltas.
TODO deltachains are revlog centric. This can probably removed
once there are better abstractions for obtaining/writing
data.
""")
_generaldelta = interfaceutil.Attribute( _generaldelta = interfaceutil.Attribute(
"""Whether deltas can be against any parent revision. """Whether deltas can be against any parent revision.
TODO this is used by changegroup code and it could probably be TODO this is used by changegroup code and it could probably be
folded into another API. folded into another API.
""") """)
def files(): def files():

mercurial/revlog.py

""" """
revlog = self.revlog revlog = self.revlog
gdelta = revlog._generaldelta gdelta = revlog._generaldelta
curr = len(revlog) curr = len(revlog)
prev = curr - 1 prev = curr - 1
p1r, p2r = revlog.rev(p1), revlog.rev(p2) p1r, p2r = revlog.rev(p1), revlog.rev(p2)
# should we try to build a delta? # should we try to build a delta?
if prev != nullrev and revlog.storedeltachains: if prev != nullrev and revlog._storedeltachains:
tested = set() tested = set()
# This condition is true most of the time when processing # This condition is true most of the time when processing
# changegroup data into a generaldelta repo. The only time it # changegroup data into a generaldelta repo. The only time it
# isn't true is if this is the first revision in a delta chain # isn't true is if this is the first revision in a delta chain
# or if ``format.generaldelta=true`` disabled ``lazydeltabase``. # or if ``format.generaldelta=true`` disabled ``lazydeltabase``.
if cachedelta and gdelta and revlog._lazydeltabase: if cachedelta and gdelta and revlog._lazydeltabase:
# Assume what we received from the server is a good choice # Assume what we received from the server is a good choice
# build delta will reuse the cache # build delta will reuse the cache
if flags & ~REVLOGV2_FLAGS: if flags & ~REVLOGV2_FLAGS:
raise RevlogError(_('unknown flags (%#04x) in version %d ' raise RevlogError(_('unknown flags (%#04x) in version %d '
'revlog %s') % 'revlog %s') %
(flags >> 16, fmt, self.indexfile)) (flags >> 16, fmt, self.indexfile))
else: else:
raise RevlogError(_('unknown version (%d) in revlog %s') % raise RevlogError(_('unknown version (%d) in revlog %s') %
(fmt, self.indexfile)) (fmt, self.indexfile))
self.storedeltachains = True self._storedeltachains = True
self._io = revlogio() self._io = revlogio()
if self.version == REVLOGV0: if self.version == REVLOGV0:
self._io = revlogoldio() self._io = revlogoldio()
try: try:
d = self._io.parseindex(indexdata, self._inline) d = self._io.parseindex(indexdata, self._inline)
except (ValueError, IndexError): except (ValueError, IndexError):
raise RevlogError(_("index %s is corrupted") % (self.indexfile)) raise RevlogError(_("index %s is corrupted") % (self.indexfile))
# Avoid sending full revisions when delta parent is null. Pick # Avoid sending full revisions when delta parent is null. Pick
# prev in that case. It's tempting to pick p1 in this case, as # prev in that case. It's tempting to pick p1 in this case, as
# p1 will be smaller in the common case. However, computing a # p1 will be smaller in the common case. However, computing a
# delta against p1 may require resolving the raw text of p1, # delta against p1 may require resolving the raw text of p1,
# which could be expensive. The revlog caches should have prev # which could be expensive. The revlog caches should have prev
# cached, meaning less CPU for delta generation. There is # cached, meaning less CPU for delta generation. There is
# likely room to add a flag and/or config option to control this # likely room to add a flag and/or config option to control this
# behavior. # behavior.
if deltaparentrev == nullrev and self.storedeltachains: if deltaparentrev == nullrev and self._storedeltachains:
baserev = prevrev baserev = prevrev
# Revlog is configured to use full snapshot for a reason. # Revlog is configured to use full snapshot for a reason.
# Stick to full snapshot. # Stick to full snapshot.
elif deltaparentrev == nullrev: elif deltaparentrev == nullrev:
baserev = nullrev baserev = nullrev
# Pick previous when we can't be sure the base is available # Pick previous when we can't be sure the base is available

tests/simplestorerepo.py

self._indexdata = indexdata or [] self._indexdata = indexdata or []
self._indexbynode = {} self._indexbynode = {}
self._indexbyrev = {} self._indexbyrev = {}
self.index = [] self.index = []
self._refreshindex() self._refreshindex()
# This is used by changegroup code :/ # This is used by changegroup code :/
self._generaldelta = True self._generaldelta = True
self.storedeltachains = False
self.version = 1 self.version = 1
def _refreshindex(self): def _refreshindex(self):
self._indexbynode.clear() self._indexbynode.clear()
self._indexbyrev.clear() self._indexbyrev.clear()
self.index = [] self.index = []

tests/test-revlog-raw.py

# TESTTMP is optional. This makes it convenient to run without run-tests.py # TESTTMP is optional. This makes it convenient to run without run-tests.py
tvfs = vfs.vfs(encoding.environ.get(b'TESTTMP', b'/tmp')) tvfs = vfs.vfs(encoding.environ.get(b'TESTTMP', b'/tmp'))
# Enable generaldelta otherwise revlog won't use delta as expected by the test # Enable generaldelta otherwise revlog won't use delta as expected by the test
tvfs.options = {b'generaldelta': True, b'revlogv1': True} tvfs.options = {b'generaldelta': True, b'revlogv1': True}
# The test wants to control whether to use delta explicitly, based on # The test wants to control whether to use delta explicitly, based on
# "storedeltachains". # "storedeltachains".
revlog.revlog._isgooddeltainfo = lambda self, d, textlen: self.storedeltachains revlog.revlog._isgooddeltainfo = lambda self, d, textlen: self._storedeltachains
def abort(msg): def abort(msg):
print('abort: %s' % msg) print('abort: %s' % msg)
# Return 0 so run-tests.py could compare the output. # Return 0 so run-tests.py could compare the output.
sys.exit() sys.exit()
# Register a revlog processor for flag EXTSTORED. # Register a revlog processor for flag EXTSTORED.
# #
nextrev = len(rlog) nextrev = len(rlog)
p1 = rlog.node(nextrev - 1) p1 = rlog.node(nextrev - 1)
p2 = node.nullid p2 = node.nullid
if isext: if isext:
flags = revlog.REVIDX_EXTSTORED flags = revlog.REVIDX_EXTSTORED
else: else:
flags = revlog.REVIDX_DEFAULT_FLAGS flags = revlog.REVIDX_DEFAULT_FLAGS
# Change storedeltachains temporarily, to override revlog's delta decision # Change storedeltachains temporarily, to override revlog's delta decision
rlog.storedeltachains = isdelta rlog._storedeltachains = isdelta
try: try:
rlog.addrevision(text, tr, nextrev, p1, p2, flags=flags) rlog.addrevision(text, tr, nextrev, p1, p2, flags=flags)
return nextrev return nextrev
except Exception as ex: except Exception as ex:
abort('rev %d: failed to append: %s' % (nextrev, ex)) abort('rev %d: failed to append: %s' % (nextrev, ex))
finally: finally:
# Restore storedeltachains. It is always True, see revlog.__init__ # Restore storedeltachains. It is always True, see revlog.__init__
rlog.storedeltachains = True rlog._storedeltachains = True
def addgroupcopy(rlog, tr, destname=b'_destrevlog.i', optimaldelta=True): def addgroupcopy(rlog, tr, destname=b'_destrevlog.i', optimaldelta=True):
'''Copy revlog to destname using revlog.addgroup. Return the copied revlog. '''Copy revlog to destname using revlog.addgroup. Return the copied revlog.
This emulates push or pull. They use changegroup. Changegroup requires This emulates push or pull. They use changegroup. Changegroup requires
repo to work. We don't have a repo, so a dummy changegroup is used. repo to work. We don't have a repo, so a dummy changegroup is used.
If optimaldelta is True, use optimized delta parent, so the destination If optimaldelta is True, use optimized delta parent, so the destination