This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7808

mmap: add a size argument to mmapread
ClosedPublic

Authored by marmoute on Jan 7 2020, 6:27 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG8ed8dfbeabb9: mmap: add a size argument to mmapread
Summary

With this argument, we can control the size of the mmap created. (previously it
was always the whole 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.Jan 7 2020, 6:27 AM
mharbison72 added a subscriber: mharbison72.Jan 7 2020, 12:39 PM
mharbison72 added inline comments.
mercurial/util.py
419–422

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

marmoute added inline comments.Jan 7 2020, 1:20 PM
mercurial/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.

durin42 added a subscriber: durin42.Jan 8 2020, 2:55 PM
durin42 added inline comments.
mercurial/util.py
419–422

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

marmoute added a commit: rHG8ed8dfbeabb9: mmap: add a size argument to mmapread.Jan 8 2020, 3:04 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG8ed8dfbeabb9: mmap: add a size argument to mmapread (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/util.py (10 lines)

Diff 19109

mercurial/util.py

self._eof = True self._eof = True
else: else:
self._lenbuf += len(data) self._lenbuf += len(data)
self._buffer.append(data) self._buffer.append(data)
return data return data
def mmapread(fp): def mmapread(fp, size=None):
if size == 0:
# size of 0 to mmap.mmap() means "all data"
# rather than "zero bytes", so special case that.
return b''
mharbison72Unsubmitted
Not Done

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

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

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.

marmoute: passing 0 in `mmap.mmap(fd, 0, access=mmap.ACCESS_READ)` get you "all the byte you can eat".
durin42Unsubmitted
Not Done

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

durin42: This merits a comment. I'll add one in flight.
elif size is None:
size = 0
try: try:
fd = getattr(fp, 'fileno', lambda: fp)() 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: except ValueError:
# Empty files cannot be mmapped, but mmapread should still work. Check # Empty files cannot be mmapped, but mmapread should still work. Check
# if the file is empty, and if so, return an empty buffer. # if the file is empty, and if so, return an empty buffer.
if os.fstat(fd).st_size == 0: if os.fstat(fd).st_size == 0:
return b'' return b''
raise raise