Skip to content

Commit 995f7fc

Browse files
committed
Optimize protocol consolidation
* Allow directory listings to be pre-computed * Do not require struct modules to be loaded The current implementation would require us to recompute protocols whenever a struct changed, something that is not currently available. Furthermore, the fact we had to load modules naturally made consolidation more expensive. Therefore we need a lazy type resolution, and until we have such feature, we will only track struct names.
1 parent 67431fc commit 995f7fc

File tree

5 files changed

+96
-38
lines changed

5 files changed

+96
-38
lines changed

lib/elixir/lib/module/types/apply.ex

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,6 +1469,17 @@ defmodule Module.Types.Apply do
14691469
"""
14701470
end
14711471

1472+
# The protocol has, at the moment, simplified clauses, so we build the complete one
1473+
clauses =
1474+
case mod.__protocol__(:impls) do
1475+
{:consolidated, mods} ->
1476+
domain = mods |> Enum.map(&Module.Types.Of.impl/1) |> Enum.reduce(&union/2)
1477+
[{[domain], dynamic()}]
1478+
1479+
_ ->
1480+
clauses
1481+
end
1482+
14721483
{"""
14731484
but expected a type that implements the #{inspect(mod)} protocol.
14741485
#{fix}\

lib/elixir/lib/module/types/of.ex

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,16 +142,25 @@ defmodule Module.Types.Of do
142142
{Any, term()}
143143
]
144144

145+
@doc """
146+
Currently, for protocol implementations, we only store
147+
the open struct definition. This is because we don't want
148+
to reconsolidate whenever the struct changes, but at the
149+
moment we can't store references either. Ideally struct
150+
types on protocol dispatches would be lazily resolved.
151+
"""
152+
def impl(for, mode \\ :closed)
153+
145154
for {for, type} <- impls do
146-
def impl(unquote(for)), do: unquote(Macro.escape(type))
155+
def impl(unquote(for), _mode), do: unquote(Macro.escape(type))
147156
end
148157

149-
def impl(struct) do
158+
def impl(struct, mode) do
150159
# Elixir did not strictly require the implementation to be available,
151160
# so we need to deal with such cases accordingly.
152161
# TODO: Assume implementation is available on Elixir v2.0.
153162
# A warning is emitted since v1.19+.
154-
if info = Code.ensure_loaded?(struct) && struct.__info__(:struct) do
163+
if info = mode == :closed && Code.ensure_loaded?(struct) && struct.__info__(:struct) do
155164
struct_type(struct, info)
156165
else
157166
open_map(__struct__: atom([struct]))

lib/elixir/lib/protocol.ex

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ defmodule Protocol do
453453
true
454454
455455
"""
456-
@spec extract_protocols([charlist | String.t()]) :: [atom]
456+
@spec extract_protocols([charlist | String.t() | {charlist, [charlist]}]) :: [atom]
457457
def extract_protocols(paths) do
458458
extract_matching_by_attribute(paths, [?E, ?l, ?i, ?x, ?i, ?r, ?.], fn module, attributes ->
459459
case attributes[:__protocol__] do
@@ -482,7 +482,7 @@ defmodule Protocol do
482482
true
483483
484484
"""
485-
@spec extract_impls(module, [charlist | String.t()]) :: [atom]
485+
@spec extract_impls(module, [charlist | String.t() | {charlist, [charlist]}]) :: [atom]
486486
def extract_impls(protocol, paths) when is_atom(protocol) do
487487
prefix = Atom.to_charlist(protocol) ++ [?.]
488488

@@ -496,17 +496,25 @@ defmodule Protocol do
496496

497497
defp extract_matching_by_attribute(paths, prefix, callback) do
498498
for path <- paths,
499-
# Do not use protocols as they may be consolidating
500-
path = if(is_list(path), do: path, else: String.to_charlist(path)),
501-
file <- list_dir(path),
499+
{path, files} = list_dir(path),
500+
file <- files,
502501
mod = extract_from_file(path, file, prefix, callback),
503502
do: mod
504503
end
505504

505+
# Do not use protocols as they may be consolidating
506+
defp list_dir({path, files}) when is_list(path) and is_list(files) do
507+
{path, files}
508+
end
509+
510+
defp list_dir(path) when is_binary(path) do
511+
list_dir(String.to_charlist(path))
512+
end
513+
506514
defp list_dir(path) when is_list(path) do
507515
case :file.list_dir(path) do
508-
{:ok, files} -> files
509-
_ -> []
516+
{:ok, files} -> {path, files}
517+
_ -> {path, []}
510518
end
511519
end
512520

