Patchwork D7808: mmap: add a size argument to mmapread

login
register
mail settings
Submitter phabricator
Date Jan. 7, 2020, 11:27 a.m.
Message ID <differential-rev-PHID-DREV-b3k4ainue6sg2xxbav65-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/44171/
State Superseded
Headers show

Comments

phabricator - Jan. 7, 2020, 11:27 a.m.
marmoute created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  With this argument, we can control the size of the mmap created. (previously it
  was always the whole file.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/util.py

CHANGE DETAILS




To: marmoute, #hg-reviewers
Cc: mercurial-devel
phabricator - Jan. 7, 2020, 5:39 p.m.
mharbison72 added inline comments.

INLINE COMMENTS

> util.py:419-422
> +    if size == 0:
> +        return b''
> +    elif size is None:
> +        size = 0

Why does an explicit size of 0 bail with an empty buffer, but an implicit size 0 proceed?

REPOSITORY
  rHG Mercurial

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

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

To: marmoute, #hg-reviewers
Cc: mharbison72, mercurial-devel
phabricator - Jan. 7, 2020, 6:20 p.m.
marmoute added inline comments.

INLINE COMMENTS

> mharbison72 wrote in util.py:419-422
> Why does an explicit size of 0 bail with an empty buffer, but an implicit size 0 proceed?

passing 0 in `mmap.mmap(fd, 0, access=mmap.ACCESS_READ)` get you "all the byte you can eat". That's is not 0 sized mmap.

The `size` argument for `util.mmapread` is (after this patch). If size is None "all you can eat". if size is specified: provided exactly that amount of bytes.

REPOSITORY
  rHG Mercurial

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

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

To: marmoute, #hg-reviewers
Cc: mharbison72, mercurial-devel
phabricator - Jan. 8, 2020, 7:55 p.m.
durin42 added inline comments.

INLINE COMMENTS

> marmoute wrote in util.py:419-422
> passing 0 in `mmap.mmap(fd, 0, access=mmap.ACCESS_READ)` get you "all the byte you can eat". That's is not 0 sized mmap.
> 
> The `size` argument for `util.mmapread` is (after this patch). If size is None "all you can eat". if size is specified: provided exactly that amount of bytes.

This merits a comment. I'll add one in flight.

REPOSITORY
  rHG Mercurial

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

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

To: marmoute, #hg-reviewers
Cc: durin42, mharbison72, mercurial-devel

Patch

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -415,10 +415,14 @@ 
         return data
 
 
-def mmapread(fp):
+def mmapread(fp, size=None):
+    if size == 0:
+        return b''
+    elif size is None:
+        size = 0
     try:
         fd = getattr(fp, 'fileno', lambda: fp)()
-        return mmap.mmap(fd, 0, access=mmap.ACCESS_READ)
+        return mmap.mmap(fd, size, access=mmap.ACCESS_READ)
     except ValueError:
         # Empty files cannot be mmapped, but mmapread should still work.  Check
         # if the file is empty, and if so, return an empty buffer.