Backport Subtype: some performance tuning. (#56007)#56117
Closed
charleskawczynski wants to merge 1 commit intoJuliaLang:backports-release-1.10from
Closed
Backport Subtype: some performance tuning. (#56007)#56117charleskawczynski wants to merge 1 commit intoJuliaLang:backports-release-1.10from
charleskawczynski wants to merge 1 commit intoJuliaLang:backports-release-1.10from
Conversation
The main motivation of this PR is to fix JuliaLang#55807. dc689fe tries to remove the slow `may_contain_union_decision` check by re-organizing the code path. Now the fast path has been removed and most of its optimization has been integrated into the preserved slow path. Since the slow path stores all inner ∃ decisions on the outer most R stack, there might be overflow risk. aee69a4 should fix that concern. The reported MWE now becomes ```julia 0.000002 seconds 0.000040 seconds (105 allocations: 4.828 KiB, 52.00% compilation time) 0.000023 seconds (105 allocations: 4.828 KiB, 49.36% compilation time) 0.000026 seconds (105 allocations: 4.828 KiB, 50.38% compilation time) 0.000027 seconds (105 allocations: 4.828 KiB, 54.95% compilation time) 0.000019 seconds (106 allocations: 4.922 KiB, 49.73% compilation time) 0.000024 seconds (105 allocations: 4.828 KiB, 52.24% compilation time) ``` Local bench also shows that 72855cd slightly accelerates `OmniPackage.jl`'s loading ```julia julia> @time using OmniPackage 20.525278 seconds (25.36 M allocations: 1.606 GiB, 8.48% gc time, 12.89% compilation time: 77% of which was recompilation) 19.527871 seconds (24.92 M allocations: 1.593 GiB, 8.88% gc time, 15.13% compilation time: 82% of which was recompilation) ```
charleskawczynski
changed the title
Member
|
I believe you want to target branch backports-release-1.10 (PR #55746). |
Contributor
Author
Ah, yes, thank you. |
DilumAluthge
changed the base branch from
release-1.10
to
backports-release-1.10
Member
|
I've changed the base branch from @charleskawczynski Can you rebase (against |
Contributor
Author
Hi @DilumAluthge, it's been a while since I've looked at this. I don't really recall the changes, and I'm not up-to-date on how to resolve the rebase conflicts. Please feel free to take over this PR if it's important to you, or close if you are looking to clean things up. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
The main motivation of this PR is to fix #55807.
dc689fe tries to remove the slow
may_contain_union_decisioncheck by re-organizing the code path. Now the fast path has been removed and most of its optimization has been integrated into the preserved slow path.Since the slow path stores all inner ∃ decisions on the outer most R stack, there might be overflow risk.
aee69a4 should fix that concern.
The reported MWE now becomes
Local bench also shows that 72855cd slightly accelerates
OmniPackage.jl's loading