Manually split string(a::Union{Char, String, SubString{String}, Symbol}...)
#43939
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
…ol}...)` 4 is one too many for automatic Union-splitting. Poor inference in this method accounts for more than 1000 invalidations.
Keno
reviewed
Jan 26, 2022
@JeffBezanson is it worth increasing the limit if the argument is annotated with an explicit Union? |
We union-split up to 4: Line 122 in 3f0ae6e and so I think this method will be inferred accurately if the union-splitting happens. I rather guess we somehow fail to infer |
|
The inference was blocked as we set julia/base/compiler/typelimits.jl Lines 7 to 8 in 3f0ae6e I guess it's not a good idea to increase them? |
vtjnash
approved these changes
Jan 26, 2022
LilithHafner
pushed a commit
to LilithHafner/julia
that referenced
this pull request
Feb 22, 2022
…ol}...)` (JuliaLang#43939) * Manually split `string(a::Union{Char, String, SubString{String}, Symbol}...)` 4 is one too many for automatic Union-splitting. Poor inference in this method accounts for more than 1000 invalidations.
timholy
added a commit
that referenced
this pull request
Mar 7, 2022
This is follow-on work to #43939
LilithHafner
pushed a commit
to LilithHafner/julia
that referenced
this pull request
Mar 8, 2022
…ol}...)` (JuliaLang#43939) * Manually split `string(a::Union{Char, String, SubString{String}, Symbol}...)` 4 is one too many for automatic Union-splitting. Poor inference in this method accounts for more than 1000 invalidations.
timholy
added a commit
that referenced
this pull request
Mar 9, 2022
This is follow-on work to #43939
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.
4 is one too many for automatic Union-splitting. Poor inference in
this method accounts for more than 2000 invalidations.
Fix (at least partial) for #43157. I don't remember my numbers before this change, but if my vague memory is correct this knocks ~1s off the
using Plotstime and ~2s offdisplay(plot(rand(10))). It also brings the number of invalidations down from 2622 to 599.The regression appears to have been introduced in #41992---a very innocuous-seeming change. There still may be more TTFP to fix, though.