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.

Download Raw Diff

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

Branch

default

Lint

No Linters Available

Unit

No Unit Test Coverage

Event Timeline

mbthomas created this revision.Aug 22 2017, 10:30 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptAug 22 2017, 10:30 AM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

mbthomas added a child revision: D477: revlog: add option to mmap revlog index.Aug 22 2017, 10:37 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.

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

Accept both file objects and fds

quark accepted this revision as: quark.Sep 13 2017, 8:38 PM

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

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

			Path	Packages
M			mercurial/util.py (8 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	1157		Aug 22 2017, 10:30 AM	★	★
Diff 2	1785	Accept both file objects and fds	Sep 13 2017, 1:21 PM	★	★
Diff 3	1963		Sep 21 2017, 8:56 AM	★	★
Diff 4	1968	rHG3bb2a9f25fe98c62076d2eae1b783ba5640685d5	Sep 21 2017, 8:54 AM	★	★

Commit	Local	Parents	Author	Summary	Date
5e5b0770e09f	36705	a763c891f36e	Mark Thomas	util: add an mmapread method (Show More…)	Sep 13 2017, 1:20 PM

	Status	Author	Revision
	Closed	mbthomas	D477 revlog: add option to mmap revlog index
	Closed	mbthomas	D476 util: add an mmapread method

Diff 1785

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: # cannot mmap an empty file
				return ''

	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,