@@ -633,7 +641,7 @@ defmodule Protocol do
633641
checker =
634642
if checker do
635643
update_in(checker.exports, fn exports ->
636-
signatures = new_signatures(definitions, protocol_funs, protocol, types)
644+
signatures = new_signatures(definitions, protocol_funs, protocol, types, structs)
637645

638646
for {fun, info} <- exports do
639647
if sig = Map.get(signatures, fun) do
@@ -652,14 +660,13 @@ defmodule Protocol do
652660
end
653661
end
654662

655-
defp new_signatures(definitions, protocol_funs, protocol, types) do
663+
defp new_signatures(definitions, protocol_funs, protocol, types, structs) do
656664
alias Module.Types.Descr
665+
types_minus_any = List.delete(types, Any)
657666

658667
clauses =
659-
types
660-
|> List.delete(Any)
661-
|> Enum.map(fn impl ->
662-
{[Module.Types.Of.impl(impl)], Descr.atom([__concat__(protocol, impl)])}
668+
Enum.map(types_minus_any, fn impl ->
669+
{[Module.Types.Of.impl(impl, :open)], Descr.atom([__concat__(protocol, impl)])}
663670
end)
664671

665672
{domain, impl_for, impl_for!} =
@@ -674,10 +681,16 @@ defmodule Protocol do
674681
end
675682

676683
_ ->
684+
structs_domain =
685+
case structs do
686+
[] -> Descr.none()
687+
_ -> Descr.open_map(__struct__: Descr.atom(structs))
688+
end
689+
677690
domain =
678-
clauses
679-
|> Enum.map(fn {[domain], _} -> domain end)
680-
|> Enum.reduce(&Descr.union/2)
691+
Enum.reduce(types_minus_any -- structs, structs_domain, fn impl, acc ->
692+
Descr.union(Module.Types.Of.impl(impl, :open), acc)
693+
end)
681694

682695
not_domain = Descr.negation(domain)
683696

lib/elixir/test/elixir/protocol/consolidation_test.exs

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -183,21 +183,23 @@ defmodule Protocol.ConsolidationTest do
183183
assert domain == [term()]
184184

185185
assert clauses == [
186-
{[Of.impl(ImplStruct)], atom([Sample.Protocol.ConsolidationTest.ImplStruct])},
187-
{[negation(Of.impl(ImplStruct))], atom([nil])}
186+
{[Of.impl(ImplStruct, :open)],
187+
atom([Sample.Protocol.ConsolidationTest.ImplStruct])},
188+
{[negation(Of.impl(ImplStruct, :open))], atom([nil])}
188189
]
189190

190191
assert %{{:impl_for!, 1} => %{sig: {:strong, domain, clauses}}} = exports
191-
assert domain == [Of.impl(ImplStruct)]
192+
assert domain == [Of.impl(ImplStruct, :open)]
192193

193194
assert clauses == [
194-
{[Of.impl(ImplStruct)], atom([Sample.Protocol.ConsolidationTest.ImplStruct])}
195+
{[Of.impl(ImplStruct, :open)],
196+
atom([Sample.Protocol.ConsolidationTest.ImplStruct])}
195197
]
196198

197199
assert %{{:ok, 1} => %{sig: {:strong, nil, clauses}}} = exports
198200

199201
assert clauses == [
200-
{[Of.impl(ImplStruct)], dynamic()}
202+
{[Of.impl(ImplStruct, :open)], dynamic()}
201203
]
202204
end
203205

@@ -212,9 +214,11 @@ defmodule Protocol.ConsolidationTest do
212214
assert domain == [term()]
213215

214216
assert clauses == [
215-
{[Of.impl(Map)], atom([WithAny.Map])},
216-
{[Of.impl(ImplStruct)], atom([WithAny.Protocol.ConsolidationTest.ImplStruct])},
217-
{[negation(union(Of.impl(ImplStruct), Of.impl(Map)))], atom([WithAny.Any])}
217+
{[Of.impl(Map, :open)], atom([WithAny.Map])},
218+
{[Of.impl(ImplStruct, :open)],
219+
atom([WithAny.Protocol.ConsolidationTest.ImplStruct])},
220+
{[negation(union(Of.impl(ImplStruct, :open), Of.impl(Map, :open)))],
221+
atom([WithAny.Any])}
218222
]
219223

220224
assert %{{:ok, 2} => %{sig: {:strong, nil, clauses}}} = exports
@@ -275,18 +279,34 @@ defmodule Protocol.ConsolidationTest do
275279
assert Inspect in protos
276280
end
277281

282+
test "protocols with expanded path" do
283+
path = to_charlist(Application.app_dir(:elixir, "ebin"))
284+
{:ok, mods} = :file.list_dir(path)
285+
protos = Protocol.extract_protocols([{path, mods}])
286+
assert Enumerable in protos
287+
assert Inspect in protos
288+
end
289+
278290
test "implementations with charlist path" do
279-
protos =
291+
impls =
280292
Protocol.extract_impls(Enumerable, [to_charlist(Application.app_dir(:elixir, "ebin"))])
281293

282-
assert List in protos
283-
assert Function in protos
294+
assert List in impls
295+
assert Function in impls
284296
end
285297

286298
test "implementations with binary path" do
287-
protos = Protocol.extract_impls(Enumerable, [Application.app_dir(:elixir, "ebin")])
288-
assert List in protos
289-
assert Function in protos
299+
impls = Protocol.extract_impls(Enumerable, [Application.app_dir(:elixir, "ebin")])
300+
assert List in impls
301+
assert Function in impls
302+
end
303+
304+
test "implementations with expanded path" do
305+
path = to_charlist(Application.app_dir(:elixir, "ebin"))
306+
{:ok, mods} = :file.list_dir(path)
307+
impls = Protocol.extract_impls(Enumerable, [{path, mods}])
308+
assert List in impls
309+
assert Function in impls
290310
end
291311
end
292312
end

lib/mix/lib/mix/compilers/protocol.ex

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,16 @@ defmodule Mix.Compilers.Protocol do
126126
end
127127

128128
defp consolidation_paths do
129-
filter_otp(:code.get_path(), :code.lib_dir())
130-
end
131-
132-
defp filter_otp(paths, otp) do
133-
Enum.filter(paths, &(not :lists.prefix(otp, &1)))
129+
otp = :code.lib_dir()
130+
131+
# Pre-list the paths in each directory for performance
132+
for path <- :code.get_path(), not :lists.prefix(otp, path) do
133+
{path,
134+
case :file.list_dir(path) do
135+
{:ok, files} -> files
136+
_ -> []
137+
end}
138+
end
134139
end
135140

136141
defp consolidate([], _paths, output, _opts) do

0 commit comments

Comments
 (0)