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

[Pierre-Yves David]

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.

-- 
Pierre-Yves David