This is an archive of the discontinued Mercurial Phabricator instance.

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

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

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
93

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