Skip to content

Commit b0db1a2

Browse files
authored
Performance boost on map operations + bugfix (#15362)
Add the two important optimization paths for negative-map projection: value-side projection for map fetch/get and shape-side projection for map put/update. Keep regression coverage for projected negative maps.
1 parent 35eeab3 commit b0db1a2

3 files changed

Lines changed: 201 additions & 13 deletions

File tree

lib/elixir/lib/module/types/descr.ex

Lines changed: 83 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3394,9 +3394,22 @@ defmodule Module.Types.Descr do
33943394
{tag, fields, negs}, acc ->
33953395
{value, bdd} = map_pop_key_bdd(tag, fields, key)
33963396

3397-
negs
3398-
|> map_split_negative_key(key, value, bdd)
3399-
|> Enum.reduce(acc, fn {value, _}, acc -> union(value, acc) end)
3397+
case map_split_negative_pairs_key(negs, key) do
3398+
:empty ->
3399+
acc
3400+
3401+
negative ->
3402+
value =
3403+
if map_pair_projection_keeps_full_fst?(negative, bdd) do
3404+
value
3405+
else
3406+
negs
3407+
|> map_split_negative_key(key, value, bdd)
3408+
|> Enum.reduce(none(), fn {value, _}, acc -> union(value, acc) end)
3409+
end
3410+
3411+
union(value, acc)
3412+
end
34003413
end)
34013414
catch
34023415
:open -> {true, term()}
@@ -3405,6 +3418,39 @@ defmodule Module.Types.Descr do
34053418
pop_optional_static(value)
34063419
end
34073420

3421+
defp map_split_negative_pairs_key(negs, key) do
3422+
Enum.reduce_while(negs, [], fn
3423+
bdd_leaf(:open, empty), _acc when is_fields_empty(empty) ->
3424+
{:halt, :empty}
3425+
3426+
bdd_leaf(tag, fields), neg_acc ->
3427+
{:cont, [map_pop_key_bdd(tag, fields, key) | neg_acc]}
3428+
end)
3429+
end
3430+
3431+
# Projection shortcuts for the pair-shaped map split below. These are
3432+
# existential checks: if at least one remaining-map sample avoids all negative
3433+
# remaining maps, the full key-value side survives; dually, if at least one
3434+
# key-value sample avoids all negative key values, the full map-shape side
3435+
# survives. If neither shortcut applies, we fall back to the regular split.
3436+
defp map_pair_projection_keeps_full_fst?(negative, bdd) do
3437+
neg_bdd =
3438+
Enum.reduce(negative, :bdd_bot, fn {_neg_value, neg_bdd}, acc ->
3439+
map_union(neg_bdd, acc)
3440+
end)
3441+
3442+
not map_empty?(map_difference(bdd, neg_bdd))
3443+
end
3444+
3445+
defp map_pair_projection_keeps_full_snd?(negative, value) do
3446+
neg_values =
3447+
Enum.reduce(negative, none(), fn {neg_value, _neg_bdd}, acc ->
3448+
union(neg_value, acc)
3449+
end)
3450+
3451+
not empty?(difference(value, neg_values))
3452+
end
3453+
34083454
defp map_split_negative_key(negs, key, value, bdd) do
34093455
map_split_negative(negs, value, bdd, fn neg_tag, neg_fields ->
34103456
case fields_take(key, neg_fields) do
@@ -3861,10 +3907,35 @@ defmodule Module.Types.Descr do
38613907

38623908
{tag, fields, negs}, {value, bdd} ->
38633909
{fst, snd} = map_pop_key_bdd(tag, fields, key)
3864-
pairs = map_split_negative_key(negs, key, fst, snd)
38653910

3866-
{maybe_union(value, fn -> Enum.reduce(pairs, none(), &union(elem(&1, 0), &2)) end),
3867-
Enum.reduce(pairs, bdd, &map_union(elem(&1, 1), &2))}
3911+
case map_split_negative_pairs_key(negs, key) do
3912+
:empty ->
3913+
{value, bdd}
3914+
3915+
negative ->
3916+
keep_fst? =
3917+
value == nil or map_pair_projection_keeps_full_fst?(negative, snd)
3918+
3919+
keep_snd? = map_pair_projection_keeps_full_snd?(negative, fst)
3920+
3921+
pairs =
3922+
if keep_fst? and keep_snd?,
3923+
do: [],
3924+
else: map_split_negative_key(negs, key, fst, snd)
3925+
3926+
{maybe_union(value, fn ->
3927+
if keep_fst? do
3928+
fst
3929+
else
3930+
Enum.reduce(pairs, none(), &union(elem(&1, 0), &2))
3931+
end
3932+
end),
3933+
if keep_snd? do
3934+
map_union(bdd, snd)
3935+
else
3936+
Enum.reduce(pairs, bdd, &map_union(elem(&1, 1), &2))
3937+
end}
3938+
end
38683939
end)
38693940

