( )⚙ D8815 dirstate: isolate node len dependency for the pure version

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8815

dirstate: isolate node len dependency for the pure version
ClosedPublic

Authored by joerg.sonnenberger on Jul 24 2020, 10:29 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG4f0e03d980f3: dirstate: isolate node len dependency for the pure version
Summary

When switching to a 256bit hash function, this still needs adjustment,
but concentrates the change in one place.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit accepted this revision.Jul 25 2020, 5:29 AM
This revision is now accepted and ready to land.Jul 25 2020, 5:29 AM
Alphare added a subscriber: Alphare.Jul 29 2020, 4:21 PM
Alphare added inline comments.
mercurial/dirstate.py
1670

I'm not convinced this change is valid. The heuristic is about dirstate entries which do not contain hashes, only a path and a few integers for size, mtime, state and mode.

1838

I know this was already queued (published?), but this is part of the Rust impl and should not have been touched if the idea is only to fix the pure version, as this breaks the Rust version.

joerg.sonnenberger added inline comments.Jul 29 2020, 7:14 PM
mercurial/dirstate.py
1670

Hm. In that case the whole comment above it wrong.

Revision Contents
Changeset List

PathPackages
Mmercurial/dirstate.py (20 lines)

Diff 22125

mercurial/dirstate.py

denormalized form that they appear as in the dirstate. denormalized form that they appear as in the dirstate.
""" """
def __init__(self, ui, opener, root): def __init__(self, ui, opener, root):
self._ui = ui self._ui = ui
self._opener = opener self._opener = opener
self._root = root self._root = root
self._filename = b'dirstate' self._filename = b'dirstate'
self._nodelen = 20
self._parents = None self._parents = None
self._dirtyparents = False self._dirtyparents = False
# for consistent view between _pl() and _read() invocations # for consistent view between _pl() and _read() invocations
self._pendingmode = None self._pendingmode = None
@propertycache @propertycache
) )
self._pendingmode = mode self._pendingmode = mode
return fp return fp
def parents(self): def parents(self):
if not self._parents: if not self._parents:
try: try:
fp = self._opendirstatefile() fp = self._opendirstatefile()
st = fp.read(40) st = fp.read(2 * self._nodelen)
fp.close() fp.close()
except IOError as err: except IOError as err:
if err.errno != errno.ENOENT: if err.errno != errno.ENOENT:
raise raise
# File doesn't exist, so the current state is empty # File doesn't exist, so the current state is empty
st = b'' st = b''
l = len(st) l = len(st)
if l == 40: if l == self._nodelen * 2:
self._parents = (st[:20], st[20:40]) self._parents = (
st[: self._nodelen],
st[self._nodelen : 2 * self._nodelen],
)
elif l == 0: elif l == 0:
self._parents = (nullid, nullid) self._parents = (nullid, nullid)
else: else:
raise error.Abort( raise error.Abort(
_(b'working directory state appears damaged!') _(b'working directory state appears damaged!')
) )
return self._parents return self._parents
raise raise
return return
if not st: if not st:
return return
if util.safehasattr(parsers, b'dict_new_presized'): if util.safehasattr(parsers, b'dict_new_presized'):
# Make an estimate of the number of files in the dirstate based on # Make an estimate of the number of files in the dirstate based on
# its size. From a linear regression on a set of real-world repos, # its size. From a linear regression on a set of real-world repos,
# all over 10,000 files, the size of a dirstate entry is 85 # all over 10,000 files, the size of a dirstate entry is 2 nodes
# bytes. The cost of resizing is significantly higher than the cost # plus 45 bytes. The cost of resizing is significantly higher than the cost
# of filling in a larger presized dict, so subtract 20% from the # of filling in a larger presized dict, so subtract 20% from the
# size. # size.
# #
# This heuristic is imperfect in many ways, so in a future dirstate # This heuristic is imperfect in many ways, so in a future dirstate
# format update it makes sense to just record the number of entries # format update it makes sense to just record the number of entries
# on write. # on write.
self._map = parsers.dict_new_presized(len(st) // 71) self._map = parsers.dict_new_presized(
AlphareUnsubmitted
Not Done

I'm not convinced this change is valid. The heuristic is about dirstate entries which do not contain hashes, only a path and a few integers for size, mtime, state and mode.

Alphare: I'm not convinced this change is valid. The heuristic is about dirstate entries which do not…
joerg.sonnenbergerAuthorUnsubmitted
Not Done

Hm. In that case the whole comment above it wrong.

joerg.sonnenberger: Hm. In that case the whole comment above it wrong.
len(st) // ((2 * self._nodelen + 45) * 4 // 5)
)
# Python's garbage collector triggers a GC each time a certain number # Python's garbage collector triggers a GC each time a certain number
# of container objects (the number being defined by # of container objects (the number being defined by
# gc.get_threshold()) are allocated. parse_dirstate creates a tuple # gc.get_threshold()) are allocated. parse_dirstate creates a tuple
# for each file in the dirstate. The C version then immediately marks # for each file in the dirstate. The C version then immediately marks
# them as not to be tracked by the collector. However, this has no # them as not to be tracked by the collector. However, this has no
# effect on when GCs are triggered, only on what objects the GC looks # effect on when GCs are triggered, only on what objects the GC looks
# into. This means that O(number of files) GCs are unavoidable. # into. This means that O(number of files) GCs are unavoidable.
self._rustmap.setparents(p1, p2) self._rustmap.setparents(p1, p2)
self._parents = (p1, p2) self._parents = (p1, p2)
self._dirtyparents = True self._dirtyparents = True
def parents(self): def parents(self):
if not self._parents: if not self._parents:
try: try:
fp = self._opendirstatefile() fp = self._opendirstatefile()
st = fp.read(40) st = fp.read(2 * self._nodelen)
AlphareUnsubmitted
Not Done

I know this was already queued (published?), but this is part of the Rust impl and should not have been touched if the idea is only to fix the pure version, as this breaks the Rust version.

Alphare: I know this was already queued (published?), but this is part of the Rust impl and should not…
fp.close() fp.close()
except IOError as err: except IOError as err:
if err.errno != errno.ENOENT: if err.errno != errno.ENOENT:
raise raise
# File doesn't exist, so the current state is empty # File doesn't exist, so the current state is empty
st = b'' st = b''
try: try: