This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3441

pycompat: export queue module instead of symbols in module (API)
ClosedPublic

Authored by indygreg on May 5 2018, 9:58 PM.

Details

Reviewers
martinvonz
Group Reviewers
hg-reviewers
Commits
rHG8fb9985382be: pycompat: export queue module instead of symbols in module (API)
Summary

Previously, pycompat and util re-exported individual symbols from the
queue module. This had the side-effect of forcing the loading of the
queue module whenever pycompat/util was imported.

These symbols aren't used very often. So importing the module to
get a handle on the symbols is wasteful.

This commit changes pycompat so it no longer exports the individual
symbols in the queue module. Instead, we make the imported module
a "public" symbol. We drop the individual symbol aliases from the
util module. All consumers are updated to use pycompat.queue.* instead.

This change makes 300 invocations of hg log -r. -T '{rev}\n' a little
faster:

before: 18.44s
after: 17.87s

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

indygreg created this revision.May 5 2018, 9:58 PM
martinvonz accepted this revision.May 6 2018, 12:18 AM
martinvonz added a subscriber: martinvonz.
martinvonz added inline comments.
contrib/check-code.py
343–345

Maybe these should be "pycompat.queue.Queue" and "pycompat.queue.Empty" instead?

This revision is now accepted and ready to land.May 6 2018, 12:18 AM
indygreg added inline comments.May 6 2018, 12:21 AM
contrib/check-code.py
343–345

Yes, they should. Will you fix in flight or do you want another submit?

martinvonz added inline comments.May 6 2018, 12:22 AM
contrib/check-code.py
343–345

I can fix. Thanks for confirming quickly, because I wasn't confident enough to just change it.

Closed by commit rHG8fb9985382be: pycompat: export queue module instead of symbols in module (API) (authored by indygreg). · Explain WhyMay 6 2018, 12:41 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mcontrib/check-code.py (3 lines)
Mcontrib/perf.py (12 lines)
Mmercurial/pycompat.py (7 lines)
Mmercurial/util.py (2 lines)
Mmercurial/vfs.py (4 lines)
Mmercurial/worker.py (6 lines)

Diff 8464

contrib/check-code.py

(r'os\.path\.join\(.*, *(""|\'\')\)', (r'os\.path\.join\(.*, *(""|\'\')\)',
"use pathutil.normasprefix(path) instead of os.path.join(path, '')"), "use pathutil.normasprefix(path) instead of os.path.join(path, '')"),
(r'\s0[0-7]+\b', 'legacy octal syntax; use "0o" prefix instead of "0"'), (r'\s0[0-7]+\b', 'legacy octal syntax; use "0o" prefix instead of "0"'),
# XXX only catch mutable arguments on the first line of the definition # XXX only catch mutable arguments on the first line of the definition
(r'def.*[( ]\w+=\{\}', "don't use mutable default arguments"), (r'def.*[( ]\w+=\{\}', "don't use mutable default arguments"),
(r'\butil\.Abort\b', "directly use error.Abort"), (r'\butil\.Abort\b', "directly use error.Abort"),
(r'^@(\w*\.)?cachefunc', "module-level @cachefunc is risky, please avoid"), (r'^@(\w*\.)?cachefunc', "module-level @cachefunc is risky, please avoid"),
(r'^import atexit', "don't use atexit, use ui.atexit"), (r'^import atexit', "don't use atexit, use ui.atexit"),
(r'^import Queue', "don't use Queue, use util.queue + util.empty"), (r'^import Queue', "don't use Queue, use pycompat.queue.Queue + "
"pycompat.queue.Empty"),
(r'^import cStringIO', "don't use cStringIO.StringIO, use util.stringio"), (r'^import cStringIO', "don't use cStringIO.StringIO, use util.stringio"),
martinvonzUnsubmitted
Not Done

Maybe these should be "pycompat.queue.Queue" and "pycompat.queue.Empty" instead?

martinvonz: Maybe these should be "pycompat.queue.Queue" and "pycompat.queue.Empty" instead?
indygregAuthorUnsubmitted
Not Done

Yes, they should. Will you fix in flight or do you want another submit?

indygreg: Yes, they should. Will you fix in flight or do you want another submit?
martinvonzUnsubmitted
Not Done

I can fix. Thanks for confirming quickly, because I wasn't confident enough to just change it.

