This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7313

revlog: move the nodemap into the index object (for pure)
ClosedPublic

Authored by marmoute on Nov 8 2019, 4:28 AM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHG845e5b313783: revlog: move the nodemap into the index object (for pure)
rHGcf5985770fdb: revlog: move the nodemap into the index object (for pure)
Summary

This make the pure code closer to the C extension one. The ultimate goal is to
merge the two into a single object and offer a unified API. This changeset
focus on gathering the data on the same object.

For now the code for revlogoldindex and BaseIndexObject index object are
quite similar. However, there will be larger divergence later on, so I don't
think is worth doing a base case.

This work is part of a refactoring to unify the revlog index and the nodemap.
This unification prepare the use of a persistent nodemap.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

marmoute created this revision.Nov 8 2019, 4:28 AM
indygreg added a comment.Nov 8 2019, 2:30 PM

This seems OK to me aside from the use of range(), which we don't want since len() can be massive and this could create perf problems on Python 2.

mercurial/pure/parsers.py
53

We want this to be pycompat.xrange.

mercurial/revlog.py
211

pycompat.xrange.

marmoute added a comment.Nov 8 2019, 7:22 PM
In D7313#107559, @indygreg wrote:

This seems OK to me aside from the use of range(), which we don't want since len() can be massive and this could create perf problems on Python 2.

I am not too convinced by a switch to range(). I've two main reason:

  1. this is the "pure" version, so people are unlikely to use it on large repository, they would get in other large performance issue before that.
  2. we are about to allocate a full nodemap, this will be significantly slower than the associated range() call.

Do you still cant the pycompat.xrange change ?

