[Updated] D10878: revlog: Extract low-level random-access file read caching logic

[Alphare (Raphaël Gomès)]

This revision now requires changes to proceed.
Alphare added inline comments.
Alphare requested changes to this revision.

INLINE COMMENTS

> randomaccessfile.py:15
> +
> +_MAX_CACHED_CHUNK_SIZE = 1048576  # 1 MiB
> +

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.

> randomaccessfile.py:51
> +    def _open_read(self, existing_file_obj=None):
> +        """File object suitable to read data"""
> +        # Use explicit file handle, if given.

nit: suitable for reading data

> randomaccessfile.py:97
> +        # involving reading the revlog backwards.
> +        cache_size = self.default_cached_chunk_size
> +        real_offset = offset & ~(cache_size - 1)

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.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D10878/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D10878

To: SimonSapin, indygreg, #hg-reviewers, Alphare
Cc: Alphare, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210617/2899eabb/attachment-0002.html>