38703941
if bdd == :bdd_bot do
@@ -4195,13 +4266,12 @@ defmodule Module.Types.Descr do
41954266
map_domain_tag_to_type(tag, domain_key) |> union(acc)
41964267

41974268
{tag_or_domains, fields, negs}, acc ->
4198-
{_found, value, bdd} = map_pop_domain_bdd(tag_or_domains, fields, domain_key)
4199-
4200-
negs
4201-
|> map_split_negative(value, bdd, fn neg_tag, neg_fields ->
4202-
map_pop_domain_bdd(neg_tag, neg_fields, domain_key)
4203-
end)
4204-
|> Enum.reduce(acc, fn {value, _}, acc -> union(value, acc) end)
4269+
if init_map_line_empty?(tag_or_domains, fields, negs) do
4270+
acc
4271+
else
4272+
{_found, value, _bdd} = map_pop_domain_bdd(tag_or_domains, fields, domain_key)
4273+
union(value, acc)
4274+
end
42054275
end)
42064276
end
42074277

lib/elixir/test/elixir/module/types/descr_test.exs

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,18 @@ defmodule Module.Types.DescrTest do
2424
defp list(elem_type, tail_type), do: union(empty_list(), non_empty_list(elem_type, tail_type))
2525
defp map_with_default(descr), do: open_map([{to_domain_keys(:term), descr}])
2626

27+
defp projected_negative_map(size) do
28+
Enum.reduce(1..size, open_map(k: open_map(), x: term()), fn index, acc ->
29+
difference(
30+
acc,
31+
open_map([
32+
{:k, open_map([{:"value#{index}", integer()}])},
33+
{:"field#{index}", integer()}
34+
])
35+
)
36+
end)
37+
end
38+
2739
describe "union" do
2840
test "bitmap" do
2941
assert union(integer(), float()) == union(float(), integer())
@@ -2071,6 +2083,11 @@ defmodule Module.Types.DescrTest do
20712083
|> map_fetch_key(:a) == {false, integer()}
20722084
end
20732085

2086+
# Times out without a projection-only map_fetch_key path
2087+
test "map_fetch_key with projected negative maps" do
2088+
assert map_fetch_key(projected_negative_map(100), :k) == {false, open_map()}
2089+
end
2090+
20742091
test "map_fetch_key with dynamic" do
20752092
assert map_fetch_key(dynamic(), :a) == {true, dynamic()}
20762093
assert map_fetch_key(union(dynamic(), integer()), :a) == :badmap
@@ -2224,6 +2241,55 @@ defmodule Module.Types.DescrTest do
22242241
map = closed_map([{:a, atom([:a])}, {:__struct__, term()}, {domain_key(:atom), pid()}])
22252242
{:ok, term} = map_get(map, atom() |> difference(atom([:a])))
22262243
assert equal?(term, term())
2244+
2245+
base = open_map([{domain_key(:atom), term()}])
2246+
bad = open_map(a: if_set(negation(integer())))
2247+
map = negation(union(negation(base), bad))
2248+
2249+
assert equal?(map, open_map(a: integer()))
2250+
2251+
{:ok, type} = map_get(map, atom())
2252+
assert equal?(type, term())
2253+
2254+
{:ok, type} = map_get(map, atom([:a]))
2255+
assert equal?(type, integer())
2256+
2257+
map = closed_map([{:a, term()}, {domain_key(:atom), integer()}])
2258+
2259+
{:ok, type} = map_get(map, atom())
2260+
assert equal?(type, term())
2261+
2262+
{:ok, type} = map_get(map, atom([:a]))
2263+
assert equal?(type, term())
2264+
2265+
{:ok, type} = map_get(map, difference(atom(), atom([:a])))
2266+
assert equal?(type, integer())
2267+
2268+
map =
2269+
closed_map([{:a, term()}, {domain_key(:atom), integer()}])
2270+
|> difference(open_map(a: negation(pid())))
2271+
2272+
{:ok, type} = map_get(map, atom())
2273+
assert equal?(type, union(integer(), pid()))
2274+
2275+
{:ok, type} = map_get(map, atom([:a]))
2276+
assert equal?(type, pid())
2277+
2278+
{:ok, type} = map_get(map, difference(atom(), atom([:a])))
2279+
assert equal?(type, integer())
2280+
2281+
map =
2282+
closed_map([{:a, term()}, {:b, binary()}, {domain_key(:atom), integer()}])
2283+
|> difference(open_map(a: negation(pid())))
2284+
2285+
{:ok, type} = map_get(map, atom())
2286+
assert equal?(type, union(union(integer(), pid()), binary()))
2287+
2288+
{:ok, type} = map_get(map, atom([:a, :b]))
2289+
assert equal?(type, union(pid(), binary()))
2290+
2291+
{:ok, type} = map_get(map, difference(atom(), atom([:a, :b])))
2292+
assert equal?(type, integer())
22272293
end
22282294

