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

Signature cache for overloads for faster compilation #6667

Closed
wants to merge 11 commits into from

Conversation

ehsantn
Copy link
Collaborator

@ehsantn ehsantn commented Jan 28, 2021

This adds a signature cache to overloads, to return the output signature quickly if the same input type set is already compiled. This is especially useful for the inlined overload cases where there is no other cache in the code path (dispatcher has a cache to avoid recompilation). My benchmark shows over 30% faster compilation time (hard to simplify and share the benchmark unfortunately).

This adds a signature cache overloads to return the
output signature quickly if the same input types are already
compiled.
@ehsantn ehsantn changed the title Signature cache for overloads for faster compilation [WIP] Signature cache for overloads for faster compilation Jan 28, 2021
@esc esc added 3 - Ready for Review 4 - Waiting on author Waiting for author to respond to review labels Jan 28, 2021
@esc
Copy link
Member

esc commented Jan 28, 2021

@ehsantn thanks for submitting this. I have added it to the queue for review. Incidentally, there seem to be some typing related failures on public CI.

@ehsantn ehsantn changed the title [WIP] Signature cache for overloads for faster compilation Signature cache for overloads for faster compilation Feb 2, 2021
@ehsantn
Copy link
Collaborator Author

ehsantn commented Feb 2, 2021

Any ideas on how to test this? Seems tricky since the effect is lower compilation time and the internal data structures are not exposed.

@sklam
Copy link
Member

sklam commented Feb 9, 2021

I think we can test by counting the number of compilation triggered for an overload using the event recording added in #6626

@stuartarchibald
Copy link
Contributor

Also have to wonder how this will impact target overloads? I think this is assuming there's a 1:1 map between a signature and compiled impl?

@ehsantn
Copy link
Collaborator Author

ehsantn commented Feb 11, 2021

I think we can test by counting the number of compilation triggered for an overload using the event recording added in #6626

This is a good idea. I'll give it a try when I get a chance.

@ehsantn
Copy link
Collaborator Author

ehsantn commented Feb 11, 2021

Also have to wonder how this will impact target overloads? I think this is assuming there's a 1:1 map between a signature and compiled impl?

Yes, this is assuming a 1:1 map between signature and implementation, but it's flexible. For example, it doesn't save the signature for the overloads that change the signature. I don't quite know how the target overloads will look like, but target could be part of the cache key or we can just turn this off for those overloads.

@ehsantn
Copy link
Collaborator Author

ehsantn commented Feb 16, 2021

@sklam I just tried adding a test using the new events feature. However, the numba:compile event is not triggered unless if actual compilation is necessary. The numba:compiler_lock even shows some difference, but doesn't seem reliable for testing (a small change somewhere else can make the test fail). We could add new events, but seems too much to me just for testing this feature. I think manual testing may be enough for this PR.

@ehsantn
Copy link
Collaborator Author

ehsantn commented Feb 23, 2021

I'm not able to reproduce the CI error locally. Any thoughts?

@stuartarchibald
Copy link
Contributor

I'm not able to reproduce the CI error locally. Any thoughts?

It'll probably be something very specific about the test slice and the order the tests are run on that CI combination.

@github-actions
Copy link

This pull request is marked as stale as it has had no activity in the past 3 months. Please respond to this comment if you're still interested in working on this. Many thanks!

@github-actions github-actions bot added the stale Marker label for stale issues. label Mar 19, 2023
@github-actions github-actions bot added the abandoned - stale PRs automatically closed due to stale status label Mar 27, 2023
@github-actions github-actions bot closed this Mar 27, 2023
@dlee992
Copy link
Contributor

dlee992 commented Sep 15, 2023

I am interested in continuing this PR. Will test and re-push if available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - Waiting on author Waiting for author to respond to review abandoned - stale PRs automatically closed due to stale status stale Marker label for stale issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants