improve precompilation coverage #33006
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
- move the place where --trace-compile outputs precompile statement to a location that catches more cases - tweak the REPL code to be more amenable to precompilation in light of - instead of trying to encode all the rules where the precompile emitter fails (#28808) just try to precompile and do nothing if it fails.
|
Just wondering: could this also explain JuliaLang/PackageCompiler.jl#188, which came up after PackageCompiler switched from using SnoopCompile to directly using |
|
I guess it could, yeah. |
|
Barring comments, I plan to merge this in a day or so. |
c42f
approved these changes
Aug 25, 2019
| @error "Failed to precompile $statement" | ||
| rethrow() | ||
| # See #28808 | ||
| # @error "Failed to precompile $statement" |
36 tasks
timholy
approved these changes
Aug 26, 2019
| @@ -765,15 +765,21 @@ setup_interface( | |||
| repl::LineEditREPL; | |||
| # those keyword arguments may be deprecated eventually in favor of the Options mechanism | |||
| hascolor::Bool = repl.options.hascolor, | |||
| extra_repl_keymap::Union{Dict,Vector{<:Dict}} = repl.options.extra_keymap | |||
| extra_repl_keymap::Any = repl.options.extra_keymap | |||
There was a problem hiding this comment.
@nospecialize? Just asserting that it's an Any doesn't prevent the compiler from trying to specialize it. Same for the one below.
There was a problem hiding this comment.
I think this takes care of that:
Line 50 in 5e75cfe
KristofferC
added a commit
that referenced
this pull request
Aug 26, 2019
- move the place where --trace-compile outputs precompile statement to a location that catches more cases - tweak the REPL code to be more amenable to precompilation in light of - instead of trying to encode all the rules where the precompile emitter fails (#28808) just try to precompile and do nothing if it fails. (cherry picked from commit c0478d8)
KristofferC
added a commit
that referenced
this pull request
Aug 26, 2019
- move the place where --trace-compile outputs precompile statement to a location that catches more cases - tweak the REPL code to be more amenable to precompilation in light of - instead of trying to encode all the rules where the precompile emitter fails (#28808) just try to precompile and do nothing if it fails. (cherry picked from commit c0478d8)
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.
--trace-compileoutputs precompile statement to a location that catches more cases (it now sits next to where SnoopCompile outputs statements)fails (Precompile statement emitter sometimes emits invalid statements #28808) just try to precompile and do nothing if it fails.
With this, startup time is back to normal and the time to prompt (measured with the patch in #32971 (comment)) is significantly improved, although not yet completely back to 1.1 levels:
1.1
1.2
PR
Probably good enough to say that it fixes #32971 after being backported