This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9283

bundle: optional multithreaded compression, ATM zstd-only
ClosedPublic

Authored by joerg.sonnenberger on Nov 8 2020, 2:49 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG471cd86c8eb4: bundle: optional multithreaded compression, ATM zstd-only
Summary

Compression type can be a huge chunk of "hg bundle", especially when
using the higher compression levels. With level=22 and threads=7, the
NetBSD test repository took 28:39 wall time and 157:47 user time.
Before, level=22 would take 129:20 wall time and 129:07 user time.

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 added a subscriber: indygreg.Nov 9 2020, 11:26 PM
indygreg added inline comments.
mercurial/configitems.py
865–873

None of these support compression. So why define the config options and give false promises?

mercurial/utils/compression.py
692

Multithreaded compression is a compile-time feature. python-zstandard builds the zstd library with this feature enabled. But I'm unsure what downstream packagers who unbundle libzstd are doing. For all I know they have removed the feature.

I'm fine with landing this and letting packagers who insist on making their lives difficult deal with it.

joerg.sonnenberger added inline comments.Nov 18 2020, 11:50 AM
mercurial/configitems.py
865–873

Primarily because otherwise the code has to check a list before querying the config option to avoid warnings. In principle, there are concurrent implementations of both gzip and bzip2, just not available as nicely packaged library. Emulating the bzip2 behavior in a small work queue would actually be moderately easy, I think. But I'm not sure how much interest there is still in bzip2 compression.

mercurial/utils/compression.py
692

I thought we don't support generic python-zstandard at this point anyway? That said, if a user has a zstd build without, it would result in an error when asking for non-default options, which sounds perfectly acceptable to me.

joerg.sonnenberger added a comment.Nov 21 2020, 4:24 PM

