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

Backport Subtype: some performance tuning. (#56007) #56117

Draft
wants to merge 1 commit into
base: release-1.10
Choose a base branch
from

Conversation

charleskawczynski
Copy link
Contributor

The main motivation of this PR is to fix #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

  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> @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)

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 charleskawczynski changed the title Subtype: some performance tuning. (#56007) Backport Subtype: some performance tuning. (#56007) Oct 11, 2024
@nsajko
Copy link
Contributor

nsajko commented Oct 11, 2024

I believe you want to target branch backports-release-1.10 (PR #55746).

@charleskawczynski
Copy link
Contributor Author

I believe you want to target branch backports-release-1.10 (PR #55746).

Ah, yes, thank you.

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