Gregory Szorc <gregory.szorc@gmail.com> [Sun, 04 Oct 2015 11:20:52 -0700] rev 26458
streamclone: refactor maybeperformstreamclone to take a pullop
Just like all the other pull steps. Consistency is good.
This seems a little excessive right now since maybeperformstreamclone is
such a short function. This will be addressed in a subsequent patch.
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 04 Oct 2015 11:17:43 -0700] rev 26457
demandimport: replace more references to _demandmod instances
_demandmod instances may be referenced by multiple importing modules.
Before this patch, the _demandmod instance only maintained a reference
to its first consumer when using the "from X import Y" syntax. This is
because we only created a single _demandmod instance (attached to the
parent X module). If multiple modules A and B performed
"from X import Y", we'd produce a single _demandmod instance
"demandmod" with the following references:
X.Y = <demandmod>
A.Y = <demandmod>
B.Y = <demandmod>
The locals from the first consumer (A) would be stored in <demandmod1>.
When <demandmod1> was loaded, we'd look at the locals for the first
consumer and replace the symbol, if necessary. This resulted in state:
X.Y = <module>
A.Y = <module>
B.Y = <demandmod>
B's reference to Y wasn't updated and was still using the proxy object
because we just didn't record that B had a reference to <demandmod> that
needed updating!
With this patch, we add support for tracking which modules in addition
to the initial importer have a reference to the _demandmod instance and
we replace those references at module load time.
In the case of posix.py, this fixes an issue where the "encoding" module
was being proxied, resulting in hundreds of thousands of
__getattribute__ lookups on the _demandmod instance during dirstate
operations on mozilla-central, speeding up execution by many
milliseconds. There are likely several other operation that benefit from
this change as well.
The new mechanism isn't perfect: references in locals (not globals) may
likely linger. So, if there is an import inside a function and a symbol
from that module is used in a hot loop, we could have unwanted overhead
from proxying through _demandmod. Non-global imports are discouraged
anyway. So hopefully this isn't a big deal in practice. We could
potentially deploy a code checker that bans use of attribute lookups of
function-level-imported modules inside loops.
This deficiency in theory could be avoided by storing the set of globals
and locals dicts to update in the _demandmod instance. However, I tried
this and it didn't work. One reason is that some globals are _demandmod
instances. We could work around this, but it's a bit more work. There
also might be other module import foo at play. The solution as
implemented is better than what we had and IMO is good enough for the
time being.
It's worth noting that this sub-optimal behavior was made worse by the
introduction of absolute_import and its recommended "from . import X"
syntax for importing modules from the "mercurial" package. If we ever
wrote performance tests, measuring the amount of module imports and
__getattribute__ proxy calls through _demandmod instances would be
something I'd have it check.
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 04 Oct 2015 10:36:54 -0700] rev 26456
demandimport: refactor processfromitem
This will match the next patch smaller and easier to read.
Gregory Szorc <gregory.szorc@gmail.com> [Sat, 03 Oct 2015 15:30:17 -0700] rev 26455
demandimport: consolidate code for processing items in fromlist
This code was mostly duplicated. An upcoming patch will add more
complexity, making the duplication harder to justify. Consolidate the
code.
Yuya Nishihara <yuya@tcha.org> [Sat, 03 Oct 2015 15:16:33 +0900] rev 26454
pager: recreate stdout to make it line-buffered
We want to see partial command results as soon as possible. But the buffering
mode of stdout (= pager's stdin) was set to fully-buffered because it isn't
associated with a tty. So, this patch recreates new stdout object to force its
buffering mode.
Because two file objects are associated with the same stdout fd and their
destructors will call close(), one of them must be closed carefully. Python
expects that the stdout fd never be closed even after sys.stdout.close() [1],
but newstdout has no such hack. So this patch calls newstdout.close()
immediately before duplicating the original stdout fd to sys.stdout.
operation sys.stdout newstdout fd
--------------------- ---------- --------- --------
newstdout.close() open closed closed
os.dup2(stdoutfd, ..) open closed open
del sys.stdout closed closed open [1]
[1]: https://hg.python.org/cpython/file/v2.7.10/Python/sysmodule.c#l1391
Yuya Nishihara <yuya@tcha.org> [Sat, 03 Oct 2015 18:48:21 +0900] rev 26453
pager: rename variables of backup fds
The next patch will add backup file objects, so the original variable names
would be confusing.
Yuya Nishihara <yuya@tcha.org> [Sat, 03 Oct 2015 15:02:52 +0900] rev 26452
pager: inline _pagersubprocess() into _runpager()
We don't need _pagersubprocess() because the fork version was removed
at 59d794154e8d.
Yuya Nishihara <yuya@tcha.org> [Sat, 03 Oct 2015 23:10:40 +0900] rev 26451
ui: send traceback of devel warning to appropriate output stream
If ui.ferr is a command-server channel, traceback should be written to it.
Yuya Nishihara <yuya@tcha.org> [Sat, 03 Oct 2015 14:57:24 +0900] rev 26450
util.system: compare fileno to see if it needs stdout redirection
Future patches will reopen stdout to be line-buffered, so sys.stdout may
be different object than sys.__stdout__.
Gregory Szorc <gregory.szorc@gmail.com> [Fri, 02 Oct 2015 23:04:52 -0700] rev 26449
exchange: move stream clone logic into pull code path
Stream clones are a special case of clones. Clones are a special case of
pull. Most of the logic for deciding what to do at pull time is in
exchange.py. It makes sense for the stream clone determination to live
there as well.
This patch moves the calling of the stream clone code into pull(). The
checks in streamclone.canperformstreamclone() ensure that we don't
perform a stream clone unless it is possible.
A future patch will convert maybeperformstreamclone() to accept a
pullop to make it consistent with everything else in pull(). It will
also grow some functionality (in case you doubted the necessity of a 4
line function).