From a9d178e3f446ffcf0cee981f7186b7ddf6d42362 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sun, 3 Nov 2019 16:14:08 -0800 Subject: [PATCH 1/4] Add modify! function for lookup/update/insert/delete in one go --- base/abstractdict.jl | 57 ++++++++++++++++++++++++++++++++++++++++++++ base/dict.jl | 39 ++++++++++++++++++++++++++++++ base/exports.jl | 1 + test/dict.jl | 51 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+) diff --git a/base/abstractdict.jl b/base/abstractdict.jl index 6bcae02d539dc..c1b8f79d1b3bc 100644 --- a/base/abstractdict.jl +++ b/base/abstractdict.jl @@ -465,6 +465,63 @@ function hash(a::AbstractDict, h::UInt) hash(hv, h) end +""" + modify!(f, d::AbstractDict{K, V}, key) + +Lookup and then update, insert or delete in one go without re-computing the hash. + +`f` is a callable object that must accept `Union{Some{V}, Nothing}` and return +`Union{T, Some{T}, Nothing}` where `T` is a type [`convert`](@ref)-able to the value type +`V`. The value `Some(d[key])` is passed to `f` if `haskey(d, key)`; otherwise `nothing` +is passed. If `f` returns `nothing`, corresponding entry in the dictionary `d` is removed. +If `f` returns non-`nothing` value `x`, `something(x)` is inserted to `d`. + +`modify!` returns whatever `f` returns as-is. + +# Examples +```jldoctest +julia> dict = Dict("a" => 1); + +julia> modify!(dict, "a") do val + Some(val === nothing ? 1 : something(val) + 1) + end +Some(2) + +julia> dict +Dict{String,Int64} with 1 entry: + "a" => 2 + +julia> dict = Dict(); + +julia> modify!(dict, "a") do val + Some(val === nothing ? 1 : something(val) + 1) + end +Some(1) + +julia> dict +Dict{Any,Any} with 1 entry: + "a" => 1 + +julia> modify!(_ -> nothing, dict, "a") + +julia> dict +Dict{Any,Any} with 0 entries +``` +""" +function modify!(f, dict::AbstractDict, key) + if haskey(dict, key) + val = f(Some(dict[key])) + else + val = f(nothing) + end + if val === nothing + delete!(dict, key) + else + dict[key] = something(val) + end + return val +end + function getindex(t::AbstractDict, key) v = get(t, key, secret_table_token) if v === secret_table_token diff --git a/base/dict.jl b/base/dict.jl index 8c1d762527bb8..bd1cb29123b8b 100644 --- a/base/dict.jl +++ b/base/dict.jl @@ -391,6 +391,45 @@ function setindex!(h::Dict{K,V}, v0, key::K) where V where K return h end +function modify!(f, h::Dict{K}, key0) where K + key = convert(K, key0) + if !isequal(key, key0) + throw(ArgumentError("$(limitrepr(key0)) is not a valid key for type $K")) + end + + # Ideally, to improve performance for the case that requires + # resizing, we should use something like `ht_keyindex` while + # keeping computed hash value and then do something like + # `ht_keyindex2!` if `f` returns non-`nothing`. + idx = ht_keyindex2!(h, key) + + age0 = h.age + if idx > 0 + @inbounds vold = h.vals[idx] + vnew = f(Some(vold)) + else + vnew = f(nothing) + end + if h.age != age0 + idx = ht_keyindex2!(h, key) + end + + if vnew === nothing + if idx > 0 + _delete!(h, idx) + end + else + if idx > 0 + h.age += 1 + @inbounds h.keys[idx] = key + @inbounds h.vals[idx] = something(vnew) + else + @inbounds _setindex!(h, something(vnew), key, -idx) + end + end + return vnew +end + """ get!(collection, key, default) diff --git a/base/exports.jl b/base/exports.jl index ca0a75d3217e1..718be011775ec 100644 --- a/base/exports.jl +++ b/base/exports.jl @@ -514,6 +514,7 @@ export mapreduce, merge!, merge, + modify!, pairs, reduce, setdiff!, diff --git a/test/dict.jl b/test/dict.jl index 1224d41bb220a..e8a0c006d73fb 100644 --- a/test/dict.jl +++ b/test/dict.jl @@ -1057,8 +1057,11 @@ end new{keytype(d), valtype(d)}(d) end end + Base.Dict(td::TestDict) = td.dict + Base.haskey(td::TestDict, key) = haskey(td.dict, key) Base.setindex!(td::TestDict, args...) = setindex!(td.dict, args...) Base.getindex(td::TestDict, args...) = getindex(td.dict, args...) + Base.delete!(td::TestDict, key) = delete!(td.dict, key) Base.pairs(D::TestDict) = pairs(D.dict) testdict = TestDict(:a=>1, :b=>2) map!(v->v-1, values(testdict)) @@ -1072,3 +1075,51 @@ end @test testdict[:b] == 1 end end + +@testset "modify!(f, ::$constructor, key)" for constructor in [ + Dict, + TestDict, +] + @testset "update" begin + dict = constructor(Dict("a" => 1)) + + @test modify!(dict, "a") do val + Some(val === nothing ? 1 : something(val) + 1) + end == Some(2) + + @test Dict(dict) == Dict("a" => 2) + end + + @testset "insert" begin + dict = constructor(Dict()) + + @test modify!(dict, "a") do val + Some(val === nothing ? 1 : something(val) + 1) + end == Some(1) + + @test Dict(dict) == Dict("a" => 1) + end + + @testset "delete" begin + dict = constructor(Dict("a" => 1)) + @test modify!(_ -> nothing, dict, "a") === nothing + @test Dict(dict) == Dict() + end + + @testset "no-op" begin + dict = constructor(Dict("a" => 1)) + @test modify!(_ -> nothing, dict, "b") === nothing + @test Dict(dict) == Dict("a" => 1) + end + + @testset "mutation inside `f`" begin + dict = constructor(Dict()) + + @test modify!(dict, "a") do val + dict["a"] = 0 + Some(val === nothing ? 1 : something(val) + 1) + end == Some(1) + + @test Dict(dict) == Dict("a" => 1) + end +end From 43fa640254f8fcdffca3ac14b9bfb15ce40559fd Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Thu, 7 Nov 2019 07:27:56 -0800 Subject: [PATCH 2/4] Apply suggestions from code review Co-Authored-By: Milan Bouchet-Valat --- base/abstractdict.jl | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/base/abstractdict.jl b/base/abstractdict.jl index c1b8f79d1b3bc..c199079e7e5f3 100644 --- a/base/abstractdict.jl +++ b/base/abstractdict.jl @@ -470,11 +470,12 @@ end Lookup and then update, insert or delete in one go without re-computing the hash. -`f` is a callable object that must accept `Union{Some{V}, Nothing}` and return -`Union{T, Some{T}, Nothing}` where `T` is a type [`convert`](@ref)-able to the value type +`f` is a callable object that must take a single `Union{Some{V}, Nothing}` argument and return +a `Union{T, Some{T}, Nothing}` value, where `T` is a type [`convert`](@ref)-able to the value type `V`. The value `Some(d[key])` is passed to `f` if `haskey(d, key)`; otherwise `nothing` is passed. If `f` returns `nothing`, corresponding entry in the dictionary `d` is removed. -If `f` returns non-`nothing` value `x`, `something(x)` is inserted to `d`. +If `f` returns non-`nothing` value `x`, `key => something(x)` is inserted or updated in `d` +(equivalent to `d[key] = something(x)` but more efficient). `modify!` returns whatever `f` returns as-is. @@ -494,7 +495,7 @@ Dict{String,Int64} with 1 entry: julia> dict = Dict(); julia> modify!(dict, "a") do val - Some(val === nothing ? 1 : something(val) + 1) + Some(something(val, 0) + 1) end Some(1) From 73e11410e31a28413ff304d884273108f20e6ca4 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Fri, 12 Nov 2021 16:13:38 -0800 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Jameson Nash Co-authored-by: Joaquim Dias Garcia --- base/abstractdict.jl | 4 ++-- base/dict.jl | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/base/abstractdict.jl b/base/abstractdict.jl index 6ec92865f2cf4..81fa3983fbcbf 100644 --- a/base/abstractdict.jl +++ b/base/abstractdict.jl @@ -530,9 +530,9 @@ julia> dict Dict{Any,Any} with 0 entries ``` """ -function modify!(f, dict::AbstractDict, key) +function modify!(f, dict::AbstractDict{K,V}, key) where {K, V} if haskey(dict, key) - val = f(Some(dict[key])) + val = f(Some{V}(dict[key])) else val = f(nothing) end diff --git a/base/dict.jl b/base/dict.jl index 4692a5cc8203b..61588225938ec 100644 --- a/base/dict.jl +++ b/base/dict.jl @@ -392,7 +392,7 @@ function setindex!(h::Dict{K,V}, v0, key::K) where V where K return h end -function modify!(f, h::Dict{K}, key0) where K +function modify!(f, h::Dict{K, V}, key0) where {K, V} key = convert(K, key0) if !isequal(key, key0) throw(ArgumentError("$(limitrepr(key0)) is not a valid key for type $K")) @@ -407,7 +407,7 @@ function modify!(f, h::Dict{K}, key0) where K age0 = h.age if idx > 0 @inbounds vold = h.vals[idx] - vnew = f(Some(vold)) + vnew = f(Some{V}(vold)) else vnew = f(nothing) end From 28900adceb1c95a65606a07f62a178e2b16f3092 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Fri, 12 Nov 2021 19:23:27 -0500 Subject: [PATCH 4/4] Return `old => new` pair --- base/abstractdict.jl | 33 ++++++++++++++++++++------------- base/dict.jl | 8 +++++--- test/dict.jl | 10 +++++----- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/base/abstractdict.jl b/base/abstractdict.jl index 11730db5a440f..d75aecefc9999 100644 --- a/base/abstractdict.jl +++ b/base/abstractdict.jl @@ -528,18 +528,22 @@ function hash(a::AbstractDict, h::UInt) end """ - modify!(f, d::AbstractDict{K, V}, key) + modify!(f, d::AbstractDict{K, V}, key) -> (old => new) Lookup and then update, insert or delete in one go without re-computing the hash. +Return a pair of `old::Union{Some,Nothing}` and `new::::Union{Some,Nothing}`. -`f` is a callable object that must take a single `Union{Some{V}, Nothing}` argument and return -a `Union{T, Some{T}, Nothing}` value, where `T` is a type [`convert`](@ref)-able to the value type -`V`. The value `Some(d[key])` is passed to `f` if `haskey(d, key)`; otherwise `nothing` -is passed. If `f` returns `nothing`, corresponding entry in the dictionary `d` is removed. -If `f` returns non-`nothing` value `x`, `key => something(x)` is inserted or updated in `d` -(equivalent to `d[key] = something(x)` but more efficient). +`f` is a callable object that must take a single `old::Union{Some, Nothing}` argument and +return a `new::Union{T, Some{T}, Nothing}` value, where `T` is a type [`convert`](@ref)-able +to the value type `V`. The value `Some(d[key])` is passed to `f` if `haskey(d, key)`; +otherwise `nothing` is passed. If `f` returns `nothing`, corresponding entry in the +dictionary `d` is removed. If `f` returns non-`nothing` value `x`, `key => something(x)` +is inserted or updated in `d` (equivalent to `d[key] = something(x)` but more efficient). -`modify!` returns whatever `f` returns as-is. +Whether `Some{V}(value)` or `Some{typeof(value)}(value)` is returned is an implementation +defined behavior. The callback function `f` must use `old === nothing` or `old isa Some` +instead of `old isa Some{valtype(d)}` unless the type of the dictionary `d` is known to +define a certain behavior. # Examples ```jldoctest @@ -548,7 +552,7 @@ julia> dict = Dict("a" => 1); julia> modify!(dict, "a") do val Some(val === nothing ? 1 : something(val) + 1) end -Some(2) +Some(1) => Some(2) julia> dict Dict{String,Int64} with 1 entry: @@ -559,13 +563,14 @@ julia> dict = Dict(); julia> modify!(dict, "a") do val Some(something(val, 0) + 1) end -Some(1) +nothing => Some(1) julia> dict Dict{Any,Any} with 1 entry: "a" => 1 julia> modify!(_ -> nothing, dict, "a") +Some("a") => nothing julia> dict Dict{Any,Any} with 0 entries @@ -573,16 +578,18 @@ Dict{Any,Any} with 0 entries """ function modify!(f, dict::AbstractDict{K,V}, key) where {K, V} if haskey(dict, key) - val = f(Some{V}(dict[key])) + old = Some{V}(dict[key]) + val = f(old) else - val = f(nothing) + old = nothing + val = f(old) end if val === nothing delete!(dict, key) else dict[key] = something(val) end - return val + return old => val end function getindex(t::AbstractDict, key) diff --git a/base/dict.jl b/base/dict.jl index 05ca7587df0d6..62dceb19bd4a8 100644 --- a/base/dict.jl +++ b/base/dict.jl @@ -407,9 +407,11 @@ function modify!(f, h::Dict{K, V}, key0) where {K, V} age0 = h.age if idx > 0 @inbounds vold = h.vals[idx] - vnew = f(Some{V}(vold)) + vold = Some{V}(vold) + vnew = f(vold) else - vnew = f(nothing) + vold = nothing + vnew = f(vold) end if h.age != age0 idx = ht_keyindex2!(h, key) @@ -428,7 +430,7 @@ function modify!(f, h::Dict{K, V}, key0) where {K, V} @inbounds _setindex!(h, something(vnew), key, -idx) end end - return vnew + return vold => vnew end """ diff --git a/test/dict.jl b/test/dict.jl index 4c3996a0ddb67..4115b0df54427 100644 --- a/test/dict.jl +++ b/test/dict.jl @@ -1218,7 +1218,7 @@ end @test modify!(dict, "a") do val Some(val === nothing ? 1 : something(val) + 1) - end == Some(2) + end == (Some(1) => Some(2)) @test Dict(dict) == Dict("a" => 2) end @@ -1228,20 +1228,20 @@ end @test modify!(dict, "a") do val Some(val === nothing ? 1 : something(val) + 1) - end == Some(1) + end == (nothing => Some(1)) @test Dict(dict) == Dict("a" => 1) end @testset "delete" begin dict = constructor(Dict("a" => 1)) - @test modify!(_ -> nothing, dict, "a") === nothing + @test modify!(_ -> nothing, dict, "a") == (Some(1) => nothing) @test Dict(dict) == Dict() end @testset "no-op" begin dict = constructor(Dict("a" => 1)) - @test modify!(_ -> nothing, dict, "b") === nothing + @test modify!(_ -> nothing, dict, "b") == (nothing => nothing) @test Dict(dict) == Dict("a" => 1) end @@ -1251,7 +1251,7 @@ end @test modify!(dict, "a") do val dict["a"] = 0 Some(val === nothing ? 1 : something(val) + 1) - end == Some(1) + end == (nothing => Some(1)) @test Dict(dict) == Dict("a" => 1) end