Patchwork [1,of,8,zstd-revlogs] revlog: make compressed size comparisons consistent

login
register
mail settings
Submitter Gregory Szorc
Date Jan. 2, 2017, 11:57 p.m.
Message ID <6740dc7106f3ce8aec48.1483401472@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/18075/
State Accepted
Headers show

Comments

Gregory Szorc - Jan. 2, 2017, 11:57 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1483386617 28800
#      Mon Jan 02 11:50:17 2017 -0800
# Node ID 6740dc7106f3ce8aec48c6ebb67153753ac88aac
# Parent  dc5b594f41e9be5820ce3f197d3817379b2d3af5
revlog: make compressed size comparisons consistent

revlog.compress() compares the compressed size to the input size
and throws away the compressed data if it is larger than the input.
This is the correct thing to do, as storing compressed data that
is larger than the input takes up more storage space and makes reading
slower.

However, the comparison was implemented inconsistently. For the
streaming compression mode, we threw away the result if it was
greater than or equal to the input size. But for the one-shot
compression, we threw away the compression only if it was greater
than the input size!

This patch changes the comparison for the simple case so it is
consistent with the streaming case.

As a few tests demonstrate, this adds 1 byte to some revlog entries.
This is because of an added 'u' header on the chunk. It seems
somewhat wrong to increase the revlog size here. However, IMO the cost
of 1 byte in storage is insignificant compared to the performance gains
of avoiding decompression. This patch should invite questions around
the heuristic for throwing away compressed data. For example, I'd argue
we should be more liberal about rejecting compressed data, additionally
doing so where the number of bytes saved fails to reach a threshold.
But we can have this discussion another time.

Patch

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -1386,7 +1386,7 @@  class revlog(object):
                 bin = "".join(p)
         else:
             bin = _compress(text)
-        if bin is None or len(bin) > l:
+        if bin is None or len(bin) >= l:
             if text[0] == '\0':
                 return ("", text)
             return ('u', text)
diff --git a/tests/test-bundle.t b/tests/test-bundle.t
--- a/tests/test-bundle.t
+++ b/tests/test-bundle.t
@@ -268,13 +268,13 @@  Cannot produce streaming clone bundles w
 packed1 is produced properly
 
   $ hg -R test debugcreatestreamclonebundle packed.hg
-  writing 2663 bytes for 6 files
+  writing 2664 bytes for 6 files
   bundle requirements: generaldelta, revlogv1
 
   $ f -B 64 --size --sha1 --hexdump packed.hg
-  packed.hg: size=2826, sha1=e139f97692a142b19cdcff64a69697d5307ce6d4
+  packed.hg: size=2827, sha1=9d14cb90c66a21462d915ab33656f38b9deed686
   0000: 48 47 53 31 55 4e 00 00 00 00 00 00 00 06 00 00 |HGS1UN..........|
-  0010: 00 00 00 00 0a 67 00 16 67 65 6e 65 72 61 6c 64 |.....g..generald|
+  0010: 00 00 00 00 0a 68 00 16 67 65 6e 65 72 61 6c 64 |.....h..generald|
   0020: 65 6c 74 61 2c 72 65 76 6c 6f 67 76 31 00 64 61 |elta,revlogv1.da|
   0030: 74 61 2f 61 64 69 66 66 65 72 65 6e 74 66 69 6c |ta/adifferentfil|
 
diff --git a/tests/test-commit-amend.t b/tests/test-commit-amend.t
--- a/tests/test-commit-amend.t
+++ b/tests/test-commit-amend.t
@@ -1146,7 +1146,7 @@  directory)
   R olddirname/newfile.py
   $ hg debugindex newdirname/newfile.py
      rev    offset  length  delta linkrev nodeid       p1           p2
-       0         0      88     -1       3 34a4d536c0c0 000000000000 000000000000
+       0         0      89     -1       3 34a4d536c0c0 000000000000 000000000000
 
   $ echo a >> newdirname/commonfile.py
   $ hg ci --amend -m bug
@@ -1154,7 +1154,7 @@  directory)
   newdirname/newfile.py renamed from olddirname/newfile.py:690b295714aed510803d3020da9c70fca8336def (glob)
   $ hg debugindex newdirname/newfile.py
      rev    offset  length  delta linkrev nodeid       p1           p2
-       0         0      88     -1       3 34a4d536c0c0 000000000000 000000000000
+       0         0      89     -1       3 34a4d536c0c0 000000000000 000000000000
 
 #if execbit