This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10878

revlog: Extract low-level random-access file read caching logic
ClosedPublic

Authored by SimonSapin on Jun 15 2021, 7:30 AM.

Details

Reviewers
indygreg
Alphare
Group Reviewers
hg-reviewers
Commits
rHGe0a314bcbc9d: revlog: Extract low-level random-access file read caching logic
Summary

The revlog class does many things, among which fulfilling requests for
arbitrary byte slices from the revlog "data file" by reading a larger chunk
and caching it in memory, in order to reduce the number of system calls.

This extracts that logic into a new class, so that it may later also be used
for the side-data file (with another instance of that class).

The copyright notice of the new file does not include a date or author name
since such information tend not to be kept up-to-date:

https://www.linuxfoundation.org/en/blog/copyright-notices-in-open-source-software-projects/

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

SimonSapin created this revision.Jun 15 2021, 7:30 AM
Alphare requested changes to this revision.Jun 17 2021, 5:08 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
mercurial/revlogutils/randomaccessfile.py
16

We should add an assertion with a comment explaining that we do some bit-wise magic afterwards that depends on it being a power of two.

52

nit: suitable for reading data

98

I would rename cache_size to chunk_size, because I was very confused for a minute with the bit-wise manipulation involving the size of the cache.

This revision now requires changes to proceed.Jun 17 2021, 5:08 AM
baymax updated this revision to Diff 28597.Jun 17 2021, 12:28 PM

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

Alphare accepted this revision.Jun 17 2021, 1:15 PM
Alphare added inline comments.
mercurial/revlog.py
92

I forgot this on the first pass, but this should be grouped with the other import from revlogutils. I will change that in flight

mercurial/revlogutils/randomaccessfile.py
16

For posterity, this was not the right constant for this comment, but the assertion was added.

This revision is now accepted and ready to land.Jun 17 2021, 1:15 PM

Revision Contents
Changeset List

Diff 28597

mercurial/changelog.py

if self._realopener.exists(self._indexfile + b'.a'): if self._realopener.exists(self._indexfile + b'.a'):
self._realopener.unlink(self._indexfile + b'.a') self._realopener.unlink(self._indexfile + b'.a')
self.opener = _divertopener(self._realopener, self._indexfile) self.opener = _divertopener(self._realopener, self._indexfile)
else: else:
self._delaybuf = [] self._delaybuf = []
self.opener = _delayopener( self.opener = _delayopener(
self._realopener, self._indexfile, self._delaybuf self._realopener, self._indexfile, self._delaybuf
) )
self._segmentfile.opener = self.opener
self._delayed = True self._delayed = True
tr.addpending(b'cl-%i' % id(self), self._writepending) tr.addpending(b'cl-%i' % id(self), self._writepending)
tr.addfinalize(b'cl-%i' % id(self), self._finalize) tr.addfinalize(b'cl-%i' % id(self), self._finalize)
def _finalize(self, tr): def _finalize(self, tr):
"""finalize index updates""" """finalize index updates"""
self._delayed = False self._delayed = False
self.opener = self._realopener self.opener = self._realopener
self._segmentfile.opener = self.opener
# move redirected index data back into place # move redirected index data back into place
if self._docket is not None: if self._docket is not None:
self._write_docket(tr) self._write_docket(tr)
elif self._divert: elif self._divert:
assert not self._delaybuf assert not self._delaybuf
tmpname = self._indexfile + b".a" tmpname = self._indexfile + b".a"
nfile = self.opener.open(tmpname) nfile = self.opener.open(tmpname)
nfile.close() nfile.close()
fp2.write(fp1.read()) fp2.write(fp1.read())
# add pending data # add pending data
fp2.write(b"".join(self._delaybuf)) fp2.write(b"".join(self._delaybuf))
fp2.close() fp2.close()
# switch modes so finalize can simply rename # switch modes so finalize can simply rename
self._delaybuf = None self._delaybuf = None
self._divert = True self._divert = True
self.opener = _divertopener(self._realopener, self._indexfile) self.opener = _divertopener(self._realopener, self._indexfile)
self._segmentfile.opener = self.opener
if self._divert: if self._divert:
return True return True
return False return False
def _enforceinlinesize(self, tr): def _enforceinlinesize(self, tr):
if not self._delayed: if not self._delayed:

mercurial/revlog.py

