This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10785

revlog: use dedicated code for reading sidedata
ClosedPublic

Authored by marmoute on May 28 2021, 6:56 PM.

Details

Reviewers
indygreg
Alphare
Group Reviewers
hg-reviewers
Commits
rHG75e1104f23a2: revlog: use dedicated code for reading sidedata
Summary

We are about to introduce a new, dedicated, file to store sidedata. Before doing so, we make sidedata reading go through different code as reading data chunk. This will simplify some of the complexity of the next changesets.

The reading is very simple right now and will need some improvement later to
reuse some of the caching strategy we use for the data file.

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

marmoute created this revision.May 28 2021, 6:56 PM
Alphare accepted this revision as: Alphare.May 31 2021, 6:23 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
mercurial/revlog.py
2081

Since this is an error case that isn't supposed to happen often, maybe we could factor this in a function?

baymax updated this revision to Diff 28298.May 31 2021, 6:37 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

marmoute added inline comments.May 31 2021, 9:25 AM
mercurial/revlog.py
2081

I am not sure we will gain much as most of the block size is about passing parameter to the formatting.

baymax updated this revision to Diff 28395.Jun 1 2021, 11:10 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 28444.Jun 4 2021, 9:54 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

marmoute added a commit: rHG75e1104f23a2: revlog: use dedicated code for reading sidedata.Jun 7 2021, 5:22 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG75e1104f23a2: revlog: use dedicated code for reading sidedata (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/configitems.py (1 line)
Mmercurial/revlog.py (18 lines)

Diff 28488

mercurial/configitems.py

# - for stripping operation # - for stripping operation
# - for rollback operation # - for rollback operation
# * proper streaming (race free) of the docket file # * proper streaming (race free) of the docket file
# * track garbage data to evemtually allow rewriting -existing- sidedata. # * track garbage data to evemtually allow rewriting -existing- sidedata.
# * Exchange-wise, we will also need to do something more efficient than # * Exchange-wise, we will also need to do something more efficient than
# keeping references to the affected revlogs, especially memory-wise when # keeping references to the affected revlogs, especially memory-wise when
# rewriting sidedata. # rewriting sidedata.
# * introduce a proper solution to reduce the number of filelog related files. # * introduce a proper solution to reduce the number of filelog related files.
# * use caching for reading sidedata (similar to what we do for data).
# * Improvement to consider # * Improvement to consider
# - avoid compression header in chunk using the default compression? # - avoid compression header in chunk using the default compression?
# - forbid "inline" compression mode entirely? # - forbid "inline" compression mode entirely?
# - split the data offset and flag field (the 2 bytes save are mostly trouble) # - split the data offset and flag field (the 2 bytes save are mostly trouble)
# - keep track of uncompressed -chunk- size (to preallocate memory better) # - keep track of uncompressed -chunk- size (to preallocate memory better)
# - keep track of chain base or size (probably not that useful anymore) # - keep track of chain base or size (probably not that useful anymore)
# - store data and sidedata in different files # - store data and sidedata in different files
coreconfigitem( coreconfigitem(

mercurial/revlog.py

else: else:
if self._inline: if self._inline:
func = self._indexfp func = self._indexfp
else: else:
func = self._datafp func = self._datafp
with func() as fp: with func() as fp:
yield fp yield fp
def _sidedatareadfp(self):
"""file object suitable to read sidedata"""
return self._datareadfp()
def tiprev(self): def tiprev(self):
return len(self.index) - 1 return len(self.index) - 1
def tip(self): def tip(self):
return self.node(self.tiprev()) return self.node(self.tiprev())
def __contains__(self, rev): def __contains__(self, rev):
return 0 <= rev < len(self) return 0 <= rev < len(self)
sidedata_offset = index_entry[8] sidedata_offset = index_entry[8]
sidedata_size = index_entry[9] sidedata_size = index_entry[9]
if self._inline: if self._inline:
sidedata_offset += self.index.entry_size * (1 + rev) sidedata_offset += self.index.entry_size * (1 + rev)
if sidedata_size == 0: if sidedata_size == 0:
return {} return {}
comp_segment = self._getsegment(sidedata_offset, sidedata_size) # XXX this need caching, as we do for data
with self._sidedatareadfp() as sdf:
sdf.seek(sidedata_offset)
comp_segment = sdf.read(sidedata_size)
if len(comp_segment) < sidedata_size:
filename = self._datafile
AlphareUnsubmitted
Not Done

Since this is an error case that isn't supposed to happen often, maybe we could factor this in a function?

Alphare: Since this is an error case that isn't supposed to happen often, maybe we could factor this in…
marmouteAuthorUnsubmitted
Done

I am not sure we will gain much as most of the block size is about passing parameter to the formatting.

marmoute: I am not sure we will gain much as most of the block size is about passing parameter to the…
length = sidedata_size
offset = sidedata_offset
got = len(comp_segment)
m = PARTIAL_READ_MSG % (filename, length, offset, got)
raise error.RevlogError(m)
comp = self.index[rev][11] comp = self.index[rev][11]
if comp == COMP_MODE_PLAIN: if comp == COMP_MODE_PLAIN:
segment = comp_segment segment = comp_segment
elif comp == COMP_MODE_DEFAULT: elif comp == COMP_MODE_DEFAULT:
segment = self._decompressor(comp_segment) segment = self._decompressor(comp_segment)
elif comp == COMP_MODE_INLINE: elif comp == COMP_MODE_INLINE:
segment = self.decompress(comp_segment) segment = self.decompress(comp_segment)
else: else: