Patchwork D1622: transaction: Use intbitset for implementing changes['phase']

login
register
mail settings
Submitter phabricator
Date Dec. 9, 2017, 12:06 a.m.
Message ID <differential-rev-PHID-DREV-d7jvmw4sjroqa6plga3l-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26132/
State New
Headers show

Comments

phabricator - Dec. 9, 2017, 12:06 a.m.
joerg.sonnenberger created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/localrepo.py

CHANGE DETAILS




To: joerg.sonnenberger, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 9, 2017, 12:31 a.m.
quark added a comment.


  Although this is definitely better for the clone case. intbitset does not seem to implement segmented bitsets so the memory usage could in theory be worse for other cases.

INLINE COMMENTS

> localrepo.py:1283
>          tr.changes['obsmarkers'] = set()
> -        tr.changes['phases'] = [set() for i in range(7)]
> +        tr.changes['phases'] = [intbitset() for i in range(6)]
>          tr.changes['bookmarks'] = {}

It's still worthwhile to have `set()` as a fallback. So the code runs in pure python or pypy environment

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers
Cc: quark, mercurial-devel
phabricator - Dec. 9, 2017, 1:18 p.m.
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.


  > Although this is definitely better for the clone case. intbitset does not seem to implement segmented bitsets so the memory usage could in theory be worse for other cases.
  
  Perhaps. And if we need to iterate a sparse bitset, the performance would
  be way worse.

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers, yuja
Cc: yuja, quark, mercurial-devel
phabricator - Dec. 9, 2017, 3:15 p.m.
joerg.sonnenberger added a comment.


  Like I said, this is primarily a proof of concept. I'm still playing with different approaches and I don't think intbitset is a good fit for other reasons, but it worked the best from the various pre-existing implementations.

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers, yuja
Cc: yuja, quark, mercurial-devel
phabricator - Dec. 10, 2017, 11:08 a.m.
yuja added a comment.


  In https://phab.mercurial-scm.org/D1622#27981, @joerg.sonnenberger wrote:
  
  > Like I said, this is primarily a proof of concept. I'm still playing with different approaches
  
  
  Ah, okay. I've added [PoC] to the subject so this won't be queued by mistake.
  
  FWIW, we generally don't move codes to C unless there is significant improvement.

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers, yuja
Cc: yuja, quark, mercurial-devel

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -15,6 +15,8 @@ 
 import time
 import weakref
 
+from intbitset import intbitset
+
 from .i18n import _
 from .node import (
     hex,
@@ -1278,7 +1280,7 @@ 
                                      checkambigfiles=_cachedfiles)
         tr.changes['revs'] = xrange(0, 0)
         tr.changes['obsmarkers'] = set()
-        tr.changes['phases'] = [set() for i in range(7)]
+        tr.changes['phases'] = [intbitset() for i in range(6)]
         tr.changes['bookmarks'] = {}
 
         tr.hookargs['txnid'] = txnid