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

Add Libdl.LazyLibrary #50074

Merged
merged 1 commit into from
Jul 27, 2023
Merged

Add Libdl.LazyLibrary #50074

merged 1 commit into from
Jul 27, 2023

Conversation

staticfloat
Copy link
Member

This provides an in-base mechanism to handle chained library dependencies. In essence, the LazyLibrary object can be used anywhere a pointer to a library can be used (dlopen, dlsym, ccall, etc...) but it delays loading the library (and its recursive dependencies) until it is actually needed.

This is the foundational piece needed to upgrade JLLs to lazily-load their libraries. In this new scheme, JLLs would generally lose all executable code and consist of nothing more than LazyLibrary definitions.

base/libdl.jl Outdated Show resolved Hide resolved
base/libdl.jl Outdated Show resolved Hide resolved
@staticfloat
Copy link
Member Author

Honestly, I'm not 100% certain I even want dlclose() defined. I don't like the refcount, I don't like that dlopen(ll) opens multiple libraries and dlclose(ll) only closes one, etc...

I think if you want to close the libraries, you just need to be responsible for closing them yourself.

@staticfloat staticfloat force-pushed the sf/lazy_libraries branch 4 times, most recently from d902efc to 1b4ea85 Compare June 6, 2023 17:15
@staticfloat
Copy link
Member Author

Ugh, running into that windows problem where I can't rm() a .DLL I've opened.

@vchuravy
Copy link
Member

vchuravy commented Jun 9, 2023

x-ref: libuv/libuv#3839

@staticfloat
Copy link
Member Author

I'm dodging the issue now by not re-using libccalltest but instead creating new (extremely small) libraries for a simple lazy dependency test.

@staticfloat
Copy link
Member Author

Converting to draft while I figure out how to make the names lazier, so that we can use these with artifact"" without needing an __init__() method. Current idea; embed calls to artifact"" inside of LazyString objects.

@staticfloat
Copy link
Member Author

I worked this over with Valentin today, and we arrived to the current, rather overly-clever solution. It features a few very useful implementation details:

  1. It stores its name as an AbstractString, and introduces LazyStringFunc which allows users (such as JLLs) to store a callback that gets evaluated during dlopen() time. This allows us to store library paths such as artifact"foo/lib/libfoo.so", which will lower down into a runtime Artifact lookup.

  2. In order to provide the ccall() magic at all stages, we need to fill out the jl_lazy_library_type and jl_libdl_dlopen_func fields. We do this in C when restoring an image, but when bootstrapping we needed a way to feed these values in just after they've been defined. We opted to Base.unsafe_store!(cglobal(:jl_lazy_library_type, Any), LazyLibrary) at the toplevel, which is either genius or madness, I'm unsure which.

Things I'm still a little unhappy about:

  1. The on_load_callback invocations happen before we've updated ll.handle, which means that if you try to ccall((:foo, libfoo), ...) from within your callback, you end up in an infinite loop in dlopen() (you are supposed to use the raw handle that is passed into the callback). This is obviously nonideal as it's the most natural way of writing those callbacks (and allows re-use of existing functions); I'm thinking we should update ll.handle first, but have some other way of knowing that we have not yet finished initialization; perhaps with an atomic bool? I'm open to suggestions here.

I have a second branch that builds on top of this to make many stdlibs lazy, most importantly OpenBLAS_jll, and while it's a bigger change, it manages to get startup time down from 122ms -> 95ms on my macbook pro, and time to first matmul stays steady at 123ms.

@staticfloat staticfloat marked this pull request as ready for review June 10, 2023 03:04
src/runtime_ccall.cpp Outdated Show resolved Hide resolved
base/libdl.jl Outdated
"""
LazyStringFunc(f) = LazyString(_LazyStringFunc(f))

struct _LazyStringFunc <: AbstractString
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
struct _LazyStringFunc <: AbstractString
struct _LazyStringFunc

base/libdl.jl Outdated Show resolved Hide resolved
base/libdl.jl Outdated Show resolved Hide resolved
base/libdl.jl Outdated Show resolved Hide resolved
base/libdl.jl Show resolved Hide resolved
@staticfloat staticfloat force-pushed the sf/lazy_libraries branch 3 times, most recently from f320209 to ef7a3f5 Compare July 26, 2023 14:27
@staticfloat
Copy link
Member Author

I've finished the Jameson refactor, and rebased my stdlib working branch on top of it, verifying it still works. We should be good to go here, as long as this goes green.

@staticfloat staticfloat added the merge me PR is reviewed. Merge when all tests are passing label Jul 26, 2023
base/libdl.jl Outdated Show resolved Hide resolved
@staticfloat staticfloat removed the merge me PR is reviewed. Merge when all tests are passing label Jul 26, 2023
@staticfloat
Copy link
Member Author

@vtjnash So our change to no longer only call dlopen() when the type is a LazyLibrary now causes some of the ccall tests to fail, as they now throw a MethodError instead of a TypeError. Do we want this? It's a little inconsistent with e.g. cglobal().

@vtjnash
Copy link
Member

vtjnash commented Jul 26, 2023

We should define that method and have it call a TypeError

@staticfloat
Copy link
Member Author

staticfloat commented Jul 26, 2023

We should define that method and have it call a TypeError

So you mean something like dlopen(::Any) = throw(TypeError(...))?

@vtjnash
Copy link
Member

vtjnash commented Jul 27, 2023

yes, like that

This provides an in-base mechanism to handle chained library
dependencies.  In essence, the `LazyLibrary` object can be used anywhere
a pointer to a library can be used (`dlopen`, `dlsym`, `ccall`, etc...)
but it delays loading the library (and its recursive dependencies) until
it is actually needed.

This is the foundational piece needed to upgrade JLLs to lazily-load
their libraries.  In this new scheme, JLLs would generally lose all
executable code and consist of nothing more than `LazyLibrary`
definitions.  As a helper type, we create a `BundledLazyLibraryPath`
type that is stringable and calculates `Sys.BINDIR`-relative paths
at runtime, and can be wrapped in a `LazyString` to act as an
`AbstractString` if required.

Many thanks to vtjnash for the Jameson Lock Makeover (TM).
@staticfloat staticfloat added the merge me PR is reviewed. Merge when all tests are passing label Jul 27, 2023
@staticfloat staticfloat mentioned this pull request Jul 27, 2023
2 tasks
@vchuravy vchuravy merged commit 4fd68e8 into master Jul 27, 2023
2 checks passed
@vchuravy vchuravy deleted the sf/lazy_libraries branch July 27, 2023 11:07
@brenhinkeller brenhinkeller added stdlib Julia's standard library ffi foreign function interfaces, ccall, etc. and removed merge me PR is reviewed. Merge when all tests are passing labels Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ffi foreign function interfaces, ccall, etc. stdlib Julia's standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants