[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Steve Dower]

Change by Steve Dower :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Steve Dower]

Steve Dower  added the comment:


New changeset db55f3fabafc046e4fca907210ced4ce16bf58d6 by Steve Dower in branch 
'main':
bpo-44530: Reverts a change to the 'code.__new__' audit event (GH-29809)
https://github.com/python/cpython/commit/db55f3fabafc046e4fca907210ced4ce16bf58d6


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Steve Dower]

Change by Steve Dower :


--
pull_requests: +28041
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/29809

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Steve Dower]

Steve Dower  added the comment:

Correction: the event is `code.__new__`

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Steve Dower]

Steve Dower  added the comment:

This change modified the audit event 'code.__name__', which requires a 
deprecation period (all events are public API, as per PEP 578).

We need to revert that part of the change. I don't think we need to add a new 
event to report the qualname here, so just dropping the new argument seems fine.

--
nosy: +steve.dower
resolution: fixed -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:


New changeset 8363c53369a582ff9ae4e797a80cdce12624a278 by Pablo Galindo in 
branch 'main':
bpo-44530: Document the new CodeObject.co_qualname attribute (GH-27052)
https://github.com/python/cpython/commit/8363c53369a582ff9ae4e797a80cdce12624a278


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
pull_requests: +25608
pull_request: https://github.com/python/cpython/pull/27052

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:


New changeset 2f180ce2cb6e6a7e3c517495e0f4873d6aaf5f2f by Gabriele N. Tornetta 
in branch 'main':
bpo-44530: Add co_qualname field to PyCodeObject (GH-26941)
https://github.com/python/cpython/commit/2f180ce2cb6e6a7e3c517495e0f4873d6aaf5f2f


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Mark Shannon]

Mark Shannon  added the comment:

I suspect that the 0.1% increase is noise.
The size of importlib.h etc show a small decrease, suggesting that the 
information content of the code object has *decreased*.

After all, the qualname has to stored somewhere and moving it from caller to 
callee should make no difference.

Organizing the code object so it wastes less memory is a separate issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Gabriele N Tornetta]

Gabriele N Tornetta  added the comment:

With commit 7a12d31a8c22cae7a9c1a2239a1bb54adee33622 the new figures are

# main (7569c0fe91): 25_059_438 bytes
# bpo-445303-code-qualname (7a12d31a8c): 25_089_917 bytes

which is a 0.1% relative increase :). This is likely due to the fact that with 
the change MAKE_FUNCTION needs to pop one less value from the TOS, as the 
qualname now comes from the code object.

I think the PR is now good for a first review. I think I'd need some help with 
the documentation sources.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> So that seems to be about half a MB increase over 25 MB (about 2% relative 
> increase).

I personally think that is acceptable, so I would be supportive of the patch 
but for context, many folks have indicated that they are worried about this 
size getting bigger during previous discussions, so we need to still think 
collectively :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Gabriele N Tornetta]

Gabriele N Tornetta  added the comment:

> That is a pointer size per code object. The most standard way is to calculate 
> the size of all pyc files in the stdlib after compiling them all with "-m 
> compileall -r 1000 Lib".

This yields the following numbers between what was main when I branched off and 
the tip of my branch:

# main (7569c0fe91): 25_059_438 bytes
# bpo-445303-code-qualname (a0252ab9ad): 25_511_492 bytes

So that seems to be about half a MB increase over 25 MB (about 2% relative 
increase).

This is potentially just an interim result, as at first sight it looks like 
MAKE_FUNCTION could require one less argument (the qualname), which can now be 
taken from the code object. So not quite the final picture yet.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> Is there a "standard" way for me to quantify the memory impact of these kinds 
> of changes?

That is a pointer size per code object. The most standard way is to calculate 
the size of all pyc files in the stdlib after compiling them all with "-m 
compileall -r 1000 Lib".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Gabriele N Tornetta]

Gabriele N Tornetta  added the comment:

@pablogsal