martinvonz: I can fix. Thanks for confirming quickly, because I wasn't confident enough to just change it.
(r'^import urllib', "don't use urllib, use util.urlreq/util.urlerr"), (r'^import urllib', "don't use urllib, use util.urlreq/util.urlerr"),
(r'^import SocketServer', "don't use SockerServer, use util.socketserver"), (r'^import SocketServer', "don't use SockerServer, use util.socketserver"),
(r'^import urlparse', "don't use urlparse, use util.urlreq"), (r'^import urlparse', "don't use urlparse, use util.urlreq"),
(r'^import xmlrpclib', "don't use xmlrpclib, use util.xmlrpclib"), (r'^import xmlrpclib', "don't use xmlrpclib, use util.xmlrpclib"),
(r'^import cPickle', "don't use cPickle, use util.pickle"), (r'^import cPickle', "don't use cPickle, use util.pickle"),
(r'^import pickle', "don't use pickle, use util.pickle"), (r'^import pickle', "don't use pickle, use util.pickle"),
(r'^import httplib', "don't use httplib, use util.httplib"), (r'^import httplib', "don't use httplib, use util.httplib"),
(r'^import BaseHTTPServer', "use util.httpserver instead"), (r'^import BaseHTTPServer', "use util.httpserver instead"),

contrib/perf.py

pass pass
try: try:
from mercurial import pycompat from mercurial import pycompat
getargspec = pycompat.getargspec # added to module after 4.5 getargspec = pycompat.getargspec # added to module after 4.5
except (ImportError, AttributeError): except (ImportError, AttributeError):
import inspect import inspect
getargspec = inspect.getargspec getargspec = inspect.getargspec
try:
# 4.7+
queue = pycompat.queue.Queue
except (AttributeError, ImportError):
# <4.7.
try:
queue = pycompat.queue
except (AttributeError, ImportError):
queue = util.queue
# for "historical portability": # for "historical portability":
# define util.safehasattr forcibly, because util.safehasattr has been # define util.safehasattr forcibly, because util.safehasattr has been
# available since 1.9.3 (or 94b200a11cf7) # available since 1.9.3 (or 94b200a11cf7)
_undefined = object() _undefined = object()
def safehasattr(thing, attr): def safehasattr(thing, attr):
return getattr(thing, attr, _undefined) is not _undefined return getattr(thing, attr, _undefined) is not _undefined
setattr(util, 'safehasattr', safehasattr) setattr(util, 'safehasattr', safehasattr)
for pair in textpairs: for pair in textpairs:
if xdiff: if xdiff:
mdiff.bdiff.xdiffblocks(*pair) mdiff.bdiff.xdiffblocks(*pair)
elif blocks: elif blocks:
mdiff.bdiff.blocks(*pair) mdiff.bdiff.blocks(*pair)
else: else:
mdiff.textdiff(*pair) mdiff.textdiff(*pair)
else: else:
q = util.queue() q = queue()
for i in xrange(threads): for i in xrange(threads):
q.put(None) q.put(None)
ready = threading.Condition() ready = threading.Condition()
done = threading.Event() done = threading.Event()
for i in xrange(threads): for i in xrange(threads):
threading.Thread(target=_bdiffworker, threading.Thread(target=_bdiffworker,
args=(q, blocks, xdiff, ready, done)).start() args=(q, blocks, xdiff, ready, done)).start()
q.join() q.join()

mercurial/pycompat.py

ispy3 = (sys.version_info[0] >= 3) ispy3 = (sys.version_info[0] >= 3)
ispypy = (r'__pypy__' in sys.builtin_module_names) ispypy = (r'__pypy__' in sys.builtin_module_names)
if not ispy3: if not ispy3:
import cookielib import cookielib
import cPickle as pickle import cPickle as pickle
import httplib import httplib
import Queue as _queue import Queue as queue
import SocketServer as socketserver import SocketServer as socketserver
import xmlrpclib import xmlrpclib
from .thirdparty.concurrent import futures from .thirdparty.concurrent import futures
def future_set_exception_info(f, exc_info): def future_set_exception_info(f, exc_info):
f.set_exception_info(*exc_info) f.set_exception_info(*exc_info)
else: else:
import concurrent.futures as futures import concurrent.futures as futures
import http.cookiejar as cookielib import http.cookiejar as cookielib
import http.client as httplib import http.client as httplib
import pickle import pickle
import queue as _queue import queue as queue
import socketserver import socketserver
import xmlrpc.client as xmlrpclib import xmlrpc.client as xmlrpclib
def future_set_exception_info(f, exc_info): def future_set_exception_info(f, exc_info):
f.set_exception(exc_info[0]) f.set_exception(exc_info[0])
empty = _queue.Empty
queue = _queue.Queue
def identity(a): def identity(a):
return a return a
if ispy3: if ispy3:
import builtins import builtins
import functools import functools
import io import io
import struct import struct

mercurial/util.py

