lazy LibGit2 initialization #28113
lazy LibGit2 initialization #28113
Conversation
stdlib/LibGit2/src/LibGit2.jl
Outdated
| @@ -972,12 +972,12 @@ function set_ssl_cert_locations(cert_loc) | |||
| Cint(Consts.SET_SSL_CERT_LOCATIONS), cert_file, cert_dir) | |||
| end | |||
|
|
|||
| function __init__() | |||
| @check ccall((:git_libgit2_init, :libgit2), Cint, ()) | |||
| function initialize() | |||
| REFCOUNT[] = 1 | |||
There was a problem hiding this comment.
Threads.atomic_cas!(REFCOUNT, 1) == 0 || return?
There was a problem hiding this comment.
The one and only call to it does the check. If I put the call in more places I'd factor it this way.
There was a problem hiding this comment.
That check doesn't have the atomic_cas!. I don't particularly know why this is using thread-safe primitives right now, but that's how the code is written, so it seems like we are supposed to carry that through all of the say
There was a problem hiding this comment.
Ah, I see what you're getting at. The old __init__ didn't bother since it's presumed that there's no raciness at startup but later on there could be. Sure, may as well respect the existing thread-safety.
stdlib/LibGit2/src/LibGit2.jl
Outdated
| @@ -972,12 +972,12 @@ function set_ssl_cert_locations(cert_loc) | |||
| Cint(Consts.SET_SSL_CERT_LOCATIONS), cert_file, cert_dir) | |||
| end | |||
|
|
|||
| function __init__() | |||
| @check ccall((:git_libgit2_init, :libgit2), Cint, ()) | |||
| function initialize() | |||
There was a problem hiding this comment.
might be worth adding @noinline here, since the compiler might see this is small enough to be feasible to inline, (even though that'll make us generate worse code)
|
Yes, this helps a lot! |
By how much? Any measurements? |
|
For me it reduces the time for |
StefanKarpinski
force-pushed
the
sk/lazy-lg2-init
branch
from
9fa52ad to
723dbf5
Compare
I did this the lazy way but it seems to work fine. The right way would be to put a
REFCOUNT[] == 0 && initialize()at the top of every function that calls into thelibgit2library. @JeffBezanson, does this get rid of that 0.1s of startup you were seeing on your system?