( )⚙ D1394 bundle2: avoid unbound read when seeking

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1394

bundle2: avoid unbound read when seeking
ClosedPublic

Authored by indygreg on Nov 14 2017, 1:26 AM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG699b2a759319: bundle2: avoid unbound read when seeking
Summary

Currently, seekableunbundlepart.seek() will perform a read() during
seek operations. This will allocate a buffer to hold the raw data
over the seek distance. This can lead to very large allocations
and cause performance to suffer.

We change the code to perform read(32768) in a loop to avoid
potentially large allocations.

hg perfbundleread on an uncompressed Firefox bundle reveals
a performance impact:

! bundle2 iterparts()
! wall 2.992605 comb 2.990000 user 2.260000 sys 0.730000 (best of 4)
! bundle2 iterparts() seekable
! wall 3.863810 comb 3.860000 user 3.000000 sys 0.860000 (best of 3)
! bundle2 part seek()
! wall 6.213387 comb 6.200000 user 3.350000 sys 2.850000 (best of 3)
! wall 3.820347 comb 3.810000 user 2.980000 sys 0.830000 (best of 3)

Since seekable bundle parts are (only) used by bundlerepo, this /may/
speed up initial loading of bundle-based repos. But any improvement
will likely only be noticed on very large bundles.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Nov 14 2017, 1:26 AM
durin42 accepted this revision.Nov 20 2017, 6:43 PM
This revision is now accepted and ready to land.Nov 20 2017, 6:43 PM
Closed by commit rHG699b2a759319: bundle2: avoid unbound read when seeking (authored by indygreg). · Explain WhyNov 20 2017, 6:50 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/bundle2.py (11 lines)

Diff 3472

mercurial/bundle2.py

def seek(self, offset, whence=os.SEEK_SET): def seek(self, offset, whence=os.SEEK_SET):
if whence == os.SEEK_SET: if whence == os.SEEK_SET:
newpos = offset newpos = offset
elif whence == os.SEEK_CUR: elif whence == os.SEEK_CUR:
newpos = self._pos + offset newpos = self._pos + offset
elif whence == os.SEEK_END: elif whence == os.SEEK_END:
if not self.consumed: if not self.consumed:
self.read() # Can't use self.consume() here because it advances self._pos.
chunk = self.read(32768)
while chunk:
chunk = self.read(32768)
newpos = self._chunkindex[-1][0] - offset newpos = self._chunkindex[-1][0] - offset
else: else:
raise ValueError('Unknown whence value: %r' % (whence,)) raise ValueError('Unknown whence value: %r' % (whence,))
if newpos > self._chunkindex[-1][0] and not self.consumed: if newpos > self._chunkindex[-1][0] and not self.consumed:
self.read() # Can't use self.consume() here because it advances self._pos.
chunk = self.read(32768)
while chunk:
chunk = self.read(32668)
if not 0 <= newpos <= self._chunkindex[-1][0]: if not 0 <= newpos <= self._chunkindex[-1][0]:
raise ValueError('Offset out of range') raise ValueError('Offset out of range')
if self._pos != newpos: if self._pos != newpos:
chunk, internaloffset = self._findchunk(newpos) chunk, internaloffset = self._findchunk(newpos)
self._payloadstream = util.chunkbuffer(self._payloadchunks(chunk)) self._payloadstream = util.chunkbuffer(self._payloadchunks(chunk))
adjust = self.read(internaloffset) adjust = self.read(internaloffset)
if len(adjust) != internaloffset: if len(adjust) != internaloffset: