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.

Sign up for GitHub

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

Jump to bottom

[REPL] improve quality of precompile script #55210

Merged
merged 2 commits into from
Jul 25, 2024
Merged

[REPL] improve quality of precompile script #55210

merged 2 commits into from
Jul 25, 2024

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Jul 22, 2024

This rewrites the REPL precompile script to no longer rely upon ugly hacks for a cross-process copy and instead supports doing the precompile wholly within in the current process.

@vtjnash vtjnash added REPL Julia's REPL (Read Eval Print Loop) compiler:precompilation Precompilation of modules labels Jul 22, 2024
@vtjnash vtjnash requested a review from vchuravy July 22, 2024 18:55
@IanButterworth
Copy link
Member

On Apple Silicon CI this makes REPL precompilation go from 58s to 25s.
That's a surprisingly large improvement!

@vtjnash
use sentinal for REPL_MODULE_REF
91ee1f4 
Working with #undef is unnecessarily trickier and more limited than
using a token, which allows more flexibility with updates.
@vtjnash vtjnash force-pushed the jn/REPL-precompile-corrected branch from 69cb6a8 to f09e95e Compare July 23, 2024 16:29
@vtjnash
[REPL] improve quality of precompile script
5871df1 
Launching a subprocess is buggy and error-prone, as partially evidenced
by the variety of bugs that needed to be fixed.
@vtjnash vtjnash force-pushed the jn/REPL-precompile-corrected branch from f09e95e to 5871df1 Compare July 24, 2024 19:51
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Jul 24, 2024
@giordano giordano merged commit 0bb650c into master Jul 25, 2024
6 of 8 checks passed
@giordano giordano deleted the jn/REPL-precompile-corrected branch July 25, 2024 10:32
@giordano giordano removed the merge me PR is reviewed. Merge when all tests are passing label Jul 25, 2024
@IanButterworth
Copy link
Member

IanButterworth commented Jul 26, 2024 •
edited
Loading

This broke Revise.

% julia +nightly
ERROR: LoadError: InitError: FieldError: type Nothing has no field ast_transforms
Stacktrace:
  [1] getproperty(x::Nothing, f::Symbol)
    @ Base ./Base.jl:49
  [2] __init__()
    @ Revise ~/.julia/packages/Revise/MHio8/src/packagedef.jl:1334
...

https://github.com/timholy/Revise.jl/blob/4ad68c211b9f31db739e451e5f67d842a0f0592e/src/packagedef.jl#L1333-L1335

@IanButterworth IanButterworth mentioned this pull request Jul 26, 2024
Closed
@christiangnrd christiangnrd mentioned this pull request Aug 2, 2024
Merged
@serenity4 serenity4 mentioned this pull request Aug 12, 2024
Merged
lazarusA pushed a commit to lazarusA/julia that referenced this pull request Aug 17, 2024
@vtjnash @lazarusA
[REPL] improve quality of precompile script (JuliaLang#55210)
f4b3177 
This rewrites the REPL precompile script to no longer rely upon ugly
hacks for a cross-process copy and instead supports doing the precompile
wholly within in the current process.
@IanButterworth IanButterworth mentioned this pull request Sep 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vchuravy vchuravy vchuravy approved these changes

Assignees
No one assigned
Labels
compiler:precompilation Precompilation of modules REPL Julia's REPL (Read Eval Print Loop)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vtjnash @IanButterworth @vchuravy @giordano