This is an archive of the discontinued Mercurial Phabricator instance.

Differential D476

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

Event Timeline

mbthomas created this revision.Aug 22 2017, 10:30 AM
simonfar added a subscriber: simonfar.Aug 23 2017, 9:06 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 a subscriber: indygreg.Aug 23 2017, 11:19 AM
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
mbthomas added a comment.Sep 21 2017, 8:57 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
Closed by commit rHG3bb2a9f25fe9: util: add an mmapread method (authored by mbthomas). · Explain WhySep 21 2017, 2:05 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/util.py (12 lines)

Diff 1968

mercurial/util.py

import codecs import codecs
import collections import collections
import contextlib import contextlib
import datetime import datetime
import errno import errno
import gc import gc
import hashlib import hashlib
import imp import imp
import mmap
import os import os
import platform as pyplatform import platform as pyplatform
import re as remod import re as remod
import shutil import shutil
import signal import signal
import socket import socket
import stat import stat
import string import string
"""read data to the buffer""" """read data to the buffer"""
data = os.read(self._input.fileno(), _chunksize) data = os.read(self._input.fileno(), _chunksize)
if not data: if not data:
self._eof = True self._eof = True
else: else:
self._lenbuf += len(data) self._lenbuf += len(data)
self._buffer.append(data) self._buffer.append(data)
def mmapread(fp):
try:
fd = getattr(fp, 'fileno', lambda: fp)()
simonfarUnsubmitted
Not Done

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

simonfar: As per comment on D477 - do we want this to be raw, or `buffer(mmap.mmap(...`?
indygregUnsubmitted
Not Done

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`.

indygreg: Some consumers may want file like objects. Others may want buffers. Whatever you do, I would…
quarkUnsubmitted
Not Done

I think we can take both:

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

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

quark: I think we can take both: fd = getattr(fp, 'fileno', lambda: fp)() ... A lot of Python…
return mmap.mmap(fd, 0, 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.
if os.fstat(fd).st_size == 0:
return ''
raise
def popen2(cmd, env=None, newlines=False): def popen2(cmd, env=None, newlines=False):
# Setting bufsize to -1 lets the system decide the buffer size. # Setting bufsize to -1 lets the system decide the buffer size.
# The default for bufsize is 0, meaning unbuffered. This leads to # The default for bufsize is 0, meaning unbuffered. This leads to
# poor performance on Mac OS X: http://bugs.python.org/issue4194 # poor performance on Mac OS X: http://bugs.python.org/issue4194
p = subprocess.Popen(cmd, shell=True, bufsize=-1, p = subprocess.Popen(cmd, shell=True, bufsize=-1,
close_fds=closefds, close_fds=closefds,
stdin=subprocess.PIPE, stdout=subprocess.PIPE, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
universal_newlines=newlines, universal_newlines=newlines,