[PATCH 3 of 3 V4] demandimport: replace more references to _demandmod instances

[Gregory Szorc]

On Sun, Oct 4, 2015 at 7:30 PM, Pierre-Yves David <
pierre-yves.david at ens-lyon.org> wrote:

>
>
> On 10/04/2015 11:18 AM, Gregory Szorc wrote:
>
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc at gmail.com>
>> # Date 1443982663 25200
>> #      Sun Oct 04 11:17:43 2015 -0700
>> # Node ID a93f857c51bb996e5a8546dcdf2f8ee8b6b8a6f0
>> # Parent  3600288ff9176f5694db81eb59fb2d4fd1a5f866
>> demandimport: replace more references to _demandmod instances
>>
>
> Series pushed to the clowncopter. Much thanks.
>
>
>
>> _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.
>>
>
> Could we have inline import be non-demand import one? I doubt anyone is
> going to inline import something and do nothing with it. A check-code rules
> coulds be made out of it.
>
>
I'll have to check on this. Unless globals == locals or locals is empty,
this would require inspect module magic, since __import__ doesn't receive
any arguments denoting module from function level "import" usage.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20151004/f32e0496/attachment.html>