Patchwork D9673: engine: prevent a function call for each store file

login
register
mail settings
Submitter phabricator
Date Dec. 31, 2020, 4:42 p.m.
Message ID <differential-rev-PHID-DREV-lsl2s5ylwt2ru7astuog-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/47984/
State Superseded
Headers show

Comments

phabricator - Dec. 31, 2020, 4:42 p.m.
pulkit created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Python function calls are not cheap. Instead of calling the function once for
  each file in store, we use a dedicated function which filters and yields the
  required files.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/upgrade_utils/engine.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/mercurial/upgrade_utils/engine.py b/mercurial/upgrade_utils/engine.py
--- a/mercurial/upgrade_utils/engine.py
+++ b/mercurial/upgrade_utils/engine.py
@@ -375,37 +375,28 @@ 
         % (revcount, util.bytecount(dstsize - srcsize))
     )
 
-
-def _filterstorefile(srcrepo, dstrepo, requirements, path, mode, st):
-    """Determine whether to copy a store file during upgrade.
-
-    This function is called when migrating store files from ``srcrepo`` to
-    ``dstrepo`` as part of upgrading a repository.
+    return True
 
-    Args:
-      srcrepo: repo we are copying from
-      dstrepo: repo we are copying to
-      requirements: set of requirements for ``dstrepo``
-      path: store file being examined
-      mode: the ``ST_MODE`` file type of ``path``
-      st: ``stat`` data structure for ``path``
 
-    Function should return ``True`` if the file is to be copied.
-    """
-    # Skip revlogs.
-    if path.endswith((b'.i', b'.d', b'.n', b'.nd')):
-        return False
-    # Skip transaction related files.
-    if path.startswith(b'undo'):
-        return False
-    # Only copy regular files.
-    if mode != stat.S_IFREG:
-        return False
-    # Skip other skipped files.
-    if path in (b'lock', b'fncache'):
-        return False
+def _files_to_copy_post_revlog_clone(srcrepo):
+    """yields files which should be copied to destination after revlogs
+    are cloned"""
+    for path, kind, st in sorted(srcrepo.store.vfs.readdir(b'', stat=True)):
+        # don't copy revlogs as they are already cloned
+        if path.endswith((b'.i', b'.d', b'.n', b'.nd')):
+            continue
+        # Skip transaction related files.
+        if path.startswith(b'undo'):
+            continue
+        # Only copy regular files.
+        if kind != stat.S_IFREG:
+            continue
+        # Skip other skipped files.
+        if path in (b'lock', b'fncache'):
+            continue
+        # TODO: should we skip cache too?
 
-    return True
+        yield path
 
 
 def finishdatamigration(ui, srcrepo, dstrepo, requirements):
@@ -446,13 +437,7 @@ 
         )
 
     # Now copy other files in the store directory.
-    # The sorted() makes execution deterministic.
-    for p, kind, st in sorted(srcrepo.store.vfs.readdir(b'', stat=True)):
-        if not _filterstorefile(
-            srcrepo, dstrepo, upgrade_op.new_requirements, p, kind, st
-        ):
-            continue
-
+    for p in _files_to_copy_post_revlog_clone(srcrepo):
         srcrepo.ui.status(_(b'copying %s\n') % p)
         src = srcrepo.store.rawvfs.join(p)
         dst = dstrepo.store.rawvfs.join(p)