Patchwork [04,of,15] streamclone: define first iteration of version 2 of stream format

login
register
mail settings
Submitter Boris Feld
Date Jan. 19, 2018, 8:08 p.m.
Message ID <4ee91fb55e208e8b1395.1516392528@FB>
Download mbox | patch
Permalink /patch/26962/
State Deferred, archived
Headers show

Comments

Boris Feld - Jan. 19, 2018, 8:08 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1516232936 -3600
#      Thu Jan 18 00:48:56 2018 +0100
# Node ID 4ee91fb55e208e8b139595ce9c2cae25aa9c54ea
# Parent  b80a8e39ac9bf984c25a666bd7f6c47d876d26af
# EXP-Topic b2-stream
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 4ee91fb55e20
streamclone: define first iteration of version 2 of stream format

(This patch is based on a first draft from Gregory Szorc, with deeper rework)

Version 1 of the stream clone format was invented many years ago and suffers
from a few deficiencies:

1) Filenames are stored in store-encoded (on filesystem) form rather than in
   their internal form. This makes future compatibility with new store
   filename encodings more difficult.
2) File entry "headers" consist of a newline of the file name followed by the
   string file size. Converting strings to integers is avoidable overhead. We
   can't store filenames with newlines (manifests have this limitation as
   well, so it isn't a major concern). But the big concern here is the
   necessity for readline(). Scanning for newlines means reading ahead and
   that means extra buffer allocations and slicing (in Python) and this makes
   performance suffer.
3) Filenames aren't compressed optimally. Filenames should be compressed well
   since there is a lot of repeated data. However, since they are scattered
   all over the stream (with revlog data in between), they typically fall
   outside the window size of the compressor and don't compress.
4) It can only exchange stored based content, being able to exchange caches
   too would be nice.
5) It is limited to a stream-based protocol and isn't suitable for an on-disk
   format for general repository reading because the offset of individual file
   entries requires scanning the entire file to find file records.

As part of enabling streaming clones to work in bundle2, #2 proved to have a
significant negative impact on performance. Since bundle2 provides the
opportunity to start fresh, Gregory Szorc figured he would take the
opportunity to invent a new streaming clone data format.

The new format devised in this series addresses #1, #2, and #4. It punts on #3
because it was complex without yielding a significant gain and on #5 because
devising a new store format that "packs" multiple revlogs into a single
"packed revlog" is massive scope bloat. However, this v2 format might be
suitable for streaming into a "packed revlog" with minimal processing. If it
works, great. If not, we can always invent stream format when it is needed.

This patch only introduces the bases of the format. We'll get it usable through
bundle2 first, then we'll extend the format in future patches to bring it to its
full potential (especially #4).
Augie Fackler - Jan. 19, 2018, 8:54 p.m.
On Fri, Jan 19, 2018 at 09:08:48PM +0100, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1516232936 -3600
> #      Thu Jan 18 00:48:56 2018 +0100
> # Node ID 4ee91fb55e208e8b139595ce9c2cae25aa9c54ea
> # Parent  b80a8e39ac9bf984c25a666bd7f6c47d876d26af
> # EXP-Topic b2-stream
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 4ee91fb55e20
> streamclone: define first iteration of version 2 of stream format

This is a good start of a series, but:

1) patch 3 is begging for doctests on the varint scheme

2) This patch should probably include help/internals/ documentation on
   the format, rather than only encoding it in a docstring

3) Patches that claim to be a big performance win, but don't include
   any concrete testing numbers.

I think at this point we're going to admit defeat in the name of
getting an RC release done before I go to bed tonight. Performance
information I'd like to see in any v3 of this series:

1) comparison between the new streaming clone and the existing one on small repos

2) comparison on a medium repo with few branches (the hg repo could be good for this)

3) comparison on a large repo with many heads (might need to use contrib/synthrepo to make something for this?)

4) comparison on a large repo with many named branches (pypy?)

5) comparison on mozilla-central

I'm gathering that this is important work for you all for an important
client or set of clients, and I'm sorry we're not going to manage it
today. In the future, you could potentially sidestep some of this
frustration by giving people a heads up earlier than a
performance-critical 14 patch series with a new wireproto format.
Boris Feld - Jan. 19, 2018, 9:35 p.m.
On Fri, 2018-01-19 at 15:54 -0500, Augie Fackler wrote:
> On Fri, Jan 19, 2018 at 09:08:48PM +0100, Boris Feld wrote:
> > # HG changeset patch
> > # User Boris Feld <boris.feld@octobus.net>
> > # Date 1516232936 -3600
> > #      Thu Jan 18 00:48:56 2018 +0100
> > # Node ID 4ee91fb55e208e8b139595ce9c2cae25aa9c54ea
> > # Parent  b80a8e39ac9bf984c25a666bd7f6c47d876d26af
> > # EXP-Topic b2-stream
> > # Available At https://bitbucket.org/octobus/mercurial-devel/
> > #              hg pull https://bitbucket.org/octobus/mercurial-deve
> > l/ -r 4ee91fb55e20
> > streamclone: define first iteration of version 2 of stream format
> 
> This is a good start of a series, but:
> 
> 1) patch 3 is begging for doctests on the varint scheme
> 

