This is an archive of the discontinued Mercurial Phabricator instance.

Differential D32

revlog: use struct.Struct instances for slight performance wins
ClosedPublic

Authored by alex_gaynor on Jul 10 2017, 4:59 PM.

Details

Reviewers
durin42
Commits
rHGac6446611ad2: revlog: use struct.Struct instances for slight performance wins
Summary

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

alex_gaynor created this revision.Jul 10 2017, 4:59 PM
dsp added a subscriber: dsp.Jul 11 2017, 11:27 AM
dsp added inline comments.
mercurial/revlog.py
189

is there a reason to rename this to indexformatv0_unpack and not call it indexformatv0.unpack? If there is no particular reason, it feels to me that indexformatv0.unpack and indexformatv0.pack is more consistent with the use of indexformatv0.size.

alex_gaynor added inline comments.Jul 11 2017, 11:29 AM
mercurial/revlog.py
189

Preserve the micro-optimization that this line performs no attribute lookups, just a single global lookup. (It's probably measurable on CPython, though not on PyPy).

dsp added a comment.Jul 11 2017, 11:33 AM

LGTM

Thanks for the explanation. Can we add this reasoning to the commit message maybe and clearly describe what the microoptimization is?

quark retitled this revision from revlog: Use struct.Struct instances for slight performance wins to revlog: use struct.Struct instances for slight performance wins.Jul 11 2017, 12:12 PM
durin42 accepted this revision.Jul 14 2017, 11:55 AM

(This is already pushed)

This revision is now accepted and ready to land.Jul 14 2017, 11:55 AM
Closed by commit rHGac6446611ad2: revlog: use struct.Struct instances for slight performance wins (authored by alex_gaynor). · Explain WhyJul 14 2017, 12:33 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/revlog.py (27 lines)

Diff 133

mercurial/revlog.py

policy, policy,
pycompat, pycompat,
templatefilters, templatefilters,
util, util,
) )
parsers = policy.importmod(r'parsers') parsers = policy.importmod(r'parsers')
_pack = struct.pack
_unpack = struct.unpack
# Aliased for performance. # Aliased for performance.
_zlibdecompress = zlib.decompress _zlibdecompress = zlib.decompress
# revlog header flags # revlog header flags
REVLOGV0 = 0 REVLOGV0 = 0
REVLOGV1 = 1 REVLOGV1 = 1
# Dummy value until file format is finalized. # Dummy value until file format is finalized.
# Reminder: change the bounds check in revlog.__init__ when this is changed. # Reminder: change the bounds check in revlog.__init__ when this is changed.
# index v0: # index v0:
# 4 bytes: offset # 4 bytes: offset
# 4 bytes: compressed length # 4 bytes: compressed length
# 4 bytes: base rev # 4 bytes: base rev
# 4 bytes: link rev # 4 bytes: link rev
# 20 bytes: parent 1 nodeid # 20 bytes: parent 1 nodeid
# 20 bytes: parent 2 nodeid # 20 bytes: parent 2 nodeid
# 20 bytes: nodeid # 20 bytes: nodeid
indexformatv0 = ">4l20s20s20s" indexformatv0 = struct.Struct(">4l20s20s20s")
indexformatv0_pack = indexformatv0.pack
indexformatv0_unpack = indexformatv0.unpack
class revlogoldio(object): class revlogoldio(object):
def __init__(self): def __init__(self):
self.size = struct.calcsize(indexformatv0) self.size = indexformatv0.size
def parseindex(self, data, inline): def parseindex(self, data, inline):
s = self.size s = self.size
index = [] index = []
nodemap = {nullid: nullrev} nodemap = {nullid: nullrev}
n = off = 0 n = off = 0
l = len(data) l = len(data)
while off + s <= l: while off + s <= l:
cur = data[off:off + s] cur = data[off:off + s]
off += s off += s
e = _unpack(indexformatv0, cur) e = indexformatv0_unpack(cur)
dspUnsubmitted
Not Done

is there a reason to rename this to indexformatv0_unpack and not call it indexformatv0.unpack? If there is no particular reason, it feels to me that indexformatv0.unpack and indexformatv0.pack is more consistent with the use of indexformatv0.size.

dsp: is there a reason to rename this to `indexformatv0_unpack` and not call it `indexformatv0.
alex_gaynorAuthorUnsubmitted
Not Done

Preserve the micro-optimization that this line performs no attribute lookups, just a single global lookup. (It's probably measurable on CPython, though not on PyPy).

alex_gaynor: Preserve the micro-optimization that this line performs no attribute lookups, just a single…
# transform to revlogv1 format # transform to revlogv1 format
e2 = (offset_type(e[0], 0), e[1], -1, e[2], e[3], e2 = (offset_type(e[0], 0), e[1], -1, e[2], e[3],
nodemap.get(e[4], nullrev), nodemap.get(e[5], nullrev), e[6]) nodemap.get(e[4], nullrev), nodemap.get(e[5], nullrev), e[6])
index.append(e2) index.append(e2)
nodemap[e[6]] = n nodemap[e[6]] = n
n += 1 n += 1
# add the magic null revision at -1 # add the magic null revision at -1
index.append((0, 0, 0, -1, -1, -1, -1, nullid)) index.append((0, 0, 0, -1, -1, -1, -1, nullid))
return index, nodemap, None return 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 RevlogError(_('index entry flags need revlog version 1')) raise RevlogError(_('index entry flags need revlog version 1'))
e2 = (getoffset(entry[0]), entry[1], entry[3], entry[4], e2 = (getoffset(entry[0]), entry[1], entry[3], entry[4],
node(entry[5]), node(entry[6]), entry[7]) node(entry[5]), node(entry[6]), entry[7])
return _pack(indexformatv0, *e2) return indexformatv0_pack(*e2)
# index ng: # index ng:
# 6 bytes: offset # 6 bytes: offset
# 2 bytes: flags # 2 bytes: flags
# 4 bytes: compressed length # 4 bytes: compressed length
# 4 bytes: uncompressed length # 4 bytes: uncompressed length
# 4 bytes: base rev # 4 bytes: base rev
# 4 bytes: link rev # 4 bytes: link rev
# 4 bytes: parent 1 rev # 4 bytes: parent 1 rev
# 4 bytes: parent 2 rev # 4 bytes: parent 2 rev
# 32 bytes: nodeid # 32 bytes: nodeid
indexformatng = ">Qiiiiii20s12x" indexformatng = struct.Struct(">Qiiiiii20s12x")
versionformat = ">I" indexformatng_pack = indexformatng.pack
versionformat = struct.Struct(">I")
versionformat_pack = versionformat.pack
versionformat_unpack = versionformat.unpack
# corresponds to uncompressed length of indexformatng (2 gigs, 4-byte # corresponds to uncompressed length of indexformatng (2 gigs, 4-byte
# signed integer) # signed integer)
_maxentrysize = 0x7fffffff _maxentrysize = 0x7fffffff
class revlogio(object): class revlogio(object):
def __init__(self): def __init__(self):
self.size = struct.calcsize(indexformatng) 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, getattr(index, 'nodemap', None), cache
def packentry(self, entry, node, version, rev): def packentry(self, entry, node, version, rev):
p = _pack(indexformatng, *entry) p = indexformatng_pack(*entry)
if rev == 0: if rev == 0:
p = _pack(versionformat, version) + p[4:] p = versionformat_pack(version) + p[4:]
return p return p
class revlog(object): class revlog(object):
""" """
the underlying revision storage object the underlying revision storage object
A revlog consists of two parts, an index and the revision data. A revlog consists of two parts, an index and the revision data.
indexdata = '' indexdata = ''
self._initempty = True self._initempty = True
try: try:
f = self.opener(self.indexfile) f = self.opener(self.indexfile)
indexdata = f.read() indexdata = f.read()
f.close() f.close()
if len(indexdata) > 0: if len(indexdata) > 0:
v = struct.unpack(versionformat, indexdata[:4])[0] v = versionformat_unpack(indexdata[:4])[0]
self._initempty = False self._initempty = False
except IOError as inst: except IOError as inst:
if inst.errno != errno.ENOENT: if inst.errno != errno.ENOENT:
raise raise
self.version = v self.version = v
self._inline = v & FLAG_INLINE_DATA self._inline = v & FLAG_INLINE_DATA
self._generaldelta = v & FLAG_GENERALDELTA self._generaldelta = v & FLAG_GENERALDELTA