Thanks for the contribution!

This could makegetclosurevars much slower if run on a very large function, because bytecode size can scale much faster than the set of names used.

In particular, if a large function has lots of references to builtins (even the same builtin used many times) we will repeatedly raise and catch an exception, possibly many times for the same builtin name. This repeated exception raising could at least be mitigated by keeping a set of seen global names, and not re-looking-up an already-seen name (or just avoiding re-looking-up names already present inbuiltin_vars).

That said, I'm not sure I see any other implementation strategy for fixing this function; we just don't preserve enough context from compilation on the code object, except in the bytecode itself.

Would like to see if any other reviewer has better ideas here, before merging this.

This could makegetclosurevars much slower if run on a very large function, because bytecode size can scale much faster than the set of names used.

In particular, if a large function has lots of references to builtins (even the same builtin used many times) we will repeatedly raise and catch an exception, possibly many times for the same builtin name. This repeated exception raising could at least be mitigated by keeping a set of seen global names, and not re-looking-up an already-seen name (or just avoiding re-looking-up names already present inbuiltin_vars).

That said, I'm not sure I see any other implementation strategy for fixing this function; we just don't preserve enough context from compilation on the code object, except in the bytecode itself.

Would like to see if any other reviewer has better ideas here, before merging this.

Thanks for the review! I totally agree with your assessments. I've updated the approach so that it collects all global names in a set first before looking them up in global and builtin namespaces. And yeah unfortunately I don't see a way not to scan through all the bytecodes if we are to aim for accuracy.

Thanks@blhsing for the PR, and@carljm for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

Thanks@blhsing for the PR, and@carljm for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

GH-126459 is a backport of this pull request to the3.13 branch.

GH-126460 is a backport of this pull request to the3.12 branch.