Skip to content

Conversation

@cspotcode
Copy link
Collaborator

@cspotcode cspotcode commented Feb 16, 2023

This is related to #1504. Where the other PR is a draft which experimented with many ideas, this PR includes only the changes that had the biggest positive improvements. It also adds the benchmark runner and collision benchmark.

I think this PR is suitable to be merged as-is.

Here are my benchmark results. I ran it against each commit twice, I'm not sure if that was necessary but I was worried that my laptop would always run the first commit fastest because it was not hot (overheating) yet. That may have been wrong or unnecessary.

Command Mean [s] Min [s] Max [s] Relative
0066a6ae _check_for_collision: optimize broadphase 30.941 ± 0.864 30.330 31.552 1.01 ± 0.03
6c023813 get_adjusted_hit_box: list comprehension into tuple is faster than generator into tuple 36.530 ± 0.386 36.257 36.803 1.19 ± 0.01
5748dec0 get_adjusted_hit_box: inline rotate_point() call, avoid attribute lookups on self and math 36.603 ± 0.262 36.418 36.789 1.19 ± 0.01
0066a6ae _check_for_collision: optimize broadphase 30.664 ± 0.028 30.644 30.683 1.00
6c023813 get_adjusted_hit_box: list comprehension into tuple is faster than generator into tuple 36.045 ± 0.347 35.800 36.291 1.18 ± 0.01
5748dec0 get_adjusted_hit_box: inline rotate_point() call, avoid attribute lookups on self and math 40.900 ± 5.103 37.291 44.508 1.33 ± 0.17

@cspotcode cspotcode changed the title Collision detection optimizations Collision detection optimizations, benchmark runner, and collision detection benchmark Feb 16, 2023
@pvcraven
Copy link
Member

I think the speed-ups would be good. I'm inclined to merge those in.

I'm not sure about leaving the benchmarks directory in.

  • Either leave it in and add a readme documenting how it works, using it, sample output
  • Or just drop it if documenting it is too burdensome.

@einarf
Copy link
Member

einarf commented Feb 17, 2023

As discussed on discord we merge this as is. The contents in the benchmarking directory we can change to using standard cProfile using the build in profiler in arcade. Optionally just use simple timeit. Instead of bringing in large examples we can test individual functions only.

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