gh-142145: Avoid timing measurements in quadratic behavior test

[colesbury]

Count the number of Element attribute accesses as a proxy for work done. With double the amount of work, a ratio of 2.0 indicates linear scaling and 4.0 quadratic scaling. Use 3.2 as an intermediate threshold.

• Issue: Remove quadratic behavior in node ID cache clearing #142145

[colesbury]

The context for this is that I'm trying to get the full test suite in a state where we can run it under TSan. TSan tends to be much slower, so timing tests are even more flaky.

Let me know what you think about this appraoch.

For context, I used ChatGPT to generate the test and then edited it. (It was pretty good, but not perfect).

[sethmlarson]

Thanks @colesbury, did you happen to run this test with the commit in question reverted to see that this test would indeed catch quadratic behavior in number of accesses?

[colesbury]

Yes, if you revert the fix then it fails with:

======================================================================
FAIL: testAppendChildNoQuadraticComplexity (test.test_minidom.MinidomTest.testAppendChildNoQuadraticComplexity)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/sgross/cpython/Lib/test/test_minidom.py", line 207, in testAppendChildNoQuadraticComplexity
    self.assertLess(
    ~~~~~~~~~~~~~~~^
        max(r1, r2), 3.2,
        ^^^^^^^^^^^^^^^^^
        msg=f"Possible quadratic behavior: work={w1,w2,w3} ratios={r1,r2}"
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
AssertionError: 3.9883495145631067 not less than 3.2 : Possible quadratic behavior: work=(1060864, 4218880, 16826368) ratios=(3.976833976833977, 3.9883495145631067)

[sethmlarson]

LGTM, thanks @colesbury!

[gpshead]

Elide the model's explanatory comment about the old no longer present code.

[gpshead]

why remove this? i realize the test shouldn't be "slow" now, but the intent of adding it was to mark it as a test that may not be suitable for slow builds and loaded systems. some of our buildbots run with the cpu resource disabled for that reason. it's more of a performance test no matter how it is implemented.

for tests where it isn't platform specific and we just need something in our support tiers to catch an unlikely regression the issue, resource tags save effort.

[colesbury]

I'll add it back. I removed it because my understanding was that "cpu" was for tests that were CPU-heavy, and now this test is very fast (~30 ms in a debug build).

[gpshead]

doing this is... gross, but likely works. your assertGreater(w1, 0) below covers the case when it stops doing so. LGTM

the other thought i was having as i did the simple "raise the timeout to 4s" to unblock the earlier ones was that we don't need absolute times at all. measuring relative time taken as we increase the amount of work to ensure that it scales - similar to this scaling measurement of attribute accesses - would be sufficient and should produce similar results on super slow builds or heavily loaded systems. it would still be time based (so maybe use resource.getrusage(resource.RUSAGE_SELF).ru_utime instead of time. APIs... though i suspect rusage is low-res) but far less "works on my system"

we have a smattering of other timing based cpu performancy tests in the repo. i don't think we've codified a reusable best practice.

[colesbury]

measuring relative time taken as we increase the amount of work ... would be sufficient

It'd be great to have some sort of standard best practice for this. If resource.getrusage(resource.RUSAGE_SELF).ru_utime isn't too affected by other processes, that would be good.

Some other things I was thinking about:

1. Count cycles (via perf_event_open?)
2. Count bytecodes executed (by instrumenting ceval.c in debug builds)