Skip to content

gh-142145: Avoid timing measurements in quadratic behavior test#143105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
colesbury merged 3 commits into from
Dec 24, 2025
Merged

gh-142145: Avoid timing measurements in quadratic behavior test #143105

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc9cf09
gh-142145: Avoid timing measurements in quadratic behavior test
colesbury Dec 23, 2025
506e978
Lint
colesbury Dec 23, 2025
ec362b3
Changes from review
colesbury Dec 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 30 additions & 18 deletions Lib/test/test_minidom.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,7 +2,6 @@

import copy
import pickle
import time
import io
from test import support
import unittest
Expand DownExpand Up@@ -178,23 +177,36 @@ def testAppendChild(self):
def testAppendChildNoQuadraticComplexity(self):
impl = getDOMImplementation()

newdoc = impl.createDocument(None, "some_tag", None)
top_element = newdoc.documentElement
children = [newdoc.createElement(f"child-{i}") for i in range(1, 2 ** 15 + 1)]
element = top_element

start = time.monotonic()
for child in children:
element.appendChild(child)
element = child
end = time.monotonic()

# This example used to take at least 30 seconds.
# Conservative assertion due to the wide variety of systems and
# build configs timing based tests wind up run under.
# A --with-address-sanitizer --with-pydebug build on a rpi5 still
# completes this loop in <0.5 seconds.
self.assertLess(end - start, 4)
def work(n):
doc = impl.createDocument(None, "some_tag", None)
element = doc.documentElement
total_calls = 0

# Count attribute accesses as a proxy for work done
def getattribute_counter(self, attr):
nonlocal total_calls
total_calls += 1
return object.__getattribute__(self, attr)

with support.swap_attr(Element, "__getattribute__", getattribute_counter):
Copy link
Member

@gpsheadgpsheadDec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
ContributorAuthor

@colesburycolesburyDec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

for _ in range(n):
child = doc.createElement("child")
element.appendChild(child)
element = child
return total_calls

# Doubling N should not ~quadruple the work.
w1 = work(1024)
w2 = work(2048)
w3 = work(4096)

self.assertGreater(w1, 0)
r1 = w2 / w1
r2 = w3 / w2
self.assertLess(
max(r1, r2), 3.2,
msg=f"Possible quadratic behavior: work={w1,w2,w3} ratios={r1,r2}"
)

def testSetAttributeNodeWithoutOwnerDocument(self):
# regression test for gh-142754
Expand Down
Loading