Skip to content
Open
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import copy
import pickle
import time
import io
from test import support
import unittest
Expand Down Expand 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

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
Contributor Author

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