Skip to content
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.

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

Speed up pytest execution wall time by ~37%. #383

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

fischman
Copy link
Contributor

Before: 644 passed in 19.89s.
After: 644 passed in 12.47s.

Running with python -m pytest --durations=5 showed this test case was a huge outlier, and timing its execution showed a huge disparity between the time TechnicalDrawing thought it took to run and the time its caller saw et up. Dropping the Builder context makes these times lineup.

Tested the individual test case speed-up with hyperfine -- 'python -m pytest tests/test_drafting.py::TestTechnicalDrawing::test_basic_drawing' which reports before & after of:

Benchmark 1: python -m pytest tests/test_drafting.py::TestTechnicalDrawing::test_basic_drawing
  Time (mean ± σ):      9.488 s ±  0.534 s    [User: 9.525 s, System: 1.478 s]
  Range (min … max):    9.117 s … 10.929 s    10 runs

Benchmark 1: python -m pytest tests/test_drafting.py::TestTechnicalDrawing::test_basic_drawing
  Time (mean ± σ):      1.951 s ±  0.040 s    [User: 2.292 s, System: 1.111 s]
  Range (min … max):    1.871 s …  2.020 s    10 runs

I dug a little into why it was so much slower under the builder and the answer seems to be ~13k calls to deepcopy, but I haven't dug deeper yet to figure out if there's a way to "transfer" to the builder rather than making copies.

(full disclosure: the next-slowest test case is test_pack.py that I added very recently; I'll be looking into speeding that up next :) )

Before: `644 passed in 19.89s`.
After: `644 passed in 12.47s`.

Running with `python -m pytest --durations=5` showed this test case
was a huge outlier, and timing its execution showed a huge disparity
between the time `TechnicalDrawing` thought it took to run and the
time its caller saw et up. Dropping the Builder context makes these
times lineup.

Tested the individual test case speed-up with `hyperfine -- 'python -m pytest tests/test_drafting.py::TestTechnicalDrawing::test_basic_drawing'`
which reports before & after of:

```
Benchmark 1: python -m pytest tests/test_drafting.py::TestTechnicalDrawing::test_basic_drawing
  Time (mean ± σ):      9.488 s ±  0.534 s    [User: 9.525 s, System: 1.478 s]
  Range (min … max):    9.117 s … 10.929 s    10 runs

Benchmark 1: python -m pytest tests/test_drafting.py::TestTechnicalDrawing::test_basic_drawing
  Time (mean ± σ):      1.951 s ±  0.040 s    [User: 2.292 s, System: 1.111 s]
  Range (min … max):    1.871 s …  2.020 s    10 runs
```

I dug a little into why it was so much slower under the builder and
the answer seems to be ~13k calls to deepcopy, but I haven't dug
deeper yet to figure out if there's a way to "transfer" to the builder
rather than making copies.
@MatthiasJ1
Copy link
Contributor

This is something we may want to open an issue for, I've noticed certain operations take an order of magnitude longer when inside a Builder context.

@fischman
Copy link
Contributor Author

This is something we may want to open an issue for, I've noticed certain operations take an order of magnitude longer when inside a Builder context.

Filed #384 which also has a pointer to a self-contained repro test case, in case that's of interest.

@gumyr gumyr merged commit fa3139f into gumyr:dev Nov 15, 2023
8 checks passed
@fischman
Copy link
Contributor Author

fischman commented Nov 15, 2023

(full disclosure: the next-slowest test case is test_pack.py that I added very recently; I'll be looking into speeding that up next :) )

That is now #386.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants