Repository navigation
pystats: optimized_trace_length histogram includes exits #116808
Description
Bug report
Bug description:
With the addition of cold exits, the "optimized trace length" histogram now includes the exit count + the optimized trace length.
While this is a useful metric for the overall executor size, it's no longer useful to see the amount of instructions that the optimizer was able to remove from the hot path.
I would suppose we break this out into two stats: the optimized trace length and the exit count.
Does this make sense, @markshannon?
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Activity
mdboom commentedon Mar 14, 2024
Additionally, this hides data since the histogram maxes out at 8,192 (MAX_TRACE_LENGTH), and trace_length + exit_counts is frequently over that.
markshannon commentedon Mar 14, 2024
The trace length historgrams should show lengths in number of uops, ignoring exits.
It should be trivial to fix once #116813 is merged, as that tracks the length of trace through optimization.
mdboom commentedon Mar 14, 2024
Sure, I think it's also trivial to fix now (and restore to old behavior), by tracking these two things separately -- I mainly wanted to get your take on whether tracking them as a single number was intentional:
But if it's easier merge-wise to do this following or as part of #116813, that also makes sense.
markshannon commentedon Mar 14, 2024
I don't think it was intended. Tracking the size of the executor in bytes doesn't make a lot of sense.
pythongh-116808: Fix optimized trace length histogram
gh-116808: Fix optimized trace length histogram (GH-116827)
pythongh-116808: Fix optimized trace length histogram (pythonGH-116827)
pythongh-116808: Fix optimized trace length histogram (pythonGH-116827)
pythongh-116808: Fix optimized trace length histogram (pythonGH-116827)
hugovk commentedon Jun 15, 2024
Triage: the linked PR is merged, can this be closed?
mdboom commentedon Jun 17, 2024
Yes, it can be closed. Sorry this fell through the cracks.