marmoute added a commit: rHGcf5985770fdb: revlog: move the nodemap into the index object (for pure).Nov 8 2019, 8:53 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGcf5985770fdb: revlog: move the nodemap into the index object (for pure) (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Nov 10 2019, 4:36 AM

@@ -662,29 +665,10 @@

except TypeError:
    raise
except error.RevlogError:
  • if not isinstance(self._nodecache, revlogutils.NodeMap):
  • # parsers.c radix tree lookup failed
  • if node == wdirid or node in wdirfilenodeids:
  • raise error.WdirUnsupported
  • raise error.LookupError(node, self.indexfile, _(b'no node'))
  • else:
  • # pure python cache lookup failed
  • n = self._nodecache
  • i = self.index
  • p = self._nodepos
  • if p is None:
  • p = len(i) - 1
  • else:
  • assert p < len(i)
  • for r in pycompat.xrange(p, -1, -1):
  • v = i[r][7]
  • n[v] = r
  • if v == node:
  • self._nodepos = r - 1
  • return r
  • if node == wdirid or node in wdirfilenodeids:
  • raise error.WdirUnsupported
  • raise error.LookupError(node, self.indexfile, _(b'no node'))

+ # parsers.c radix tree lookup failed
+ if node == wdirid or node in wdirfilenodeids:
+ raise error.WdirUnsupported
+ raise error.LookupError(node, self.indexfile, _(b'no node'))

So revlog._nodepos is now useless since the node map is no longer built
incrementally.

Revision Contents
Changeset List

PathPackages
Mmercurial/pure/parsers.py (19 lines)
Mmercurial/revlog.py (60 lines)

Diff 17729

mercurial/pure/parsers.py

# parsers.py - Python implementation of parsers.c # parsers.py - Python implementation of parsers.c
# #
# Copyright 2009 Matt Mackall <mpm@selenic.com> and others # Copyright 2009 Matt Mackall <mpm@selenic.com> and others
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
import struct import struct
import zlib import zlib
from ..node import nullid from ..node import nullid, nullrev
from .. import pycompat from .. import (
pycompat,
revlogutils,
util,
)
stringio = pycompat.bytesio stringio = pycompat.bytesio
_pack = struct.pack _pack = struct.pack
_unpack = struct.unpack _unpack = struct.unpack
_compress = zlib.compress _compress = zlib.compress
_decompress = zlib.decompress _decompress = zlib.decompress
return int(q & 0xFFFF) return int(q & 0xFFFF)
def offset_type(offset, type): def offset_type(offset, type):
return int(int(offset) << 16 | type) return int(int(offset) << 16 | type)
class BaseIndexObject(object): class BaseIndexObject(object):
@util.propertycache
def nodemap(self):
nodemap = revlogutils.NodeMap({nullid: nullrev})
for r in range(0, len(self)):
indygregUnsubmitted
Not Done

We want this to be pycompat.xrange.

indygreg: We want this to be `pycompat.xrange`.
n = self[r][7]
nodemap[n] = r
return nodemap
def clearcaches(self):
self.__dict__.pop('nodemap', None)
def __len__(self): def __len__(self):
return self._lgt + len(self._extra) return self._lgt + len(self._extra)
def append(self, tup): def append(self, tup):
self._extra.append(tup) self._extra.append(tup)
def _check_index(self, i): def _check_index(self, i):
if not isinstance(i, int): if not isinstance(i, int):

mercurial/revlog.py

# 20 bytes: parent 2 nodeid # 20 bytes: parent 2 nodeid
# 20 bytes: nodeid # 20 bytes: nodeid
indexformatv0 = struct.Struct(b">4l20s20s20s") indexformatv0 = struct.Struct(b">4l20s20s20s")
indexformatv0_pack = indexformatv0.pack indexformatv0_pack = indexformatv0.pack
indexformatv0_unpack = indexformatv0.unpack indexformatv0_unpack = indexformatv0.unpack
class revlogoldindex(list): class revlogoldindex(list):
@util.propertycache
def nodemap(self):
nodemap = revlogutils.NodeMap({nullid: nullrev})
for r in range(0, len(self)):
indygregUnsubmitted
Not Done

pycompat.xrange.

indygreg: `pycompat.xrange`.
n = self[r][7]
nodemap[n] = r
return nodemap
def clearcaches(self):
self.__dict__.pop('nodemap', None)
def __getitem__(self, i): def __getitem__(self, i):
if i == -1: if i == -1:
return (0, 0, 0, -1, -1, -1, -1, nullid) return (0, 0, 0, -1, -1, -1, -1, nullid)
return list.__getitem__(self, i) return list.__getitem__(self, i)
class revlogoldio(object): class revlogoldio(object):
def __init__(self): def __init__(self):
nodemap.get(e[4], nullrev), nodemap.get(e[4], nullrev),
nodemap.get(e[5], nullrev), nodemap.get(e[5], nullrev),
e[6], e[6],
) )
index.append(e2) index.append(e2)
nodemap[e[6]] = n nodemap[e[6]] = n
n += 1 n += 1
return revlogoldindex(index), nodemap, None index = revlogoldindex(index)
return index, index.nodemap, None
def packentry(self, entry, node, version, rev): def packentry(self, entry, node, version, rev):
if gettype(entry[0]): if gettype(entry[0]):
raise error.RevlogError( raise error.RevlogError(
_(b'index entry flags need revlog version 1') _(b'index entry flags need revlog version 1')
) )
e2 = ( e2 = (
getoffset(entry[0]), getoffset(entry[0]),
class revlogio(object): class revlogio(object):
def __init__(self): def __init__(self):
self.size = indexformatng.size self.size = indexformatng.size
def parseindex(self, data, inline): def parseindex(self, data, inline):
# call the C implementation to parse the index data # call the C implementation to parse the index data
index, cache = parsers.parse_index2(data, inline) index, cache = parsers.parse_index2(data, inline)
return index, getattr(index, 'nodemap', None), cache return index, index.nodemap, cache
def packentry(self, entry, node, version, rev): def packentry(self, entry, node, version, rev):
p = indexformatng_pack(*entry) p = indexformatng_pack(*entry)
if rev == 0: if rev == 0:
p = versionformat_pack(version) + p[4:] p = versionformat_pack(version) + p[4:]
return p return p
# Maps rev to chain base rev. # Maps rev to chain base rev.
self._chainbasecache = util.lrucachedict(100) self._chainbasecache = util.lrucachedict(100)
# 2-tuple of (offset, data) of raw data from the revlog at an offset. # 2-tuple of (offset, data) of raw data from the revlog at an offset.
self._chunkcache = (0, b'') self._chunkcache = (0, b'')
# How much data to read and cache into the raw revlog data cache. # How much data to read and cache into the raw revlog data cache.
self._chunkcachesize = 65536 self._chunkcachesize = 65536
self._maxchainlen = None self._maxchainlen = None
self._deltabothparents = True self._deltabothparents = True
self.index = [] self.index = None
# Mapping of partial identifiers to full nodes. # Mapping of partial identifiers to full nodes.
self._pcache = {} self._pcache = {}
# Mapping of revision integer to full node. # Mapping of revision integer to full node.
self._nodecache = revlogutils.NodeMap({nullid: nullrev}) self._nodecache = None
self._nodepos = None self._nodepos = None
self._compengine = b'zlib' self._compengine = b'zlib'
self._compengineopts = {} self._compengineopts = {}
self._maxdeltachainspan = -1 self._maxdeltachainspan = -1
self._withsparseread = False self._withsparseread = False
self._sparserevlog = False self._sparserevlog = False
self._srdensitythreshold = 0.50 self._srdensitythreshold = 0.50
self._srmingapsize = 262144 self._srmingapsize = 262144
self._io = revlogoldio() self._io = revlogoldio()
try: try:
d = self._io.parseindex(indexdata, self._inline) d = self._io.parseindex(indexdata, self._inline)
except (ValueError, IndexError): except (ValueError, IndexError):
raise error.RevlogError( raise error.RevlogError(
_(b"index %s is corrupted") % self.indexfile _(b"index %s is corrupted") % self.indexfile
) )
self.index, nodemap, self._chunkcache = d self.index, nodemap, self._chunkcache = d
if nodemap is not None:
self.nodemap = self._nodecache = nodemap self.nodemap = self._nodecache = nodemap
if not self._chunkcache: if not self._chunkcache:
self._chunkclear() self._chunkclear()
# revnum -> (chain-length, sum-delta-length) # revnum -> (chain-length, sum-delta-length)
self._chaininfocache = {} self._chaininfocache = {}
# revlog header -> revlog compressor # revlog header -> revlog compressor
self._decompressors = {} self._decompressors = {}
@util.propertycache @util.propertycache
return False return False
return True return True
def clearcaches(self): def clearcaches(self):
self._revisioncache = None self._revisioncache = None
self._chainbasecache.clear() self._chainbasecache.clear()
self._chunkcache = (0, b'') self._chunkcache = (0, b'')
self._pcache = {} self._pcache = {}
self.index.clearcaches()
try:
# If we are using the native C version, you are in a fun case
# where self.index, self.nodemap and self._nodecaches is the same
# object.
self._nodecache.clearcaches()
except AttributeError:
self._nodecache = revlogutils.NodeMap({nullid: nullrev})
self._nodepos = None
def rev(self, node): def rev(self, node):
try: try:
return self._nodecache[node] return self._nodecache[node]
except TypeError: except TypeError:
raise raise
except error.RevlogError: except error.RevlogError:
if not isinstance(self._nodecache, revlogutils.NodeMap):
# parsers.c radix tree lookup failed # parsers.c radix tree lookup failed
if node == wdirid or node in wdirfilenodeids: if node == wdirid or node in wdirfilenodeids:
raise error.WdirUnsupported raise error.WdirUnsupported
raise error.LookupError(node, self.indexfile, _(b'no node')) raise error.LookupError(node, self.indexfile, _(b'no node'))
else:
# pure python cache lookup failed
n = self._nodecache
i = self.index
p = self._nodepos
if p is None:
p = len(i) - 1
else:
assert p < len(i)
for r in pycompat.xrange(p, -1, -1):
v = i[r][7]
n[v] = r
if v == node:
self._nodepos = r - 1
return r
if node == wdirid or node in wdirfilenodeids:
raise error.WdirUnsupported
raise error.LookupError(node, self.indexfile, _(b'no node'))
# Accessors for index entries. # Accessors for index entries.
# First tuple entry is 8 bytes. First 6 bytes are offset. Last 2 bytes # First tuple entry is 8 bytes. First 6 bytes are offset. Last 2 bytes
# are flags. # are flags.
def start(self, rev): def start(self, rev):
return int(self.index[rev][0] >> 16) return int(self.index[rev][0] >> 16)