Commit a0252ab9add7d48e9e0604ebf97342e46cf00419 exposes co_qualname to Python. 
I've added a test case to check that the result coincides with the current 
implementation of __qualname__. Is there a "standard" way for me to quantify 
the memory impact of these kinds of changes?

@Mark.Shannon

I'll look into making __qualname__ use __code__.co_qualname as a next step.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Gabriele N Tornetta]

Gabriele N Tornetta  added the comment:

Thanks for the feedback. I certainly appreciate all the concerns around this 
proposed change.

@Mark.Shannon

1. This is the desired behaviour as profiling data should be attached to the 
actual code objects that correlate with the actual source content. Besides, in 
this respect, given the immutability of code objects, qualname behaves like 
name.

2. At the Python level it is already possible to get the __qualname__ of a 
function (but not of a code object). As this is all I needed, I kept my initial 
implementation to the bare minimum.

3. I agree with this analysis. Whilst this increases the memory footprint, it 
might have the benefit of making the derivation of __qualname__ faster as this 
would now be cached on code objects.

However, I also appreciate @pablogsal's point of view of evaluating the actual 
benefits. The extra point in favour of this change that I can make is that it 
would resolve name clashes in profiling data from tools that have minimal 
impact on the runtime. Whether this is enough to justify the extra memory cost 
is certainly up for debate. From my side, I don't have the numbers to make this 
case more concrete.

As a next step, I'll look into exposing the new field to Python to see what it 
actually ends up looking like.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

This also has the side effect of making marshalled code objects bigger, and we 
keep adding to this in 3.11 with the new exception table and soon with PEP 657. 
Given that a lot of people are concerned about this and that this change only 
affects a very small set of tools, we need to analyze very carefully the 
balance.

--
nosy: +pablogsal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Mark Shannon]

Mark Shannon  added the comment:

I think this is a worthwhile improvement.
A few comments on this issue and your PR.

1. We already have qualified names on functions and generators and those can be 
modified by @decorators.
In those cases, the code object and the function would disagree.
Code objects are immutable, so any thoughts on the usability implications of 
qualname differing between code object and function?

2. If we are adding a new attribute, it should be at the Python level. The 
internal layout of the code object is likely to change. (I have no problem with 
it being visible to tools like austin, just it won't be part of the API.)

3. As it stands, this is beneficial to only a small set of tools, but has a 
negative impact on memory use.
If we were to leverage the new field to improve the performance of function 
creation (important for closures) by omitting the qualname (and defaulting to 
the code object's) then it would benefit everyone.

--
nosy: +Mark.Shannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Gabriele N Tornetta]

Change by Gabriele N Tornetta :


--
keywords: +patch
pull_requests: +25508
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/26941

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44530] Propagate qualname from the compiler unit to code objects for finer grained profiling data

[Gabriele N Tornetta]

New submission from Gabriele N Tornetta :

When dumping profiling data out of code objects using out-of-process tools like 
Austin (https://github.com/p403n1x87/austin) one has access only to file name, 
function name, and line number. Consider the flame graph generated by running 
the following script, and aggregating on function names


class Foo:
def on_cpu(self, n):
a = []
for i in range(n):
a.append(i)


class Bar:
def on_cpu(self, n):
a = []
for i in range(n):
a.append(i)


if __name__ == "__main__":
f = Foo()
b = Bar()

f.on_cpu(1_000_000)
b.on_cpu(5_000_000)


Without the extra information coming from the actual Python source, one would 
not be able to tell, by looking at the flame graph alone, that on_cpu has 
contributions from two different methods. By propagating the qualname 
information from the compiler to code objects, such names would be 
disambiguated and the resulting flame graph would be clearer.

I would like to propose adding the co_qualname field to the PyCodeObject 
structure, which is to be set to NULL except for when the code object is 
created by the compiler in compile.c.

--
components: C API
files: py_main.png
messages: 396667
nosy: Gabriele Tornetta
priority: normal
severity: normal
status: open
title: Propagate qualname from the compiler unit to code objects for finer 
grained profiling data
type: enhancement
versions: Python 3.11
Added file: https://bugs.python.org/file50128/py_main.png

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com