Patchwork D7894: nodemap: introduce an option to use mmap to read the nodemap mapping

login
register
mail settings
Submitter phabricator
Date Feb. 15, 2020, 7:15 p.m.
Message ID <c019be7903a97dce0114529593651d0f@localhost.localdomain>
Download mbox | patch
Permalink /patch/45257/
State Not Applicable
Headers show

Comments

phabricator - Feb. 15, 2020, 7:15 p.m.
Closed by commit rHGf7459da77f23: nodemap: introduce an option to use mmap to read the nodemap mapping (authored by marmoute).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs Revision".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7894?vs=20165&id=20240

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7894/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7894

AFFECTED FILES
  mercurial/configitems.py
  mercurial/debugcommands.py
  mercurial/localrepo.py
  mercurial/revlog.py
  mercurial/revlogutils/nodemap.py
  tests/test-persistent-nodemap.t

CHANGE DETAILS




To: marmoute, #hg-reviewers, durin42, indygreg
Cc: durin42, gracinet, martinvonz, mercurial-devel

Patch

diff --git a/tests/test-persistent-nodemap.t b/tests/test-persistent-nodemap.t
--- a/tests/test-persistent-nodemap.t
+++ b/tests/test-persistent-nodemap.t
@@ -84,3 +84,37 @@ 
   $ hg debugnodemap --check
   revision in index:   5002
   revision in nodemap: 5002
+
+Test code path without mmap
+---------------------------
+
+  $ echo bar > bar
+  $ hg add bar
+  $ hg ci -m 'bar' --config experimental.exp-persistent-nodemap.mmap=no
+
+  $ hg debugnodemap --check --config experimental.exp-persistent-nodemap.mmap=yes
+  revision in index:   5003
+  revision in nodemap: 5003
+  $ hg debugnodemap --check --config experimental.exp-persistent-nodemap.mmap=no
+  revision in index:   5003
+  revision in nodemap: 5003
+
+
+#if pure
+  $ hg debugnodemap --metadata
+  uid: ???????????????? (glob)
+  tip-rev: 5002
+  data-length: 123328
+  data-unused: 384
+  $ f --sha256 .hg/store/00changelog-*.nd --size
+  .hg/store/00changelog-????????????????.nd: size=123328, sha256=10d26e9776b6596af0f89143a54eba8cc581e929c38242a02a7b0760698c6c70 (glob)
+
+#else
+  $ hg debugnodemap --metadata
+  uid: ???????????????? (glob)
+  tip-rev: 5002
+  data-length: 122944
+  data-unused: 0
+  $ f --sha256 .hg/store/00changelog-*.nd --size
+  .hg/store/00changelog-????????????????.nd: size=122944, sha256=755976b22b64ab680401b45395953504e64e7fa8c31ac570f58dee21e15f9bc0 (glob)
+#endif
diff --git a/mercurial/revlogutils/nodemap.py b/mercurial/revlogutils/nodemap.py
--- a/mercurial/revlogutils/nodemap.py
+++ b/mercurial/revlogutils/nodemap.py
@@ -8,6 +8,7 @@ 
 
 from __future__ import absolute_import
 
+import errno
 import os
 import re
 import struct
@@ -45,11 +46,18 @@ 
     docket.data_unused = data_unused
 
     filename = _rawdata_filepath(revlog, docket)
-    data = revlog.opener.tryread(filename)
+    use_mmap = revlog.opener.options.get("exp-persistent-nodemap.mmap")
+    try:
+        with revlog.opener(filename) as fd:
+            if use_mmap:
+                data = util.buffer(util.mmapread(fd, data_length))
+            else:
+                data = fd.read(data_length)
+    except OSError as e:
+        if e.errno != errno.ENOENT:
+            raise
     if len(data) < data_length:
         return None
-    elif len(data) > data_length:
-        data = data[:data_length]
     return docket, data
 
 
@@ -81,6 +89,8 @@ 
 
     can_incremental = util.safehasattr(revlog.index, "nodemap_data_incremental")
     ondisk_docket = revlog._nodemap_docket
+    feed_data = util.safehasattr(revlog.index, "update_nodemap_data")
+    use_mmap = revlog.opener.options.get("exp-persistent-nodemap.mmap")
 
     data = None
     # first attemp an incremental update of the data
@@ -97,12 +107,18 @@ 
             datafile = _rawdata_filepath(revlog, target_docket)
             # EXP-TODO: if this is a cache, this should use a cache vfs, not a
             # store vfs
+            new_length = target_docket.data_length + len(data)
             with revlog.opener(datafile, b'r+') as fd:
                 fd.seek(target_docket.data_length)
                 fd.write(data)
-                fd.seek(0)
-                new_data = fd.read(target_docket.data_length + len(data))
-            target_docket.data_length += len(data)
+                if feed_data:
+                    if use_mmap:
+                        fd.seek(0)
+                        new_data = fd.read(new_length)
+                    else:
+                        fd.flush()
+                        new_data = util.buffer(util.mmapread(fd, new_length))
+            target_docket.data_length = new_length
             target_docket.data_unused += data_changed_count
 
     if data is None:
@@ -115,9 +131,14 @@ 
             data = persistent_data(revlog.index)
         # EXP-TODO: if this is a cache, this should use a cache vfs, not a
         # store vfs
-        new_data = data
-        with revlog.opener(datafile, b'w') as fd:
+        with revlog.opener(datafile, b'w+') as fd:
             fd.write(data)
+            if feed_data:
+                if use_mmap:
+                    new_data = data
+                else:
+                    fd.flush()
+                    new_data = util.buffer(util.mmapread(fd, len(data)))
         target_docket.data_length = len(data)
     target_docket.tip_rev = revlog.tiprev()
     # EXP-TODO: if this is a cache, this should use a cache vfs, not a
@@ -125,7 +146,7 @@ 
     with revlog.opener(revlog.nodemap_file, b'w', atomictemp=True) as fp:
         fp.write(target_docket.serialize())
     revlog._nodemap_docket = target_docket
-    if util.safehasattr(revlog.index, "update_nodemap_data"):
+    if feed_data:
         revlog.index.update_nodemap_data(target_docket, new_data)
 
     # EXP-TODO: if the transaction abort, we should remove the new data and
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -545,9 +545,6 @@ 
         indexdata = b''
         self._initempty = True
         try:
-            nodemap_data = nodemaputil.persisted_data(self)
-            if nodemap_data is not None:
-                self._nodemap_docket = nodemap_data[0]
             with self._indexfp() as f:
                 if (
                     mmapindexthreshold is not None
@@ -639,6 +636,7 @@ 
             if use_nodemap:
                 nodemap_data = nodemaputil.persisted_data(self)
                 if nodemap_data is not None:
+                    self._nodemap_docket = nodemap_data[0]
                     index.update_nodemap_data(*nodemap_data)
         except (ValueError, IndexError):
             raise error.RevlogError(
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -934,6 +934,8 @@ 
         options[b'rust.index'] = True
     if ui.configbool(b'experimental', b'exp-persistent-nodemap'):
         options[b'exp-persistent-nodemap'] = True
+    if ui.configbool(b'experimental', b'exp-persistent-nodemap.mmap'):
+        options[b'exp-persistent-nodemap.mmap'] = True
     if ui.configbool(b'devel', b'persistent-nodemap'):
         options[b'devel-force-nodemap'] = True
 
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -2122,7 +2122,7 @@ 
         nm_data = nodemap.persisted_data(cl)
         if nm_data is not None:
             docket, data = nm_data
-            ui.write(data)
+            ui.write(data[:])
     elif opts['check']:
         unfi = repo.unfiltered()
         cl = unfi.changelog
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -666,6 +666,9 @@ 
     b'experimental', b'exp-persistent-nodemap', default=False,
 )
 coreconfigitem(
+    b'experimental', b'exp-persistent-nodemap.mmap', default=True,
+)
+coreconfigitem(
     b'experimental', b'server.filesdata.recommended-batch-size', default=50000,
 )
 coreconfigitem(