( )⚙ D476 util: add an mmapread method

This is an archive of the discontinued Mercurial Phabricator instance.

util: add an mmapread method
ClosedPublic

Authored by mbthomas on Aug 22 2017, 10:30 AM.

Details

Reviewers
quark
durin42
Group Reviewers
Restricted Project
hg-reviewers
Commits
rHG3bb2a9f25fe9: util: add an mmapread method
Summary

This is useful for large files that are only partly touched.

Test Plan

Will be used and tested in a later patch.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

mbthomas created this revision.Aug 22 2017, 10:30 AM
simonfar added inline comments.
mercurial/util.py
413

As per comment on D477 - do we want this to be raw, or buffer(mmap.mmap(...?

indygreg added inline comments.
mercurial/util.py
413

Some consumers may want file like objects. Others may want buffers.

Whatever you do, I would rename the function to what it actually does. e.g. `mmapfd or mmapbuffer`.

quark added a subscriber: quark.Aug 23 2017, 12:00 PM
quark added inline comments.
mercurial/util.py
413

I think we can take both:

fd = getattr(fp, 'fileno', lambda: fp)()
...

A lot of Python stdlib function accepts both fd and fp.

quark requested changes to this revision.Sep 11 2017, 1:56 PM

Scanning staling patches - I think this patch is good after making fd work with both file object and file descriptor number.

I will fix it in-place if you don't get it on Wednesday.

This revision now requires changes to proceed.Sep 11 2017, 1:56 PM
mbthomas updated this revision to Diff 1785.Sep 13 2017, 1:21 PM

Accept both file objects and fds

quark accepted this revision as: quark.Sep 13 2017, 8:38 PM
durin42 requested changes to this revision.Sep 20 2017, 12:03 PM
durin42 added a subscriber: durin42.

Either this or D477 needs updating per the comment I made there. I'm just too nervous about the potential for silent mysterious changelog read failures here.

This revision now requires changes to proceed.Sep 20 2017, 12:03 PM
mbthomas updated this revision to Diff 1963.Sep 21 2017, 8:56 AM

As it stands, mmap.mmap only raises ValueError if the arguments are incorrect or if the file is zero-length. But since that's not guaranteed, I've added a check that the reason for the exception is that the file is zero length before returning the empty string.

durin42 accepted this revision.Sep 21 2017, 11:35 AM
This revision was automatically updated to reflect the committed changes.