Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Rethink @requires dependencies #476

Open
rafaqz opened this issue Jan 20, 2022 · 2 comments · May be fixed by #478
Open

Rethink @requires dependencies #476

rafaqz opened this issue Jan 20, 2022 · 2 comments · May be fixed by #478

Comments

@rafaqz
Copy link

rafaqz commented Jan 20, 2022

The startup time for Blink.jl almost entirely consists of the requires blocks here.

The WebSockets block is unnecessary because its a dependency anyway. Mux.jl is a small package that shares many dependencies. It's much faster to depend on it here than to include it in requires.

Blink.jl already depends on WebIO.jl - the code should just live in Blink.jl where it can be precompiled.

All up this reduces the package load time of Blink.jl (on my laptop) from 5 seconds to 1 second.

See: JuliaGizmos/Blink.jl#288

If people agree I can PR Blink and WebIO with the changes.

@rafaqz rafaqz linked a pull request Jan 20, 2022 that will close this issue
@halleysfifthinc
Copy link
Contributor

I will second rethinking @require dependencies. The current issue which has me looking at WebIO and related packages is dependencies on WebSockets.jl, which is forcing HTTP.jl to be downgraded to <v1.

I might be misreading the code, but afaict, WebSockets isn't actually a hard dependency. So it should be in the [extras] section of the Project file, along with IJulia and Blink.

If removing @requireing dependencies, I would vote for putting glue code into glue packages. Since Blink already depends on WebIO, a glue package wouldn't be needed in that case. A partial solution might be to put the glue code into subpackages that are still @required. This is supposed to be precompilable, which might help package load time, but I haven't tried this and its not currently in use in any registered packages (code search).

@ufechner7
Copy link

What is missing to get this merged?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants