This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1690

repack: reuse deltas when possible
ClosedPublic

Authored by durham on Dec 14 2017, 5:39 PM.
Tags
None
Subscribers
quark

Details

Reviewers
quark
Group Reviewers
Restricted Project
Commits
rFBHGX8cb19938d4a7: repack: reuse deltas when possible
Summary

Previously, hg repack would undelta every entry, then redelta it. If the delta
base node was the same before and after (which should be common), this resulted
in a lot of work and made repacking every entry in a chain an n^2 operation.
Let's use the new getdelta API to reuse the delta when possible.

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.Dec 14 2017, 5:39 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 14 2017, 5:39 PM
quark accepted this revision.Dec 14 2017, 10:23 PM
quark added a subscriber: quark.
quark added inline comments.
remotefilelog/repack.py
625

Is the "deltabasename != filename" and "size is None" test necessary?

This revision is now accepted and ready to land.Dec 14 2017, 10:23 PM
durham added inline comments.Dec 15 2017, 2:09 PM
remotefilelog/repack.py
625

The pack format supports delta's against revision with a different name, but repack currently does not. So if the pack has a delta from a different name, we should not try to reuse it.

The size is None test is just because Phil made a change to use size to improve delta choice during repack, and it relies on the size metadata being filled in during a repack. So I'm assuming it is advantageous to do the onetime work here to get the size, so all future packs will be more compact.

Closed by commit rFBHGX8cb19938d4a7: repack: reuse deltas when possible (authored by durham). · Explain WhyDec 15 2017, 2:34 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 4504

remotefilelog/repack.py

# Compute deltas and write to the pack # Compute deltas and write to the pack
for i, node in enumerate(orderednodes): for i, node in enumerate(orderednodes):
deltabase, chainlen = deltabases[node] deltabase, chainlen = deltabases[node]
# Compute delta # Compute delta
# TODO: Optimize the deltachain fetching. Since we're # TODO: Optimize the deltachain fetching. Since we're
# iterating over the different version of the file, we may # iterating over the different version of the file, we may
# be fetching the same deltachain over and over again. # be fetching the same deltachain over and over again.
# TODO: reuse existing deltas if it matches our deltabase meta = None
if deltabase != nullid: if deltabase != nullid:
deltaentry = self.data.getdelta(filename, node)
delta, deltabasename, origdeltabase, meta = deltaentry
size = meta.get(constants.METAKEYSIZE)
if (deltabasename != filename or origdeltabase != deltabase
or size is None):
quarkUnsubmitted
Not Done

Is the "deltabasename != filename" and "size is None" test necessary?

quark: Is the "deltabasename != filename" and "size is None" test necessary?
durhamAuthorUnsubmitted
Not Done

The pack format supports delta's against revision with a different name, but repack currently does not. So if the pack has a delta from a different name, we should not try to reuse it.

The size is None test is just because Phil made a change to use size to improve delta choice during repack, and it relies on the size metadata being filled in during a repack. So I'm assuming it is advantageous to do the onetime work here to get the size, so all future packs will be more compact.

durham: The pack format supports delta's against revision with a different name, but repack currently…
deltabasetext = self.data.get(filename, deltabase) deltabasetext = self.data.get(filename, deltabase)
original = self.data.get(filename, node) original = self.data.get(filename, node)
size = len(original) size = len(original)
delta = mdiff.textdiff(deltabasetext, original) delta = mdiff.textdiff(deltabasetext, original)
else: else:
delta = self.data.get(filename, node) delta = self.data.get(filename, node)
size = len(delta) size = len(delta)
meta = self.data.getmeta(filename, node)
# TODO: don't use the delta if it's larger than the fulltext # TODO: don't use the delta if it's larger than the fulltext
# TODO: don't use the delta if the chain is already long
meta = self.data.getmeta(filename, node)
if constants.METAKEYSIZE not in meta: if constants.METAKEYSIZE not in meta:
meta[constants.METAKEYSIZE] = size meta[constants.METAKEYSIZE] = size
target.add(filename, node, deltabase, delta, meta) target.add(filename, node, deltabase, delta, meta)
entries[node].datarepacked = True entries[node].datarepacked = True
ui.progress(_("processing nodes"), None) ui.progress(_("processing nodes"), None)
count += 1 count += 1

tests/test-remotefilelog-repack.t

Total: 8 8 (0.0% bigger) Total: 8 8 (0.0% bigger)
Test huge pack cleanup using different values of packs.maxpacksize: Test huge pack cleanup using different values of packs.maxpacksize:
$ hg repack --incremental --debug $ hg repack --incremental --debug
$ hg repack --incremental --debug --config packs.maxpacksize=512 $ hg repack --incremental --debug --config packs.maxpacksize=512
removing oversize packfile $TESTTMP/hgcache/master/packs/a2731c9a16403457b67337a620931797fce8c821.datapack (365 bytes) removing oversize packfile $TESTTMP/hgcache/master/packs/a2731c9a16403457b67337a620931797fce8c821.datapack (365 bytes)
removing oversize packfile $TESTTMP/hgcache/master/packs/a2731c9a16403457b67337a620931797fce8c821.dataidx (1.21 KB) removing oversize packfile $TESTTMP/hgcache/master/packs/a2731c9a16403457b67337a620931797fce8c821.dataidx (1.21 KB)
Do a repack where the new pack reuses a delta from the old pack
$ clearcache
$ hg prefetch -r '2::3'
2 files fetched over 1 fetches - (0 misses, 100.00% hit ratio) over * (glob)
$ hg repack
$ hg debugdatapack $CACHEDIR/master/packs/*.datapack
$TESTTMP/hgcache/master/packs/abf210f6c3aa4dd0ecc7033633ad73591be16c95:
x:
Node Delta Base Delta Length Blob Size
1bb2e6237e03 000000000000 8 8
d4a3ed9310e5 1bb2e6237e03 12 6
Total: 20 14 (42.9% bigger)
$ hg prefetch -r '0::1'
2 files fetched over 1 fetches - (0 misses, 100.00% hit ratio) over * (glob)
$ hg repack
$ hg debugdatapack $CACHEDIR/master/packs/*.datapack
$TESTTMP/hgcache/master/packs/09b8bf49256b3fc2175977ba97d6402e91a9a604:
x:
Node Delta Base Delta Length Blob Size
1bb2e6237e03 000000000000 8 8
d4a3ed9310e5 1bb2e6237e03 12 6
aee31534993a d4a3ed9310e5 12 4
1406e7411862 aee31534993a 12 2
Total: 44 20 (120.0% bigger)