base85 = policy.importmod(r'base85') base85 = policy.importmod(r'base85')
osutil = policy.importmod(r'osutil') osutil = policy.importmod(r'osutil')
parsers = policy.importmod(r'parsers') parsers = policy.importmod(r'parsers')
b85decode = base85.b85decode b85decode = base85.b85decode
b85encode = base85.b85encode b85encode = base85.b85encode
cookielib = pycompat.cookielib cookielib = pycompat.cookielib
empty = pycompat.empty
httplib = pycompat.httplib httplib = pycompat.httplib
pickle = pycompat.pickle pickle = pycompat.pickle
queue = pycompat.queue
safehasattr = pycompat.safehasattr safehasattr = pycompat.safehasattr
socketserver = pycompat.socketserver socketserver = pycompat.socketserver
bytesio = pycompat.bytesio bytesio = pycompat.bytesio
# TODO deprecate stringio name, as it is a lie on Python 3. # TODO deprecate stringio name, as it is a lie on Python 3.
stringio = bytesio stringio = bytesio
xmlrpclib = pycompat.xmlrpclib xmlrpclib = pycompat.xmlrpclib
httpserver = urllibcompat.httpserver httpserver = urllibcompat.httpserver

mercurial/vfs.py

return return
maxqueue = ui.configint('worker', 'backgroundclosemaxqueue') maxqueue = ui.configint('worker', 'backgroundclosemaxqueue')
threadcount = ui.configint('worker', 'backgroundclosethreadcount') threadcount = ui.configint('worker', 'backgroundclosethreadcount')
ui.debug('starting %d threads for background file closing\n' % ui.debug('starting %d threads for background file closing\n' %
threadcount) threadcount)
self._queue = util.queue(maxsize=maxqueue) self._queue = pycompat.queue.Queue(maxsize=maxqueue)
self._running = True self._running = True
for i in range(threadcount): for i in range(threadcount):
t = threading.Thread(target=self._worker, name='backgroundcloser') t = threading.Thread(target=self._worker, name='backgroundcloser')
self._threads.append(t) self._threads.append(t)
t.start() t.start()
def __enter__(self): def __enter__(self):
fh = self._queue.get(block=True, timeout=0.100) fh = self._queue.get(block=True, timeout=0.100)
# Need to catch or the thread will terminate and # Need to catch or the thread will terminate and
# we could orphan file descriptors. # we could orphan file descriptors.
try: try:
fh.close() fh.close()
except Exception as e: except Exception as e:
# Stash so can re-raise from main thread later. # Stash so can re-raise from main thread later.
self._threadexception = e self._threadexception = e
except util.empty: except pycompat.queue.Empty:
if not self._running: if not self._running:
break break
def close(self, fh): def close(self, fh):
"""Schedule a file for closing.""" """Schedule a file for closing."""
if not self._entered: if not self._entered:
raise error.Abort(_('can only call close() when context manager ' raise error.Abort(_('can only call close() when context manager '
'active')) 'active'))

mercurial/worker.py

args = self._taskqueue.get_nowait() args = self._taskqueue.get_nowait()
for res in self._func(*self._staticargs + (args,)): for res in self._func(*self._staticargs + (args,)):
self._resultqueue.put(res) self._resultqueue.put(res)
# threading doesn't provide a native way to # threading doesn't provide a native way to
# interrupt execution. handle it manually at every # interrupt execution. handle it manually at every
# iteration. # iteration.
if self._interrupted: if self._interrupted:
return return
except util.empty: except pycompat.queue.Empty:
break break
except Exception as e: except Exception as e:
# store the exception such that the main thread can resurface # store the exception such that the main thread can resurface
# it as if the func was running without workers. # it as if the func was running without workers.
self.exception = e self.exception = e
raise raise
threads = [] threads = []
# important to surface the inital exception than the # important to surface the inital exception than the
# fact that one of workers may be processing a large # fact that one of workers may be processing a large
# task and does not get to handle the interruption. # task and does not get to handle the interruption.
ui.warn(_("failed to kill worker threads while " ui.warn(_("failed to kill worker threads while "
"handling an exception\n")) "handling an exception\n"))
return return
workers = _numworkers(ui) workers = _numworkers(ui)
resultqueue = util.queue() resultqueue = pycompat.queue.Queue()
taskqueue = util.queue() taskqueue = pycompat.queue.Queue()
# partition work to more pieces than workers to minimize the chance # partition work to more pieces than workers to minimize the chance
# of uneven distribution of large tasks between the workers # of uneven distribution of large tasks between the workers
for pargs in partition(args, workers * 20): for pargs in partition(args, workers * 20):
taskqueue.put(pargs) taskqueue.put(pargs)
for _i in range(workers): for _i in range(workers):
t = Worker(taskqueue, resultqueue, func, staticargs) t = Worker(taskqueue, resultqueue, func, staticargs)
threads.append(t) threads.append(t)
t.start() t.start()