We are about to follow up with them.

> 2) This patch should probably include help/internals/ documentation
> on
>    the format, rather than only encoding it in a docstring
> 

We can follow up with that.

> 3) Patches that claim to be a big performance win, but don't include
>    any concrete testing numbers.

The performance win is about recomputing cache. We are getting number
as we speak, but performance issue related to branchmap and tags have
been well documented in the past.

On our repository with the most heads, it took 25 minutes to recompute
the tags cache and 1 minute for the branch cache. And the laptop wasn't
doing much else.

> 
> I think at this point we're going to admit defeat in the name of
> getting an RC release done before I go to bed tonight. Performance
> information I'd like to see in any v3 of this series:
> 
> 1) comparison between the new streaming clone and the existing one on
> small repos
> 
> 2) comparison on a medium repo with few branches (the hg repo could
> be good for this)
> 
> 3) comparison on a large repo with many heads (might need to use
> contrib/synthrepo to make something for this?)
> 
> 4) comparison on a large repo with many named branches (pypy?)
> 
> 5) comparison on mozilla-central

Those characteristics shouldn't impact the performance of the stream
clone.

> 
> I'm gathering that this is important work for you all for an
> important
> client or set of clients, and I'm sorry we're not going to manage it
> today. In the future, you could potentially sidestep some of this
> frustration by giving people a heads up earlier than a
> performance-critical 14 patch series with a new wireproto format.
Augie Fackler - Jan. 19, 2018, 9:39 p.m.
> On Jan 19, 2018, at 16:35, Boris Feld <boris.feld@octobus.net> wrote:
> 
> On Fri, 2018-01-19 at 15:54 -0500, Augie Fackler wrote:
>> On Fri, Jan 19, 2018 at 09:08:48PM +0100, Boris Feld wrote:
>>> # HG changeset patch
>>> # User Boris Feld <boris.feld@octobus.net>
>>> # Date 1516232936 -3600
>>> #      Thu Jan 18 00:48:56 2018 +0100
>>> # Node ID 4ee91fb55e208e8b139595ce9c2cae25aa9c54ea
>>> # Parent  b80a8e39ac9bf984c25a666bd7f6c47d876d26af
>>> # EXP-Topic b2-stream
>>> # Available At https://bitbucket.org/octobus/mercurial-devel/
>>> #              hg pull https://bitbucket.org/octobus/mercurial-deve
>>> l/ -r 4ee91fb55e20
>>> streamclone: define first iteration of version 2 of stream format
>> 
>> This is a good start of a series, but:
>> 
>> 1) patch 3 is begging for doctests on the varint scheme
>> 
> 
> We are about to follow up with them.

Just hold them for 4.6 please.

> 
>> 2) This patch should probably include help/internals/ documentation
>> on
>>   the format, rather than only encoding it in a docstring
>> 
> 
> We can follow up with that.
> 
>> 3) Patches that claim to be a big performance win, but don't include
>>   any concrete testing numbers.
> 
> The performance win is about recomputing cache. We are getting number
> as we speak, but performance issue related to branchmap and tags have
> been well documented in the past.

You're mostly missing the point: the reason for this significant body of new functionality was performance wins. I'd expect patches that come in under a performance banner to explain, including with benchmarks why they're a win. In this case, that includes not only explaining that we avoid a 25 minute cache hit (which is great), but also some testing that demonstrates that the new format is at least *not worse* than what was there before. bundle2 doesn't have a spotless performance record, so I get reflexively dubious when a new format inside bundle2 claims to be a win. :)

> On our repository with the most heads, it took 25 minutes to recompute
> the tags cache and 1 minute for the branch cache. And the laptop wasn't
> doing much else.
> 
>> 
>> I think at this point we're going to admit defeat in the name of
>> getting an RC release done before I go to bed tonight. Performance
>> information I'd like to see in any v3 of this series:
>> 
>> 1) comparison between the new streaming clone and the existing one on
>> small repos
>> 
>> 2) comparison on a medium repo with few branches (the hg repo could
>> be good for this)
>> 
>> 3) comparison on a large repo with many heads (might need to use
>> contrib/synthrepo to make something for this?)
>> 
>> 4) comparison on a large repo with many named branches (pypy?)
>> 
>> 5) comparison on mozilla-central
> 
> Those characteristics shouldn't impact the performance of the stream
> clone.

It's more about relative overheads and things. My gut is that involving bundle2 adds measurable overhead to the process (though I could be wrong!), so I'd like a decent spectrum of repo types to try and help figure out how much the cache recomputation is a win or not. Does that make sense?

Anyway, it's too late for this cycle, as I'm about out of time in my day to make a release. Sorry. :/

Patch

diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py
--- a/mercurial/streamclone.py
+++ b/mercurial/streamclone.py
@@ -428,3 +428,115 @@  class streamcloneapplier(object):
 
     def apply(self, repo):
         return applybundlev1(repo, self._fh)
+
+def _emit(repo, entries, totalfilesize):
+    """actually emit the stream bundle"""
+    progress = repo.ui.progress
+    progress(_('bundle'), 0, total=totalfilesize, unit=_('bytes'))
+    vfs = repo.svfs
+    try:
+        seen = 0
+        for name, size in entries:
+            yield util.uvarintencode(len(name))
+            fp = vfs(name)
+            try:
+                yield util.uvarintencode(size)
+                yield name
+                if size <= 65536:
+                    chunks = (fp.read(size),)
+                else:
+                    chunks = util.filechunkiter(fp, limit=size)
+                for chunk in chunks:
+                    seen += len(chunk)
+                    progress(_('bundle'), seen, total=totalfilesize,
+                             unit=_('bytes'))
+                    yield chunk
+            finally:
+                fp.close()
+    finally:
+        progress(_('bundle'), None)
+
+def generatev2(repo):
+    """Emit content for version 2 of a streaming clone.
+
+    the data stream consists the following entries:
+    1) A varint containing the length of the filename
+    2) A varint containing the length of file data
+    3) N bytes containing the filename (the internal, store-agnostic form)
+    4) N bytes containing the file data
+
+    Returns a 3-tuple of (file count, file size, data iterator).
+    """
+
+    with repo.lock():
+
+        entries = []
+        totalfilesize = 0
+
+        repo.ui.debug('scanning\n')
+        for name, ename, size in _walkstreamfiles(repo):
+            if size:
+                entries.append((name, size))
+                totalfilesize += size
+
+        chunks = _emit(repo, entries, totalfilesize)
+
+    return len(entries), totalfilesize, chunks
+
+def consumev2(repo, fp, filecount, filesize):
+    """Apply the contents from a version 2 streaming clone.
+
+    Data is read from an object that only needs to provide a ``read(size)``
+    method.
+    """
+    with repo.lock():
+        repo.ui.status(_('%d files to transfer, %s of data\n') %
+                       (filecount, util.bytecount(filesize)))
+
+        start = util.timer()
+        handledbytes = 0
+        progress = repo.ui.progress
+
+        progress(_('clone'), handledbytes, total=filesize, unit=_('bytes'))
+
+        vfs = repo.svfs
+
+        with repo.transaction('clone'):
+            with vfs.backgroundclosing(repo.ui):
+                for i in range(filecount):
+                    namelen = util.uvarintdecodestream(fp)
+                    datalen = util.uvarintdecodestream(fp)
+
+                    name = fp.read(namelen)
+
+                    if repo.ui.debugflag:
+                        repo.ui.debug('adding %s (%s)\n' %
+                                      (name, util.bytecount(datalen)))
+
+                    with vfs(name, 'w') as ofp:
+                        for chunk in util.filechunkiter(fp, limit=datalen):
+                            handledbytes += len(chunk)
+                            progress(_('clone'), handledbytes, total=filesize,
+                                     unit=_('bytes'))
+                            ofp.write(chunk)
+
+            # force @filecache properties to be reloaded from
+            # streamclone-ed file at next access
+            repo.invalidate(clearfilecache=True)
+
+        elapsed = util.timer() - start
+        if elapsed <= 0:
+            elapsed = 0.001
+        progress(_('clone'), None)
+        repo.ui.status(_('transferred %s in %.1f seconds (%s/sec)\n') %
+                       (util.bytecount(handledbytes), elapsed,
+                        util.bytecount(handledbytes / elapsed)))
+
+def applybundlev2(repo, fp, filecount, filesize, requirements):
+    missingreqs = [r for r in requirements if r not in repo.supported]
+    if missingreqs:
+        raise error.Abort(_('unable to apply stream clone: '
+                            'unsupported format: %s') %
+                          ', '.join(sorted(missingreqs)))
+
+    consumev2(repo, fp, filecount, filesize)