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

Move startup code to new Client.jl stdlib #51350

Closed
wants to merge 9 commits into from
Closed

Move startup code to new Client.jl stdlib #51350

wants to merge 9 commits into from

Conversation

vchuravy
Copy link
Member

Based on a brief discussion during triage with Jeff.

The idea is that we could move most of the "Client" things into it's
own seperated stdlib in preperation for it to become a standalone library
this would allow us to move InteractiveUtils, REPL, and Pkg out of the sysimg
immediatly.

There is still a few kinks with generating the precompile statements for Client.

@brenhinkeller brenhinkeller added the stdlib Julia's standard library label Sep 16, 2023
@KristofferC
Copy link
Sponsor Member

Does this really have to be a real standard library that you can add and get in your Manifest files, autocomplete with the package manager etc? It feels this should be implemented in a way that makes it just an implementation detail that is completely opaque to e.g. the package manager.

@vchuravy
Copy link
Member Author

Does this really have to be a real standard library that you can add and get in your Manifest files, autocomplete with the package manager etc? It feels this should be implemented in a way that makes it just an implementation detail that is completely opaque to e.g. the package manager.

Once the mythical juliac exists yes, ideally we would just compile this into a program. I mostly did it as a stdlib to make my life easier, pkgimg is right now the only way to compile and cache the necessary code.

@vchuravy vchuravy added speculative Whether the change will be implemented is speculative excision Removal of code from Base or the repository labels Sep 16, 2023
@vchuravy
Copy link
Member Author

vchuravy@odin ~/b/julia> hyperfine --warmup 3 "julia -e 'exit()'"
Benchmark 1: julia -e 'exit()'
  Time (mean ± σ):     135.6 ms ±   0.9 ms    [User: 66.2 ms, System: 79.9 ms]
  Range (min … max):   134.0 ms … 138.0 ms    21 runs
vchuravy@odin ~/b/julia> hyperfine --warmup 3 "./julia -e 'exit()'"
Benchmark 1: ./julia -e 'exit()'
  Time (mean ± σ):      1.287 s ±  0.011 s    [User: 1.206 s, System: 0.541 s]
  Range (min … max):    1.271 s …  1.306 s    10 runs

So we need to make package loading just another 10x faster xD

Interesting data-point after looking at the load with Tracy.
REPL is responsible for all most all invalidations. ~300

We are also currently eagerly loading Pkg and we need #51189 first.

Here I am using Client.jl to contain the precompile statements for Pkg, REPL, InteractiveUtils.
But really we need to move those to their respective packages so that this can load them on demand instead of unconditionally.

@oscardssmith
Copy link
Member

What is the REPL invalidating?

@vchuravy
Copy link
Member Author

I haven't checked :) This was me just getting distracted from my actual work and trying to see how far I can get.

@vchuravy
Copy link
Member Author

I am not sure it's worth moving this code out. Certainly a good lesson in how we eventually want to handle REPL precompilation.

The original allure was that we could remove from Base the special knowledge about packages like REPL, Distributed, but really just replaced it with a special package called Client xD.

Loading the Client lib on my sysimage (after the dependency inversion) is about 14.5ms extra.

@vchuravy
Copy link
Member Author

vchuravy@odin ~/b/julia> JULIA_MINIMAL_CLIENT=1 hyperfine --warmup 3 "./julia -e 'exit()'"
Benchmark 1: ./julia -e 'exit()'
  Time (mean ± σ):     112.0 ms ±   2.6 ms    [User: 62.5 ms, System: 92.1 ms]
  Range (min … max):   109.9 ms … 120.5 ms    26 runs
 
vchuravy@odin ~/b/julia> JULIA_MINIMAL_CLIENT=0 hyperfine --warmup 3 "./julia -e 'exit()'"
Benchmark 1: ./julia -e 'exit()'
  Time (mean ± σ):     129.6 ms ±   2.7 ms    [User: 89.4 ms, System: 190.6 ms]
  Range (min … max):   127.3 ms … 135.9 ms    23 runs

So yeah we lose 15ms for little gain.

@JeffBezanson
Copy link
Sponsor Member

JeffBezanson commented Sep 18, 2023

I think there is value in moving the code even if it is all still in the default sysimg. It's cleaner design and should also reduce the minimum size image you can generate with PackageCompiler and filter_stdlibs. If we dig we can probably find more code that can move to Client.jl.

@tecosaur
Copy link
Contributor

tecosaur commented Oct 7, 2023

I think I've recently become more interested in this PR after the triage decision that StyledStrings should become a new stdlib, instead of part of base. In #50726 (comment) I show some banner variants for smaller TTYs that I am interested in PRing that are written using the string styling macro (code: https://ix.io/4ImW).

If client.jl is made a stdlib, perhaps it could depend on StyledStrings.jl and use that resizable-banner code?

@vchuravy
Copy link
Member Author

vchuravy commented Oct 7, 2023

Agreed to that would be nice.

I am currently not pursuing this PR since the latency implications were a bit on the high side.aybe we can reevaluate this again, but for now I want to finish moving out the rest of the stdlibs first

@vchuravy vchuravy closed this Mar 11, 2024
@vchuravy vchuravy deleted the vc/driver branch March 11, 2024 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
excision Removal of code from Base or the repository speculative Whether the change will be implemented is speculative stdlib Julia's standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants