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

Increase collection comparison performance #836

Merged

Conversation

stephenswat
Copy link
Member

@stephenswat stephenswat commented Feb 3, 2025

Although the collection comparison code is not performance-critical, it is still very, very slow. In part, this is because it is written very naively. This commit improves the performance of this code by several orders of magnitude by sorting the collections, and by avoiding checks for elements which have already been matched at a lower uncertainty level. After these optimisations, it turned out that the Iterators-based progress bar was the performance-limiting factor, so I had to remove that as well.

We can now do GPU-vs-CPU physics comparisons for pile-up 200 events in a fraction of a second. 😄

@stephenswat stephenswat added improvement Improve an existing feature performance Performance-relevant changes physics Physics performance monitoring examples Changes to the examples labels Feb 3, 2025
@stephenswat stephenswat force-pushed the impr/binary_search_comparators branch from e8d8720 to b4ffd16 Compare February 3, 2025 16:12
Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

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

I'm on board with this, since the code is indeed painfully slow at the moment.

Just, you know... make it pass the CI first. 😉

Although the collection comparison code is not performance-critical, it
is still very, very slow. In part, this is because it is written very
naively. This commit improves the performance of this code by several
orders of magnitude by sorting the collections, and by avoiding checks
for elements which have already been matched at a lower uncertainty
level. After these optimisations, it turned out that the Iterators-based
progress bar was the performance-limiting factor, so I had to remove
that as well.
@stephenswat stephenswat force-pushed the impr/binary_search_comparators branch from b4ffd16 to 56c5dad Compare February 4, 2025 08:19
Copy link

sonarqubecloud bot commented Feb 4, 2025

Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

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

Sure!

@krasznaa krasznaa merged commit c68908f into acts-project:main Feb 4, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples Changes to the examples improvement Improve an existing feature performance Performance-relevant changes physics Physics performance monitoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants