[3.12] GH-107674: Avoid allocating boxed ints for sys.settrace line events (GH-107780)

[miss-islington]

(cherry picked from commit 37d8b90)

Co-authored-by: Mark Shannon [email protected]

• Issue: sys.settrace dramatic slowdown in 3.12 #107674

Closes#107674

[markshannon]

@Yhg1s

[Yhg1s]

How important is this performance improvement? Does it solve all of the reported slowdown, or just some of it? I don't really want pure performance improvements, especially of this complexity, in the bug fix stage, let alone between rc1 and rc2...

[markshannon]

How important is this performance improvement?

It largely depends on whether coverage.py moves to using PEP 669 for 3.12. It is does, then this is not important at all.

Does it solve all of the reported slowdown, or just some of it?

I don't know as yet. I'll get back to you when I have some numbers.

I don't really want pure performance improvements, especially of this complexity, in the bug fix stage, let alone between rc1 and rc2...

That's understandable.

[ambv]

Closing and re-opening to retrigger CLA checks. Sorry for the noise.

[Yhg1s]

How important is this performance improvement?

It largely depends on whether coverage.py moves to using PEP 669 for 3.12. It is does, then this is not important at all.

Does it solve all of the reported slowdown, or just some of it?

I don't know as yet. I'll get back to you when I have some numbers.

Where are we at with this? Does it make sense to try and get this in now, or are you comfortable making these changes in 3.12.1 if they turn out to be necessary?

[markshannon]

It definitely helps performance of ``sys.settrace`, but performance is not what it was for 3.11.

TBH, I think the best way to forward is to help coverage.py move to using sys.monitoring which will give a huge speedup and not worry about the performance of sys.settrace based approaches.

So, I'd leave it for now.

[vstinner]

@markshannon@Yhg1s: What's the status of this change? Python 3.12.0 was released without this optimization.

I don't think that coverage.py was ported to PEP 669 yet.

[tarpas]

The overhead is killing the utility of https://github.com/tarpas/pytest-testmon. It would be great if it's addressed.

[serhiy-storchaka]

Was this PR forgotten? Should it be merged or closed?

[hugovk]

I don't think that coverage.py was ported to PEP 669 yet.

coverage.py now has support for PEP 669 / sys.monitoring for line coverage, but not yet for branch coverage: coveragepy/coveragepy#1746 (comment)

Does this still need backporting to 3.12?

cc @nedbat

[patrys]

I just found this PR while investigating why pytest was taking several minutes longer after upgrading to Python 3.12. While coverage.py has experimental support for sys.monitoring, it's incomplete and not enabled by default. In our case it won't help until it works with branch coverage, so we're forced to endure the slow path.

Another tool affected is pytest-testmon, which programmatically uses coverage.py as a library. I'm not even sure it supports COVERAGE_CORE=sysmon as setting it did not make a difference.

[nedbat]

Definitely coverage.py still needs sys.settrace to work well for all versions of Python. We are working on sys.monitoring support, but have not found a solution for branch coverage.

[nedbat]

I haven't heard people complaining about the speed on 3.12, but looking at my own coverage runs, I can see a definitely slowdown from 3.11 to 3.12.

[nedbat]

I ran some benchmarks:

cov	proj	python3.10	python3.11	python3.12	python3.13	3.13 vs 3.12	3.12 vs 3.11	3.11 vs 3.10
761	mashumaro	59.0s	56.9s	62.6s	69.4s	111%	110%	96%
761	pygments	26.5s	25.1s	56.4s	38.6s	68%	224%	95%
761	mypy	109.3s	105.7s	114.1s	102.2s	90%	108%	97%

(the last three columns would make more sense in reverse order).

[vstinner]

What's the status of this PR created 1 year 1/2 ago?

[Yhg1s]

@markshannon I received some questions about whether this can get applied to 3.12 (because people are still upgrading to 3.12 even though it's nearing the end of its bugfix state). What do you think?

[markshannon]

Boxing and unboxing ints is expensive, so avoiding the box/unbox pair is definitely going to help sys.settrace.

This is going to slow down other users of LINE events a tiny bit, but those should be DISABLEing events anyway if they care about performance.

We'll fix this properly for 3.15 with tagged ints, but this fix looks like a worthwhile change in the meantime.