Skip to content

Commit f3b53f0

Browse files
KristofferCDrvi
authored andcommitted
use a Dict instead of an IdDict for caching of the cwstring for Windows env variables (JuliaLang#52758)
Should fix JuliaLang#52711. My analysis of the invalidation is as follows: We added code to cache the conversion to `cwstring` in env handling on Windows (JuliaLang#51371): ```julia const env_dict = IdDict{String, Vector{UInt16}}() function memoized_env_lookup(str::AbstractString) ... env_dict[str] = cwstring(str) ... end function access_env(onError::Function, str::AbstractString) var = memoized_env_lookup(str) ... end ``` Since `IdDict` has `@nospecialize` on `setindex!` we compile this method: ```julia setindex!(::IdDict{String, Vector{UInt16}}, ::Any, ::Any) ``` which has an edge to: ```julia convert(Type{Vector{Int64}}, Any}) ``` But then StaticArrays comes along and adds a method ```julia convert(::Type{Array{T, N}}, ::StaticArray) ``` which invalidates the `setindex!` (due to the edge to `convert`) which invalidates the whole env handling on Windows which causes 4k other methods downstream to be invalidated, in particular, the artifact string macro which causes a significant delay in the next jll package you load after loading StaticArrays. There should be no performance penalty to this since strings already does a hash for their `objectid`. (cherry picked from commit b7c24ed)
1 parent e328137 commit f3b53f0

File tree

1 file changed

+7
-6
lines changed

1 file changed

+7
-6
lines changed

base/env.jl

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,22 @@
33
if Sys.iswindows()
44
const ERROR_ENVVAR_NOT_FOUND = UInt32(203)
55

6-
const env_dict = IdDict{String, Vector{Cwchar_t}}()
6+
const env_dict = Dict{String, Vector{Cwchar_t}}()
77
const env_lock = ReentrantLock()
88

99
function memoized_env_lookup(str::AbstractString)
1010
# Windows environment variables have a different format from Linux / MacOS, and previously
1111
# incurred allocations because we had to convert a String to a Vector{Cwchar_t} each time
1212
# an environment variable was looked up. This function memoizes that lookup process, storing
1313
# the String => Vector{Cwchar_t} pairs in env_dict
14-
var = get(env_dict, str, nothing)
15-
if isnothing(var)
16-
var = @lock env_lock begin
17-
env_dict[str] = cwstring(str)
14+
@lock env_lock begin
15+
var = get(env_dict, str, nothing)
16+
if isnothing(var)
17+
var = cwstring(str)
18+
env_dict[str] = var
1819
end
20+
return var
1921
end
20-
var
2122
end
2223

2324
_getenvlen(var::Vector{UInt16}) = ccall(:GetEnvironmentVariableW,stdcall,UInt32,(Ptr{UInt16},Ptr{UInt16},UInt32),var,C_NULL,0)

0 commit comments

Comments
 (0)