censor, censor,
deltas as deltautil, deltas as deltautil,
docket as docketutil, docket as docketutil,
flagutil, flagutil,
nodemap as nodemaputil, nodemap as nodemaputil,
revlogv0, revlogv0,
sidedata as sidedatautil, sidedata as sidedatautil,
) )
from .revlogutils import randomaccessfile
AlphareUnsubmitted
Not Done

I forgot this on the first pass, but this should be grouped with the other import from revlogutils. I will change that in flight

Alphare: I forgot this on the first pass, but this should be grouped with the other import from…
from .utils import ( from .utils import (
storageutil, storageutil,
stringutil, stringutil,
) )
# blanked usage of all the name to prevent pyflakes constraints # blanked usage of all the name to prevent pyflakes constraints
# We need these name available in the module for extensions. # We need these name available in the module for extensions.
rustdagop = policy.importrust('dagop') rustdagop = policy.importrust('dagop')
rustrevlog = policy.importrust('revlog') rustrevlog = policy.importrust('revlog')
# Aliased for performance. # Aliased for performance.
_zlibdecompress = zlib.decompress _zlibdecompress = zlib.decompress
# max size of revlog with inline data # max size of revlog with inline data
_maxinline = 131072 _maxinline = 131072
_chunksize = 1048576
# Flag processors for REVIDX_ELLIPSIS. # Flag processors for REVIDX_ELLIPSIS.
def ellipsisreadprocessor(rl, text): def ellipsisreadprocessor(rl, text):
return text, False return text, False
def ellipsiswriteprocessor(rl, text): def ellipsiswriteprocessor(rl, text):
return text, False return text, False
index, cache = parse_index_v1(data, inline) index, cache = parse_index_v1(data, inline)
return rustrevlog.MixedIndex(index), cache return rustrevlog.MixedIndex(index), cache
# 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
PARTIAL_READ_MSG = _(
b'partial read of revlog %s; expected %d bytes from offset %d, got %d'
)
FILE_TOO_SHORT_MSG = _( FILE_TOO_SHORT_MSG = _(
b'cannot read from revlog %s;' b'cannot read from revlog %s;'
b' expected %d bytes from offset %d, data size is %d' b' expected %d bytes from offset %d, data size is %d'
) )
class revlog(object): class revlog(object):
""" """
elif self._format_version == CHANGELOGV2: elif self._format_version == CHANGELOGV2:
self._parse_index = parse_index_cl_v2 self._parse_index = parse_index_cl_v2
elif devel_nodemap: elif devel_nodemap:
self._parse_index = parse_index_v1_nodemap self._parse_index = parse_index_v1_nodemap
elif use_rust_index: elif use_rust_index:
self._parse_index = parse_index_v1_mixed self._parse_index = parse_index_v1_mixed
try: try:
d = self._parse_index(index_data, self._inline) d = self._parse_index(index_data, self._inline)
index, _chunkcache = d index, chunkcache = d
use_nodemap = ( use_nodemap = (
not self._inline not self._inline
and self._nodemap_file is not None and self._nodemap_file is not None
and util.safehasattr(index, 'update_nodemap_data') and util.safehasattr(index, 'update_nodemap_data')
) )
if use_nodemap: if use_nodemap:
nodemap_data = nodemaputil.persisted_data(self) nodemap_data = nodemaputil.persisted_data(self)
if nodemap_data is not None: if nodemap_data is not None:
docket = nodemap_data[0] docket = nodemap_data[0]
if ( if (
len(d[0]) > docket.tip_rev len(d[0]) > docket.tip_rev
and d[0][docket.tip_rev][7] == docket.tip_node and d[0][docket.tip_rev][7] == docket.tip_node
): ):
# no changelog tampering # no changelog tampering
self._nodemap_docket = docket self._nodemap_docket = docket
index.update_nodemap_data(*nodemap_data) index.update_nodemap_data(*nodemap_data)
except (ValueError, IndexError): except (ValueError, IndexError):
raise error.RevlogError( raise error.RevlogError(
_(b"index %s is corrupted") % self.display_id _(b"index %s is corrupted") % self.display_id
) )
self.index, self._chunkcache = d self.index = index
if not self._chunkcache: self._segmentfile = randomaccessfile.randomaccessfile(
self._chunkclear() self.opener,
(self._indexfile if self._inline else self._datafile),
self._chunkcachesize,
chunkcache,
)
# revnum -> (chain-length, sum-delta-length) # revnum -> (chain-length, sum-delta-length)
self._chaininfocache = util.lrucachedict(500) self._chaininfocache = util.lrucachedict(500)
# revlog header -> revlog compressor # revlog header -> revlog compressor
self._decompressors = {} self._decompressors = {}
@util.propertycache @util.propertycache
def revlog_kind(self): def revlog_kind(self):
return self.target[0] return self.target[0]
atomictemp=True, atomictemp=True,
) )
def _datafp(self, mode=b'r'): def _datafp(self, mode=b'r'):
"""file object for the revlog's data file""" """file object for the revlog's data file"""
return self.opener(self._datafile, mode=mode) return self.opener(self._datafile, mode=mode)
@contextlib.contextmanager @contextlib.contextmanager
def _datareadfp(self, existingfp=None):
"""file object suitable to read data"""
# Use explicit file handle, if given.
if existingfp is not None:
yield existingfp
# Use a file handle being actively used for writes, if available.
# There is some danger to doing this because reads will seek the
# file. However, _writeentry() performs a SEEK_END before all writes,
# so we should be safe.
elif self._writinghandles:
if self._inline:
yield self._writinghandles[0]
else:
yield self._writinghandles[1]
# Otherwise open a new file handle.
else:
if self._inline:
func = self._indexfp
else:
func = self._datafp
with func() as fp:
yield fp
@contextlib.contextmanager
def _sidedatareadfp(self): def _sidedatareadfp(self):
"""file object suitable to read sidedata""" """file object suitable to read sidedata"""
if self._writinghandles: if self._writinghandles:
yield self._writinghandles[2] yield self._writinghandles[2]
else: else:
with self.opener(self._sidedatafile) as fp: with self.opener(self._sidedatafile) as fp:
yield fp yield fp
if transaction is None: if transaction is None:
nodemaputil.update_persistent_nodemap(self) nodemaputil.update_persistent_nodemap(self)
else: else:
nodemaputil.setup_persistent_nodemap(transaction, self) nodemaputil.setup_persistent_nodemap(transaction, self)
def clearcaches(self): def clearcaches(self):
self._revisioncache = None self._revisioncache = None
self._chainbasecache.clear() self._chainbasecache.clear()
self._chunkcache = (0, b'') self._segmentfile.clear_cache()
self._pcache = {} self._pcache = {}
self._nodemap_docket = None self._nodemap_docket = None
self.index.clearcaches() self.index.clearcaches()
# The python code is the one responsible for validating the docket, we # The python code is the one responsible for validating the docket, we
# end up having to refresh it here. # end up having to refresh it here.
use_nodemap = ( use_nodemap = (
not self._inline not self._inline
and self._nodemap_file is not None and self._nodemap_file is not None
def cmp(self, node, text): def cmp(self, node, text):
"""compare text with a given file revision """compare text with a given file revision
returns True if text is different than what is stored. returns True if text is different than what is stored.
""" """
p1, p2 = self.parents(node) p1, p2 = self.parents(node)
return storageutil.hashrevisionsha1(text, p1, p2) != node return storageutil.hashrevisionsha1(text, p1, p2) != node
def _cachesegment(self, offset, data):
"""Add a segment to the revlog cache.
Accepts an absolute offset and the data that is at that location.
"""
o, d = self._chunkcache
# try to add to existing cache
if o + len(d) == offset and len(d) + len(data) < _chunksize:
self._chunkcache = o, d + data
else:
self._chunkcache = offset, data
def _readsegment(self, offset, length, df=None):
"""Load a segment of raw data from the revlog.
Accepts an absolute offset, length to read, and an optional existing
file handle to read from.
If an existing file handle is passed, it will be seeked and the
original seek position will NOT be restored.
Returns a str or buffer of raw byte data.
Raises if the requested number of bytes could not be read.
"""
# Cache data both forward and backward around the requested
# data, in a fixed size window. This helps speed up operations
# involving reading the revlog backwards.
cachesize = self._chunkcachesize
realoffset = offset & ~(cachesize - 1)
reallength = (
(offset + length + cachesize) & ~(cachesize - 1)
) - realoffset
with self._datareadfp(df) as df:
df.seek(realoffset)
d = df.read(reallength)
self._cachesegment(realoffset, d)
if offset != realoffset or reallength != length:
startoffset = offset - realoffset
if len(d) - startoffset < length:
filename = self._indexfile if self._inline else self._datafile
got = len(d) - startoffset
m = PARTIAL_READ_MSG % (filename, length, offset, got)
raise error.RevlogError(m)
return util.buffer(d, startoffset, length)
if len(d) < length:
filename = self._indexfile if self._inline else self._datafile
got = len(d) - startoffset
m = PARTIAL_READ_MSG % (filename, length, offset, got)
raise error.RevlogError(m)
return d
def _getsegment(self, offset, length, df=None):
"""Obtain a segment of raw data from the revlog.
Accepts an absolute offset, length of bytes to obtain, and an
optional file handle to the already-opened revlog. If the file
handle is used, it's original seek position will not be preserved.
Requests for data may be returned from a cache.
Returns a str or a buffer instance of raw byte data.
"""
o, d = self._chunkcache
l = len(d)
# is it in the cache?
cachestart = offset - o
cacheend = cachestart + length
if cachestart >= 0 and cacheend <= l:
if cachestart == 0 and cacheend == l:
return d # avoid a copy
return util.buffer(d, cachestart, cacheend - cachestart)
return self._readsegment(offset, length, df=df)
def _getsegmentforrevs(self, startrev, endrev, df=None): def _getsegmentforrevs(self, startrev, endrev, df=None):
"""Obtain a segment of raw data corresponding to a range of revisions. """Obtain a segment of raw data corresponding to a range of revisions.
Accepts the start and end revisions and an optional already-open Accepts the start and end revisions and an optional already-open
file handle to be used for reading. If the file handle is read, its file handle to be used for reading. If the file handle is read, its
seek position will not be preserved. seek position will not be preserved.
Requests for data may be satisfied by a cache. Requests for data may be satisfied by a cache.
iend = index[endrev] iend = index[endrev]
end = int(iend[0] >> 16) + iend[1] end = int(iend[0] >> 16) + iend[1]
if self._inline: if self._inline:
start += (startrev + 1) * self.index.entry_size start += (startrev + 1) * self.index.entry_size
end += (endrev + 1) * self.index.entry_size end += (endrev + 1) * self.index.entry_size
length = end - start length = end - start
return start, self._getsegment(start, length, df=df) return start, self._segmentfile.read_chunk(start, length, df)
def _chunk(self, rev, df=None): def _chunk(self, rev, df=None):
"""Obtain a single decompressed chunk for a revision. """Obtain a single decompressed chunk for a revision.
Accepts an integer revision and an optional already-open file handle Accepts an integer revision and an optional already-open file handle
to be used for reading. If used, the seek position of the file will not to be used for reading. If used, the seek position of the file will not
be preserved. be preserved.
ladd(def_decomp(c)) ladd(def_decomp(c))
else: else:
msg = 'unknown compression mode %d' msg = 'unknown compression mode %d'
msg %= comp_mode msg %= comp_mode
raise error.RevlogError(msg) raise error.RevlogError(msg)
return l return l
def _chunkclear(self):
"""Clear the raw chunk cache."""
self._chunkcache = (0, b'')
def deltaparent(self, rev): def deltaparent(self, rev):
"""return deltaparent of the given revision""" """return deltaparent of the given revision"""
base = self.index[rev][3] base = self.index[rev][3]
if base == rev: if base == rev:
return nullrev return nullrev
elif self._generaldelta: elif self._generaldelta:
return base return base
else: else:
sdf.seek(sidedata_offset, os.SEEK_SET) sdf.seek(sidedata_offset, os.SEEK_SET)
comp_segment = sdf.read(sidedata_size) comp_segment = sdf.read(sidedata_size)
if len(comp_segment) < sidedata_size: if len(comp_segment) < sidedata_size:
filename = self._sidedatafile filename = self._sidedatafile
length = sidedata_size length = sidedata_size
offset = sidedata_offset offset = sidedata_offset
got = len(comp_segment) got = len(comp_segment)
m = PARTIAL_READ_MSG % (filename, length, offset, got) m = randomaccessfile.PARTIAL_READ_MSG % (
filename,
length,
offset,
got,
)
raise error.RevlogError(m) 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:
if self._writinghandles is not None: if self._writinghandles is not None:
existing_handles = True existing_handles = True
fp = self._writinghandles[0] fp = self._writinghandles[0]
fp.flush() fp.flush()
fp.close() fp.close()
# We can't use the cached file handle after close(). So prevent # We can't use the cached file handle after close(). So prevent
# its usage. # its usage.
self._writinghandles = None self._writinghandles = None
self._segmentfile.writing_handle = None
new_dfh = self._datafp(b'w+') new_dfh = self._datafp(b'w+')
new_dfh.truncate(0) # drop any potentially existing data new_dfh.truncate(0) # drop any potentially existing data
try: try:
with self._indexfp() as read_ifh: with self._indexfp() as read_ifh:
for r in self: for r in self:
new_dfh.write(self._getsegmentforrevs(r, r, df=read_ifh)[1]) new_dfh.write(self._getsegmentforrevs(r, r, df=read_ifh)[1])
if troffset <= self.start(r) + r * self.index.entry_size: if troffset <= self.start(r) + r * self.index.entry_size:
# when the index is replaced as otherwise data is lost. # when the index is replaced as otherwise data is lost.
tr.replace(self._datafile, self.start(trindex)) tr.replace(self._datafile, self.start(trindex))
# the temp file replace the real index when we exit the context # the temp file replace the real index when we exit the context
# manager # manager
tr.replace(self._indexfile, trindex * self.index.entry_size) tr.replace(self._indexfile, trindex * self.index.entry_size)
nodemaputil.setup_persistent_nodemap(tr, self) nodemaputil.setup_persistent_nodemap(tr, self)
self._chunkclear() self._segmentfile = randomaccessfile.randomaccessfile(
self.opener,
self._datafile,
self._chunkcachesize,
)
if existing_handles: if existing_handles:
# switched from inline to conventional reopen the index # switched from inline to conventional reopen the index
ifh = self.__index_write_fp() ifh = self.__index_write_fp()
self._writinghandles = (ifh, new_dfh, None) self._writinghandles = (ifh, new_dfh, None)
self._segmentfile.writing_handle = new_dfh
new_dfh = None new_dfh = None
finally: finally:
if new_dfh is not None: if new_dfh is not None:
new_dfh.close() new_dfh.close()
def _nodeduplicatecallback(self, transaction, node): def _nodeduplicatecallback(self, transaction, node):
"""called when trying to add a node already stored.""" """called when trying to add a node already stored."""
isize = r * self.index.entry_size isize = r * self.index.entry_size
ifh = self.__index_write_fp() ifh = self.__index_write_fp()
if self._inline: if self._inline:
transaction.add(self._indexfile, dsize + isize) transaction.add(self._indexfile, dsize + isize)
else: else:
transaction.add(self._indexfile, isize) transaction.add(self._indexfile, isize)
# exposing all file handle for writing. # exposing all file handle for writing.
self._writinghandles = (ifh, dfh, sdfh) self._writinghandles = (ifh, dfh, sdfh)
self._segmentfile.writing_handle = ifh if self._inline else dfh
yield yield
if self._docket is not None: if self._docket is not None:
self._write_docket(transaction) self._write_docket(transaction)
finally: finally:
self._writinghandles = None self._writinghandles = None
self._segmentfile.writing_handle = None
if dfh is not None: if dfh is not None:
dfh.close() dfh.close()
if sdfh is not None: if sdfh is not None:
dfh.close() dfh.close()
# closing the index file last to avoid exposing referent to # closing the index file last to avoid exposing referent to
# potential unflushed data content. # potential unflushed data content.
if ifh is not None: if ifh is not None:
ifh.close() ifh.close()
self._docket.index_end = end self._docket.index_end = end
self._docket.data_end = data_end self._docket.data_end = data_end
self._docket.sidedata_end = sidedata_end self._docket.sidedata_end = sidedata_end
self._docket.write(transaction, stripping=True) self._docket.write(transaction, stripping=True)
# then reset internal state in memory to forget those revisions # then reset internal state in memory to forget those revisions
self._revisioncache = None self._revisioncache = None
self._chaininfocache = util.lrucachedict(500) self._chaininfocache = util.lrucachedict(500)
self._chunkclear() self._segmentfile.clear_cache()
del self.index[rev:-1] del self.index[rev:-1]
def checksize(self): def checksize(self):
"""Check size of index and data files """Check size of index and data files
return a (dd, di) tuple. return a (dd, di) tuple.
- dd: extra bytes for the "data" file - dd: extra bytes for the "data" file

mercurial/revlogutils/randomaccessfile.py

  • This file was added.
# Copyright Mercurial Contributors
#
# This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version.
import contextlib
from ..i18n import _
from .. import (
error,
util,
)
_MAX_CACHED_CHUNK_SIZE = 1048576 # 1 MiB
AlphareUnsubmitted
Not Done

We should add an assertion with a comment explaining that we do some bit-wise magic afterwards that depends on it being a power of two.

Alphare: We should add an assertion with a comment explaining that we do some bit-wise magic afterwards…
AlphareUnsubmitted
Not Done

For posterity, this was not the right constant for this comment, but the assertion was added.

Alphare: For posterity, this was not the right constant for this comment, but the assertion was added.
PARTIAL_READ_MSG = _(
b'partial read of revlog %s; expected %d bytes from offset %d, got %d'
)
def _is_power_of_two(n):
return (n & (n - 1) == 0) and n != 0
class randomaccessfile(object):
"""Accessing arbitrary chuncks of data within a file, with some caching"""
def __init__(
self,
opener,
filename,
default_cached_chunk_size,
initial_cache=None,
):
# Required by bitwise manipulation below
assert _is_power_of_two(default_cached_chunk_size)
self.opener = opener
self.filename = filename
self.default_cached_chunk_size = default_cached_chunk_size
self.writing_handle = None # This is set from revlog.py
self._cached_chunk = b''
self._cached_chunk_position = 0 # Offset from the start of the file
if initial_cache:
self._cached_chunk_position, self._cached_chunk = initial_cache
def clear_cache(self):
self._cached_chunk = b''
self._cached_chunk_position = 0
def _open(self, mode=b'r'):
AlphareUnsubmitted
Not Done

nit: suitable for reading data

Alphare: nit: suitable for reading data
"""Return a file object"""
return self.opener(self.filename, mode=mode)
@contextlib.contextmanager
def _open_read(self, existing_file_obj=None):
"""File object suitable for reading data"""
# Use explicit file handle, if given.
if existing_file_obj is not None:
yield existing_file_obj
# Use a file handle being actively used for writes, if available.
# There is some danger to doing this because reads will seek the
# file. However, revlog._writeentry performs a SEEK_END before all
# writes, so we should be safe.
elif self.writing_handle:
yield self.writing_handle
# Otherwise open a new file handle.
else:
with self._open() as fp:
yield fp
def read_chunk(self, offset, length, existing_file_obj=None):
"""Read a chunk of bytes from the file.
Accepts an absolute offset, length to read, and an optional existing
file handle to read from.
If an existing file handle is passed, it will be seeked and the
original seek position will NOT be restored.
Returns a str or buffer of raw byte data.
Raises if the requested number of bytes could not be read.
"""
end = offset + length
cache_start = self._cached_chunk_position
cache_end = cache_start + len(self._cached_chunk)
# Is the requested chunk within the cache?
if cache_start <= offset and end <= cache_end:
if cache_start == offset and end == cache_end:
return self._cached_chunk # avoid a copy
relative_start = offset - cache_start
return util.buffer(self._cached_chunk, relative_start, length)
return self._read_and_update_cache(offset, length, existing_file_obj)
AlphareUnsubmitted
Not Done

I would rename cache_size to chunk_size, because I was very confused for a minute with the bit-wise manipulation involving the size of the cache.

Alphare: I would rename `cache_size` to `chunk_size`, because I was very confused for a minute with the…
def _read_and_update_cache(self, offset, length, existing_file_obj=None):
# Cache data both forward and backward around the requested
# data, in a fixed size window. This helps speed up operations
# involving reading the revlog backwards.
real_offset = offset & ~(self.default_cached_chunk_size - 1)
real_length = (
(offset + length + self.default_cached_chunk_size)
& ~(self.default_cached_chunk_size - 1)
) - real_offset
with self._open_read(existing_file_obj) as file_obj:
file_obj.seek(real_offset)
data = file_obj.read(real_length)
self._add_cached_chunk(real_offset, data)
relative_offset = offset - real_offset
got = len(data) - relative_offset
if got < length:
message = PARTIAL_READ_MSG % (self.filename, length, offset, got)
raise error.RevlogError(message)
if offset != real_offset or real_length != length:
return util.buffer(data, relative_offset, length)
return data
def _add_cached_chunk(self, offset, data):
"""Add to or replace the cached data chunk.
Accepts an absolute offset and the data that is at that location.
"""
if (
self._cached_chunk_position + len(self._cached_chunk) == offset
and len(self._cached_chunk) + len(data) < _MAX_CACHED_CHUNK_SIZE
):
# add to existing cache
self._cached_chunk += data
else:
self._cached_chunk = data
self._cached_chunk_position = offset