From 30e68cd67d6c6c08e55dcbaf5073863907bb974f Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sun, 9 May 2021 23:04:08 -0400 Subject: [PATCH 01/14] Don't skip clusters test --- .github/workflows/test-broken.yml | 35 ------------------------------- test/test_dict.jl | 14 ++----------- 2 files changed, 2 insertions(+), 47 deletions(-) delete mode 100644 .github/workflows/test-broken.yml diff --git a/.github/workflows/test-broken.yml b/.github/workflows/test-broken.yml deleted file mode 100644 index 6df87b0..0000000 --- a/.github/workflows/test-broken.yml +++ /dev/null @@ -1,35 +0,0 @@ -name: Run broken tests - -on: - push: - branches: - - master - tags: '*' - pull_request: - -jobs: - test: - runs-on: ubuntu-latest - strategy: - matrix: - julia-version: ['nightly'] - fail-fast: false - name: Broken test Julia ${{ matrix.julia-version }} - steps: - - uses: actions/checkout@v2 - - name: Setup julia - uses: julia-actions/setup-julia@v1 - with: - version: ${{ matrix.julia-version }} - # Use `JULIA_PKG_SERVER` mitigation implemented in julia-buildpkg: - - uses: julia-actions/julia-buildpkg@v1 - - # Maybe some useful information: - - run: julia -e 'using InteractiveUtils; versioninfo()' - - run: lscpu - - - uses: julia-actions/julia-runtest@v1 - id: test - env: - CI: "false" - continue-on-error: true diff --git a/test/test_dict.jl b/test/test_dict.jl index 797779e..b621202 100644 --- a/test/test_dict.jl +++ b/test/test_dict.jl @@ -4,8 +4,6 @@ using ConcurrentCollections using ConcurrentCollections.Implementations: clusters using Test -const IS_CI = lowercase(get(ENV, "CI", "false")) == "true" - @testset for npairs in [2, 100] @testset for Key in [Int8, Int32, Int64], Value in [Int] d = ConcurrentDict{Key,Value}() @@ -52,11 +50,7 @@ const IS_CI = lowercase(get(ENV, "CI", "false")) == "true" end @testset "clusters" begin - if VERSION >= v"1.7-" && IS_CI - @info "skipping `clusters` (known bug)" - else - @test length(clusters(d)::Vector{UnitRange{Int}}) > 0 - end + @test length(clusters(d)::Vector{UnitRange{Int}}) > 0 end end @@ -84,11 +78,7 @@ const IS_CI = lowercase(get(ENV, "CI", "false")) == "true" @test "001" โˆ‰ sort!(collect(keys(d))) end @testset "clusters" begin - if VERSION >= v"1.7-" && IS_CI - @info "skipping `clusters` (known bug)" - else - @test length(clusters(d)::Vector{UnitRange{Int}}) > 0 - end + @test length(clusters(d)::Vector{UnitRange{Int}}) > 0 end end end From 4212dcd50f7f3459522bde4e75e4f037f84ede9a Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sun, 9 May 2021 23:13:35 -0400 Subject: [PATCH 02/14] DEBUG: at-show typeof(d.slots) length(d.slots) --- test/test_dict.jl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test_dict.jl b/test/test_dict.jl index b621202..3c8c385 100644 --- a/test/test_dict.jl +++ b/test/test_dict.jl @@ -50,6 +50,7 @@ using Test end @testset "clusters" begin + @show typeof(d.slots) length(d.slots) @test length(clusters(d)::Vector{UnitRange{Int}}) > 0 end end @@ -78,6 +79,7 @@ using Test @test "001" โˆ‰ sort!(collect(keys(d))) end @testset "clusters" begin + @show typeof(d.slots) length(d.slots) @test length(clusters(d)::Vector{UnitRange{Int}}) > 0 end end From 51a80e1e6d2f3da7632a1e14f7aa9eeda24fd953 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sun, 9 May 2021 23:14:48 -0400 Subject: [PATCH 03/14] (rerun) From b51099fcc49276116b246f60cbb00e0ce97c6062 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sun, 9 May 2021 23:14:55 -0400 Subject: [PATCH 04/14] (rerun) From 6b20ddc395c246e74d9b4f96e3633e549d2832aa Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Wed, 12 May 2021 22:38:31 -0400 Subject: [PATCH 05/14] Make the bug more reproducible --- test/test_dict.jl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/test_dict.jl b/test/test_dict.jl index 3c8c385..06dcae6 100644 --- a/test/test_dict.jl +++ b/test/test_dict.jl @@ -4,6 +4,10 @@ using ConcurrentCollections using ConcurrentCollections.Implementations: clusters using Test +# make garbages +garbages = [Vector{Int}(undef, 2^rand(1:5)) for _ in 1:100000] +@show length(garbages) + @testset for npairs in [2, 100] @testset for Key in [Int8, Int32, Int64], Value in [Int] d = ConcurrentDict{Key,Value}() From 614c3731207d316beaa28289fe0e9db3276acc31 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sun, 9 May 2021 23:21:47 -0400 Subject: [PATCH 06/14] Use `@julia.write_barrier` --- src/atomicsutils.jl | 2 ++ src/dict.jl | 19 +++++++++++++------ src/utils.jl | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/atomicsutils.jl b/src/atomicsutils.jl index a7a1990..3c70363 100644 --- a/src/atomicsutils.jl +++ b/src/atomicsutils.jl @@ -35,6 +35,8 @@ end else fptr = fieldpointer(obj, field) vint = UInt(pointer_from_objref(value)) + julia_write_barrier(value) + julia_write_barrier(obj, value) GC.@preserve obj value begin UnsafeAtomics.store!(fptr, vint, order) end diff --git a/src/dict.jl b/src/dict.jl index ca5ca36..f75bbc9 100644 --- a/src/dict.jl +++ b/src/dict.jl @@ -274,11 +274,13 @@ end allocate_slot(::AbstractVector{<:AbstractPair}) = nothing -@inline cas_slot!(slotref, new_slot, key) = cas_slot!(slotref, new_slot, key, NoValue()) +@inline cas_slot!(slotref, new_slot, root, key) = + cas_slot!(slotref, new_slot, root, key, NoValue()) @inline function cas_slot!( slotref::InlinedSlotRef{Slot}, ::Nothing, + root, key::KeyUnion{Key}, value, ) where {Key,Value,Slot<:AbstractPair{Key,Value}} @@ -300,8 +302,10 @@ allocate_slot(::AbstractVector{<:AbstractPair}) = nothing if Slot <: InlinedPair newkeyint = uint_from(Inlined{KeyUnion{Key}}(key)) elseif key isa Moved{Key} - ref = forceheap(Ref(key)) + ref = Ref(key) newkeyint = UInt(pointer_from_objref(ref)) + julia_write_barrier(ref) + julia_write_barrier(root, ref) else newkeyint = UInt(_pointer_from_objref(key)) end @@ -349,6 +353,7 @@ allocate_slot(::AbstractVector{Slot}) where {Slot<:Ref} = forceheap(Slot()) @inline function cas_slot!( slotref::RefSlotRef{Slot}, new_slot::Slot, + root, key, value, ) where {P,Slot<:Ref{P}} @@ -359,6 +364,8 @@ allocate_slot(::AbstractVector{Slot}) where {Slot<:Ref} = forceheap(Slot()) GC.@preserve new_slot begin fu = UnsafeAtomics.cas!(Ptr{typeof(nu)}(ptr), ou, nu) end + julia_write_barrier(new_slot) + julia_write_barrier(root, new_slot) return fu == ou end @@ -462,7 +469,7 @@ function ConcurrentCollections.modify!( if reply isa Keep return reply elseif reply isa Union{Nothing,Delete} - if cas_slot!(slotref, new_slot, Deleted()) + if cas_slot!(slotref, new_slot, slots, Deleted()) ndeleted = Threads.atomic_add!(dict.ndeleted, 1) + 1 approx_len = dict.nadded[] - ndeleted if approx_len < length(slots) รท 2 @@ -471,7 +478,7 @@ function ConcurrentCollections.modify!( return reply end else - if cas_slot!(slotref, new_slot, nsk, something(reply)) + if cas_slot!(slotref, new_slot, slots, nsk, something(reply)) if sk isa Empty Threads.atomic_add!(dict.nadded, 1) end @@ -661,7 +668,7 @@ function migrate_impl!( continue elseif sk isa Empty # Mark that this slot is not usable anymore - if !cas_slot!(slotref, allocate_slot(slots), MovedEmpty()) + if !cas_slot!(slotref, allocate_slot(slots), slots, MovedEmpty()) @goto reload end stop_on_empty == Val(true) && return Stopped(i, nadded) @@ -675,7 +682,7 @@ function migrate_impl!( if sk isa Moved key = sk.key else - if !cas_slot!(slotref, allocate_slot(slots), Moved(sk), sv) + if !cas_slot!(slotref, allocate_slot(slots), slots, Moved(sk), sv) @goto reload end key = sk diff --git a/src/utils.jl b/src/utils.jl index 512eeaf..0f0afea 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -176,6 +176,43 @@ const _FALSE_ = Ref(false) return x end +@generated function julia_write_barrier(args::Vararg{Any,N}) where {N} + pointer_exprs = map(1:N) do i + :(_pointer_from_objref(args[$i])) + end + jlp = "{} addrspace(10)*" + llvm_args = string.("%", 0:N-1) + word = "i$(Base.Sys.WORD_SIZE)" + entry_sig = join(word .* llvm_args, ", ") + ptrs = string.("%ptr", 0:N-1) + wb_sig = join("$jlp " .* ptrs, ", ") + inttoptr = join( + (ptrs .* "_tmp = inttoptr $word " .* llvm_args .* " to {}*\n") .* + (ptrs .* " = addrspacecast {}* " .* ptrs .* "_tmp to $jlp"), + "\n", + ) + IR = ( + """ + define void @entry($entry_sig) #0 { + top: + $inttoptr + call void ($jlp, ...) @julia.write_barrier($wb_sig) + ret void + } + + declare void @julia.write_barrier($jlp, ...) #1 + + attributes #0 = { alwaysinline } + attributes #1 = { inaccessiblememonly norecurse nounwind } + """, + "entry", + ) + quote + $(Expr(:meta, :inline)) + Base.llvmcall($IR, Cvoid, NTuple{N,Ptr{Cvoid}}, $(pointer_exprs...)) + end +end + @generated allocate_singleton_ref(::Type{T}) where {T} = Ref{Any}(T.instance) @inline function pointer_from_singleton(::T) where {T} From e054c9fee4360e6dae93ab2617fd73adad89db07 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Wed, 12 May 2021 22:43:00 -0400 Subject: [PATCH 07/14] (rerun) From 27da5267ba7cf5a210c9d55c6e4ed4cf24738010 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Wed, 12 May 2021 22:43:12 -0400 Subject: [PATCH 08/14] (rerun) From e116b34090cc5d16954015634e54895bdfab5180 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Wed, 12 May 2021 22:43:28 -0400 Subject: [PATCH 09/14] (rerun) From ed8d3d888d95efadad15e1e6889ae2b68e428992 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Wed, 12 May 2021 22:41:19 -0400 Subject: [PATCH 10/14] Revert "DEBUG: at-show typeof(d.slots) length(d.slots)" This reverts commit 4212dcd50f7f3459522bde4e75e4f037f84ede9a. --- test/test_dict.jl | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/test_dict.jl b/test/test_dict.jl index 06dcae6..03e0a0a 100644 --- a/test/test_dict.jl +++ b/test/test_dict.jl @@ -54,7 +54,6 @@ garbages = [Vector{Int}(undef, 2^rand(1:5)) for _ in 1:100000] end @testset "clusters" begin - @show typeof(d.slots) length(d.slots) @test length(clusters(d)::Vector{UnitRange{Int}}) > 0 end end @@ -83,7 +82,6 @@ garbages = [Vector{Int}(undef, 2^rand(1:5)) for _ in 1:100000] @test "001" โˆ‰ sort!(collect(keys(d))) end @testset "clusters" begin - @show typeof(d.slots) length(d.slots) @test length(clusters(d)::Vector{UnitRange{Int}}) > 0 end end From c4acf8257478bff1785aa05fdcb822461d5c8404 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Wed, 12 May 2021 22:41:54 -0400 Subject: [PATCH 11/14] Revert "Make the bug more reproducible" This reverts commit 6b20ddc395c246e74d9b4f96e3633e549d2832aa. --- test/test_dict.jl | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/test_dict.jl b/test/test_dict.jl index 03e0a0a..b621202 100644 --- a/test/test_dict.jl +++ b/test/test_dict.jl @@ -4,10 +4,6 @@ using ConcurrentCollections using ConcurrentCollections.Implementations: clusters using Test -# make garbages -garbages = [Vector{Int}(undef, 2^rand(1:5)) for _ in 1:100000] -@show length(garbages) - @testset for npairs in [2, 100] @testset for Key in [Int8, Int32, Int64], Value in [Int] d = ConcurrentDict{Key,Value}() From 8a342d0f23656cf058f04c581f37cedead404c9f Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Wed, 12 May 2021 22:45:59 -0400 Subject: [PATCH 12/14] (rerun) From 9376a398676a065d7b850757fd3aae93165124b4 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Wed, 12 May 2021 22:46:58 -0400 Subject: [PATCH 13/14] (rerun) From 39b2ff0327c4f4661083fa35cb77e71f2a98cd5b Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Wed, 12 May 2021 22:47:06 -0400 Subject: [PATCH 14/14] (rerun)