-
Notifications
You must be signed in to change notification settings - Fork 109
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
TypeTree speedup shiftindicies #1744
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
note I left the slow version right now to do correctness tests, which already found a bug locally (which have fixed). Going to run it on the whole test suite, then disable the check once it comes back valid. |
wsmoses
force-pushed
the
fastshiftind
branch
from
February 19, 2024 23:42
c3a1ae4
to
510a1b4
Compare
wsmoses
force-pushed
the
fastshiftind
branch
from
February 19, 2024 23:51
510a1b4
to
587a834
Compare
Also note this fix hopefully changes the asymptotic behavior of ShiftIndices from |
wsmoses
force-pushed
the
fastshiftind
branch
from
February 20, 2024 01:35
f5c4c4d
to
e88fe95
Compare
Considering I don’t see ShiftIndices more on the flame graph I’m going to
call this a win.
I’ll see what I can do about the Julia function. mergeTypeTree is the C API
and primarily used in that function so I think it’s the next most
meaningful place we should be able to reduce overhead from. Essentially
because it should be not terrible to make it called only once. CheckedOrIn
however doesn’t have any obvious speed ups I remember.
…On Tue, Feb 20, 2024 at 2:02 AM Manuel Drehwald ***@***.***> wrote:
@vchuravy <https://github.com/vchuravy> If already at it, could you
please also profile
#1641 <#1641> if you have a few
minutes? My server never had the permissions for vtune. My new Laptop is
reasonable otherwise, so I can also set it up myself
—
Reply to this email directly, view it on GitHub
<#1744 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJTUXDS4ZNE23F3VAS4O43YURC7BAVCNFSM6AAAAABDQF76POVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJTGU4TKMZVGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1743