-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Cache dlpath on MacOS because it gets really slow #58409
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
Conversation
num_images = ccall(:_dyld_image_count, Cint, ()) | ||
# start at 1 instead of 0 to skip self | ||
for i in 1:num_images-1 # 0-based | ||
name = unsafe_string(ccall(:_dyld_get_image_name, Cstring, (UInt32,), i)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're already relying on the structure of the macos handle here, so this whole thing can just be
dlpath_cache[(@ccall _dyld_get_image_header(i::UInt32)::Ptr{Cvoid}) >> 5]
with a corresponding mask of the bottom bit in the lookup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(To avoid the extra dlopen)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into that first but the handle returned by _dyld_get_image_header
seems to not be the expected heap allocated handles. They're in the 0x1xx
range where the handle we have in julia is in the 0x3xx
range. Does that sound right?
At least I did a two stage approach trying that first then falling back to the current way, and it never seemed to fast path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Note the shift by 5
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside to whether cache or not I can't seem to get this right. It always hits the fallthrough error
const dlpath_cache_lock = Base.ReentrantLock()
const dlpath_cache = Dict{UInt,String}()
function dlpath(handle::Ptr{Cvoid})
@lock dlpath_cache_lock begin
key = UInt(handle) & ~UInt(1) # mask the flags bit. see `makeDlHandle`
path = get(dlpath_cache, key, nothing)
if path !== nothing
return path
else
num_images = ccall(:_dyld_image_count, Cint, ())
# start at 1 instead of 0 to skip self
for i in 1:num_images-1 # 0-based
h = UInt(@ccall _dyld_get_image_header(i::UInt32)::Ptr{Cvoid}) << 5
haskey(dlpath_cache, h) && continue
name = unsafe_string(ccall(:_dyld_get_image_name, Cstring, (UInt32,), i))
key2 = UInt(h)
dlpath_cache[key] = name
if key2 == key
return name
end
end
end
end
error("dlpath: could not find path for handle $(handle)")
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're applying the shift in the wrong direction. You either need to << 5
the key or >> 5
the image header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>> 5
on the image header gives quite different values
function dlpath(handle::Ptr{Cvoid})
@lock dlpath_cache_lock begin
key = UInt(handle) & ~UInt(1) # mask the flags bit. see `makeDlHandle`
path = get(dlpath_cache, key, nothing)
if path !== nothing
return path
else
num_images = ccall(:_dyld_image_count, Cint, ())
# start at 1 instead of 0 to skip self
for i in 1:num_images-1 # 0-based
key2 = UInt(@ccall _dyld_get_image_header(i::UInt32)::Ptr{Cvoid}) >> 5
haskey(dlpath_cache, key2) && continue
name = unsafe_string(ccall(:_dyld_get_image_name, Cstring, (UInt32,), i))
dlpath_cache[key2] = name
@show key, key2
if key2 == key
return name
end
end
end
end
error("dlpath: could not find path for handle $(handle)")
end
(key, key2) = (0x0000000397000010, 0x000000000d60bd00)
(key, key2) = (0x0000000397000010, 0x000000000d73a180)
(key, key2) = (0x0000000397000010, 0x000000000e226680)
(key, key2) = (0x0000000397000010, 0x000000000d72f200)
(key, key2) = (0x0000000397000010, 0x000000000d655300)
(key, key2) = (0x0000000397000010, 0x000000000d73a880)
(key, key2) = (0x0000000397000010, 0x00000000123a3b80)
(key, key2) = (0x0000000397000010, 0x000000000f449880)
Also tried
key2 = (UInt(@ccall _dyld_get_image_header(i::UInt32)::Ptr{Cvoid}) >> 5) & ~UInt(1)
I found myself trying << 5
because the values are quite similar..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I don't know. I guess it's possible that dyld changed the way it computes handles - the open version is somewhat old.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the cache and implement this inside jl_pathname_for_handle
as Keno directed
Isn't avoiding the iteration still part of the point? |
I'd recommend just removing |
For example, the single use of it recently added in Base would wrong for anyone that statically links their own binaries using libjulia or renames it. That use could be using Lines 344 to 345 in 58daba4
|
This will also already be fixed by #58405 without needing this hack |
I had assumed JLLWrappers also used So yeah #58405 seems to go in the right direction (but not fully, there's still 20 |
Just FYI @staticfloat |
And just adding that |
JLL inits can get really slow on MacOS when lots of libs are loaded because
jl_pathname_for_handle
is expensive on MacOS.This caches the work within
dlpath
on MacOS.master
PR