I was looking specifically at bzip2 for a bit. There are essentially two kinds of threaded compressors for it. pbzip2 is the more common and creates effectively multiple independent streams. That's not handled transparently by Python's bz2, so it would break all existing clients, making this a big no-go. Sourceforge has a more proper implementation for POSIX platforms (http://bzip2smp.sourceforge.net/) which doesn't have that problem and it would be nice if someone re-implemented the idea for modern libbz2. It can be done cleaner too. While this doesn't allow multi-threaded decompression for multi-stream-aware clients, it does work with all bzip2 decoders. Sadly the way it is done can't be from Python without re-implementing a good chunk of bz2 as it hooks deeply into the implementation. So in short, it would be possible to provide it as C extension and possibly even vendored, but it is more work than I currently want to do. I haven't looked into the state of pigz.

pulkit requested changes to this revision.Mar 10 2021, 4:14 AM
pulkit added a subscriber: pulkit.

I like the idea and since it's an experimental feature, I am happy to push it. @joerg.sonnenberger kindly rebase over current default tip and resend.

mercurial/utils/compression.py
692

Can we add this to documentation somewhere and probably in release notes also?

This revision now requires changes to proceed.Mar 10 2021, 4:14 AM
pulkit added inline comments.Mar 10 2021, 1:38 PM
mercurial/utils/compression.py
692

I will still like to have an explicit comment stating that multithreaded is a compile-time feature and .... somewhere. Above this line of code will work too. I personally know few companies which build hg in their own way and use zstandard compression.

joerg.sonnenberger added a comment.Mar 16 2021, 8:03 AM

D10226 adds a separate packaging note about zstd.

pulkit accepted this revision.Mar 17 2021, 9:50 AM
This revision is now accepted and ready to land.Mar 17 2021, 9:50 AM

Revision Contents
Changeset List

Diff 26482

mercurial/commands.py

complevel = ui.configint( complevel = ui.configint(
b'experimental', b'bundlecomplevel.' + bundlespec.compression b'experimental', b'bundlecomplevel.' + bundlespec.compression
) )
if complevel is None: if complevel is None:
complevel = ui.configint(b'experimental', b'bundlecomplevel') complevel = ui.configint(b'experimental', b'bundlecomplevel')
if complevel is not None: if complevel is not None:
compopts[b'level'] = complevel compopts[b'level'] = complevel
compthreads = ui.configint(
b'experimental', b'bundlecompthreads.' + bundlespec.compression
)
if compthreads is None:
compthreads = ui.configint(b'experimental', b'bundlecompthreads')
if compthreads is not None:
compopts[b'threads'] = compthreads
# Bundling of obsmarker and phases is optional as not all clients # Bundling of obsmarker and phases is optional as not all clients
# support the necessary features. # support the necessary features.
cfg = ui.configbool cfg = ui.configbool
contentopts = { contentopts = {
b'obsolescence': cfg(b'experimental', b'evolution.bundle-obsmarker'), b'obsolescence': cfg(b'experimental', b'evolution.bundle-obsmarker'),
b'obsolescence-mandatory': cfg( b'obsolescence-mandatory': cfg(
b'experimental', b'evolution.bundle-obsmarker:mandatory' b'experimental', b'evolution.bundle-obsmarker:mandatory'
), ),

mercurial/configitems.py

coreconfigitem( coreconfigitem(
b'experimental', b'experimental',
b'bundlecomplevel.none', b'bundlecomplevel.none',
default=None, default=None,
) )
coreconfigitem( coreconfigitem(
b'experimental', b'experimental',
b'bundlecomplevel.zstd', b'bundlecomplevel.zstd',
default=None, default=None,
) )
coreconfigitem( coreconfigitem(
b'experimental', b'experimental',
b'bundlecompthreads',
default=None,
)
coreconfigitem(
b'experimental',
indygregUnsubmitted
Not Done

None of these support compression. So why define the config options and give false promises?

indygreg: None of these support compression. So why define the config options and give false promises?
joerg.sonnenbergerAuthorUnsubmitted
Done

Primarily because otherwise the code has to check a list before querying the config option to avoid warnings. In principle, there are concurrent implementations of both gzip and bzip2, just not available as nicely packaged library. Emulating the bzip2 behavior in a small work queue would actually be moderately easy, I think. But I'm not sure how much interest there is still in bzip2 compression.

joerg.sonnenberger: Primarily because otherwise the code has to check a list before querying the config option to…
b'bundlecompthreads.bzip2',
default=None,
)
coreconfigitem(
b'experimental',
b'bundlecompthreads.gzip',
default=None,
)
coreconfigitem(
b'experimental',
b'bundlecompthreads.none',
default=None,
)
coreconfigitem(
b'experimental',
b'bundlecompthreads.zstd',
default=None,
)
coreconfigitem(
b'experimental',
b'changegroup3', b'changegroup3',
default=False, default=False,
) )
coreconfigitem( coreconfigitem(
b'experimental', b'experimental',
b'cleanup-as-archived', b'cleanup-as-archived',
default=False, default=False,
) )

mercurial/utils/compression.py

return b'\x28' return b'\x28'
def compressstream(self, it, opts=None): def compressstream(self, it, opts=None):
opts = opts or {} opts = opts or {}
# zstd level 3 is almost always significantly faster than zlib # zstd level 3 is almost always significantly faster than zlib
# while providing no worse compression. It strikes a good balance # while providing no worse compression. It strikes a good balance
# between speed and compression. # between speed and compression.
level = opts.get(b'level', 3) level = opts.get(b'level', 3)
# default to single-threaded compression
threads = opts.get(b'threads', 0)
zstd = self._module zstd = self._module
z = zstd.ZstdCompressor(level=level).compressobj() z = zstd.ZstdCompressor(level=level, threads=threads).compressobj()
indygregUnsubmitted
Not Done

Multithreaded compression is a compile-time feature. python-zstandard builds the zstd library with this feature enabled. But I'm unsure what downstream packagers who unbundle libzstd are doing. For all I know they have removed the feature.

I'm fine with landing this and letting packagers who insist on making their lives difficult deal with it.

indygreg: Multithreaded compression is a compile-time feature. python-zstandard builds the zstd library…
joerg.sonnenbergerAuthorUnsubmitted
Done

I thought we don't support generic python-zstandard at this point anyway? That said, if a user has a zstd build without, it would result in an error when asking for non-default options, which sounds perfectly acceptable to me.

joerg.sonnenberger: I thought we don't support generic python-zstandard at this point anyway? That said, if a user…
pulkitUnsubmitted
Not Done

Can we add this to documentation somewhere and probably in release notes also?

pulkit: Can we add this to documentation somewhere and probably in release notes also?
pulkitUnsubmitted
Not Done

I will still like to have an explicit comment stating that multithreaded is a compile-time feature and .... somewhere. Above this line of code will work too. I personally know few companies which build hg in their own way and use zstandard compression.

pulkit: I will still like to have an explicit comment stating that `multithreaded is a compile-time…
for chunk in it: for chunk in it:
data = z.compress(chunk) data = z.compress(chunk)
if data: if data:
yield data yield data
yield z.flush() yield z.flush()
def decompressorreader(self, fh): def decompressorreader(self, fh):

relnotes/next

== New Features == == New Features ==
* `hg purge` is now a core command using `--confirm` by default. * `hg purge` is now a core command using `--confirm` by default.
* The `rev-branch-cache` is now updated incrementally whenever changesets * The `rev-branch-cache` is now updated incrementally whenever changesets
are added. are added.
* The new options `experimental.bundlecompthreads` and
`experimental.bundlecompthreads.<engine>` can be used to instruct
the compression engines for bundle operations to use multiple threads
for compression. The default is single threaded operation. Currently
only supported for zstd.
== New Experimental Features == == New Experimental Features ==
* There's a new `diff.merge` config option to show the changes * There's a new `diff.merge` config option to show the changes
relative to an automerge for merge changesets. This makes it relative to an automerge for merge changesets. This makes it
easier to detect and review manual changes performed in merge easier to detect and review manual changes performed in merge
changesets. It is supported by `hg diff --change`, `hg log -p` changesets. It is supported by `hg diff --change`, `hg log -p`
`hg incoming -p`, and `hg outgoing -p` so far. `hg incoming -p`, and `hg outgoing -p` so far.

tests/test-bundle-type.t

zstd-v1 always fails zstd-v1 always fails
$ hg -R tzstd bundle -a -t zstd-v1 zstd-v1 $ hg -R tzstd bundle -a -t zstd-v1 zstd-v1
abort: compression engine zstd is not supported on v1 bundles abort: compression engine zstd is not supported on v1 bundles
(see 'hg help bundlespec' for supported values for --type) (see 'hg help bundlespec' for supported values for --type)
[10] [10]
zstd supports threading
$ hg init test-compthreads
$ cd test-compthreads
$ hg debugbuilddag +3
$ hg --config experimental.bundlecompthreads=1 bundle -a -t zstd-v2 zstd-v2-threaded.hg
3 changesets found
$ cd ..
#else #else
zstd is a valid engine but isn't available zstd is a valid engine but isn't available
$ hg -R t1 bundle -a -t zstd irrelevant.hg $ hg -R t1 bundle -a -t zstd irrelevant.hg
abort: compression engine zstd could not be loaded abort: compression engine zstd could not be loaded
[255] [255]