22292295
test "with lists" do
@@ -2252,6 +2318,11 @@ defmodule Module.Types.DescrTest do
22522318

22532319
assert map_get(map, list(integer())) == {:ok, atom([:empty, :non_empty])}
22542320
end
2321+
2322+
# Times out without a projection-only map_get path
2323+
test "with projected negative maps" do
2324+
assert map_get(projected_negative_map(100), atom([:k])) == {:ok, open_map()}
2325+
end
22552326
end
22562327

22572328
describe "map_update" do
@@ -2337,6 +2408,12 @@ defmodule Module.Types.DescrTest do
23372408
|> map_update(atom([:b]), integer(), true, true) == {none(), none(), []}
23382409
end
23392410

2411+
# Times out without a projection-aware map_update path
2412+
test "with projected negative maps" do
2413+
assert map_update(projected_negative_map(100), atom([:k]), binary()) ==
2414+
{open_map(), open_map(k: binary(), x: term()), []}
2415+
end
2416+
23402417
test "with non-empty open maps does not call the callback with none from absent branches" do
23412418
# This is a test of the map_update_fun/5 with forced?: false parameter.
23422419
# We check that it does not call its typed_fun argument with `none()`
@@ -2802,6 +2879,31 @@ defmodule Module.Types.DescrTest do
28022879
])
28032880
)}
28042881
end
2882+
2883+
# Times out without proper map_put
2884+
test "with projected negative maps" do
2885+
map = projected_negative_map(100)
2886+
2887+
assert map_put(map, atom([:k]), binary()) == {:ok, open_map(k: binary(), x: term())}
2888+
2889+
map = difference(open_map(k: integer(), x: term()), open_map(k: integer(), a: integer()))
2890+
2891+
{:ok, type} = map_put(map, atom([:k]), binary())
2892+
2893+
assert equal?(
2894+
type,
2895+
difference(open_map(k: binary(), x: term()), open_map(k: binary(), a: integer()))
2896+
)
2897+
end
2898+
2899+
test "with projected negative maps and no popped value projection" do
2900+
# map_put/3 passes nil as the popped value accumulator because it only needs the map side.
2901+
map =
2902+
projected_negative_map(100)
2903+
|> difference(open_map(k: atom(), x: term()))
2904+
2905+
assert map_put(map, atom([:k]), binary()) == {:ok, open_map(k: binary(), x: term())}
2906+
end
28052907
end
28062908

28072909
describe "disjoint" do

lib/elixir/test/elixir/module/types/map_test.exs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,22 @@ defmodule Module.Types.MapTest do
833833
assert typeerror!([x = []], Map.put(x, :key, :value)) =~
834834
"incompatible types given to Map.put/3"
835835
end
836+
837+
test "errors with dynamic key and value" do
838+
assert typeerror!([key, value], Map.put(1, key, value)) |> strip_ansi() =~ """
839+
incompatible types given to Map.put/3:
840+
841+
Map.put(1, key, value)
842+
843+
given types:
844+
845+
integer(), dynamic(), dynamic()
846+
847+
but expected one of:
848+
849+
map(), term(), term()
850+
"""
851+
end
836852
end
837853

838854
describe "Map.put_new_lazy/3" do

0 commit comments

Comments
 (0)