revlog: use dedicated code for reading sidedata
ClosedPublic

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

Download Raw Diff

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

Herald added a reviewer: indygreg. · View Herald TranscriptMay 28 2021, 6:56 PM

Herald added a reviewer: hg-reviewers. · View Herald Transcript

Herald added a subscriber: mercurial-patches. · View Herald Transcript

marmoute added a child revision: D10786: revlog: fix docket.date_filepath docstring.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?

✅ 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.

✅ 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

			Path	Packages
M			mercurial/configitems.py (1 line)
M			mercurial/revlog.py (18 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	28245		May 28 2021, 6:56 PM	★	★
Diff 2	28298		May 31 2021, 6:37 AM	★	★
Diff 3	28395		Jun 1 2021, 11:10 PM	★	★
Diff 4	28444		Jun 4 2021, 9:54 AM	★	★
Diff 5	28488	rHG75e1104f23a2c455359b7aec5cb6636c32c94e96	May 26 2021, 10:09 PM	★	★

Status	Author	Revision
Closed	marmoute	D10800 revlog: allow to pass an existing docket to `_loadindex()`
Closed	marmoute	D10799 revlog: add a ways to blacklist some revision when searching for a delta
Closed	marmoute	D10798 util: add `nb_bytes` argument to `copyfile` to partially copy a file
Closed	marmoute	D10797 revlog: move `revisioninfo` in `revlogutils`
Closed	marmoute	D10796 revlog: use the `entry` function in bundlerepo
Closed	marmoute	D10795 revlog: use entry in revlogv0.py
Closed	marmoute	D10794 revlog: use the new `entry` function in revlog.py
Closed	marmoute	D10793 revlog: add a function to build index entry tuple
Closed	marmoute	D10792 revlog: move `offset_type` to `revlogutils`
Closed	marmoute	D10791 revlog: move entry documentation alongside new related constants
Closed	marmoute	D10790 censor: drop size limitation on the tombstone
Closed	marmoute	D10789 revlog: move censoring code in a dedicated module
Closed	marmoute	D10788 censor: reduce risk of censor test blowing up output limit
Closed	marmoute	D10787 revlog: store sidedata in their own file
Closed	marmoute	D10786 revlog: fix docket.date_filepath docstring
Closed	marmoute	D10785 revlog: use dedicated code for reading sidedata
Closed	marmoute	D10784 revlog: simplify "partial read" error message
Closed	marmoute	D10783 revlog: no longer return sidedata from `_revisiondata`
Closed	marmoute	D10782 revlog: use `self.sidedata` directly in `revlog.clone`
Closed	marmoute	D10781 revlog: use `self.sidedata` directly to construct changelogrevision
Closed	marmoute	D10780 revlog: implement sidedata without using _revisiondata
Closed	marmoute	D10779 revlogv2: drop "sidedatacompression" from the list of todos
Closed	marmoute	D10831 index: use an explicit constant for INDEX_HEADER format and use it for docket

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:

This is an archive of the discontinued Mercurial Phabricator instance.

revlog: use dedicated code for reading sidedataClosedPublic