Skip to content

Commit 0642f11

Browse files
pluxplux
committed
Improve performance of crossref diagnostics
1 parent 0216dc4 commit 0642f11

7 files changed

+402
-148
lines changed

‎apps/els_lsp/priv/code_navigation/src/diagnostics_xref_pseudo.erl

+2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ main() ->
1414
unknown_module:module_info(module),
1515
?MODULE:behaviour_info(callbacks),
1616
lager:debug("log message", []),
17+
lager:debug_unsafe("log message", []),
1718
lager:info("log message", []),
1819
lager:notice("log message", []),
1920
lager:warning("log message", []),
@@ -23,6 +24,7 @@ main() ->
2324
lager:emergency("log message", []),
2425

2526
lager:debug("log message"),
27+
lager:debug_unsafe("log message", []),
2628
lager:info("log message"),
2729
lager:notice("log message"),
2830
lager:warning("log message"),

‎apps/els_lsp/src/els_crossref_diagnostics.erl

+170-96
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
%% Includes
2323
%%==============================================================================
2424
-include("els_lsp.hrl").
25+
-include_lib("kernel/include/logger.hrl").
2526
%%==============================================================================
2627
%% Callback Functions
2728
%%==============================================================================
@@ -32,19 +33,41 @@ is_default() ->
3233

3334
-spec run(uri()) -> [els_diagnostics:diagnostic()].
3435
run(Uri) ->
35-
case els_utils:lookup_document(Uri) of
36-
{error, _Error} ->
37-
[];
38-
{ok, Document} ->
39-
POIs = els_dt_document:pois(Document, [
40-
application,
41-
implicit_fun,
42-
import_entry,
43-
export_entry,
44-
nifs_entry
45-
]),
46-
[make_diagnostic(POI) || POI <- POIs, not has_definition(POI, Document)]
47-
end.
36+
EnabledDiagnostics = els_diagnostics:enabled_diagnostics(),
37+
CompilerEnabled = lists:member(<<"compiler">>, EnabledDiagnostics),
38+
Start = erlang:monotonic_time(millisecond),
39+
Res =
40+
case els_utils:lookup_document(Uri) of
41+
{error, _Error} ->
42+
[];
43+
{ok, Document} ->
44+
POIs = els_dt_document:pois(Document, kinds()),
45+
Opts = #{
46+
compiler_enabled => CompilerEnabled
47+
},
48+
{Diags, _Cache} =
49+
lists:mapfoldl(
50+
fun(#{id := Id} = POI, Cache) ->
51+
case find_in_cache(Id, Cache) of
52+
{ok, HasDef} ->
53+
{[make_diagnostic(HasDef, POI)], Cache};
54+
error ->
55+
HasDef = has_definition(POI, Document, Opts),
56+
{
57+
make_diagnostic(HasDef, POI),
58+
update_cache(HasDef, Id, Cache)
59+
}
60+
end
61+
end,
62+
#{},
63+
POIs
64+
),
65+
lists:flatten(Diags)
66+
end,
67+
End = erlang:monotonic_time(millisecond),
68+
Duration = End - Start,
69+
?LOG_DEBUG("Crossref done for ~p [duration: ~p ms]", [els_uri:module(Uri), Duration]),
70+
Res.
4871

4972
-spec source() -> binary().
5073
source() ->
@@ -53,106 +76,157 @@ source() ->
5376
%%==============================================================================
5477
%% Internal Functions
5578
%%==============================================================================
56-
-spec make_diagnostic(els_poi:poi()) -> els_diagnostics:diagnostic().
57-
make_diagnostic(#{range := Range, id := Id}) ->
58-
Function =
59-
case Id of
60-
{F, A} -> lists:flatten(io_lib:format("~p/~p", [F, A]));
61-
{M, F, A} -> lists:flatten(io_lib:format("~p:~p/~p", [M, F, A]))
62-
end,
63-
Message = els_utils:to_binary(
64-
io_lib:format(
65-
"Cannot find definition for function ~s",
66-
[Function]
67-
)
68-
),
79+
-spec update_cache(true | {missing, function | module}, els_poi:poi_id(), map()) -> map().
80+
update_cache({missing, module}, {M, _F, _A}, Cache) ->
81+
%% Cache missing module to avoid repeated lookups
82+
Cache#{M => missing};
83+
update_cache(HasDef, Id, Cache) ->
84+
Cache#{Id => HasDef}.
85+
86+
-spec find_in_cache(els_poi:poi_id(), map()) -> _.
87+
find_in_cache({M, _F, _A}, Cache) when is_map_key(M, Cache) ->
88+
{ok, {missing, module}};
89+
find_in_cache(Id, Cache) ->
90+
maps:find(Id, Cache).
91+
92+
-spec kinds() -> [els_poi:poi_kind()].
93+
kinds() ->
94+
[
95+
application,
96+
implicit_fun,
97+
import_entry,
98+
export_entry,
99+
nifs_entry
100+
].
101+
102+
-spec make_diagnostic(_, els_poi:poi()) -> [els_diagnostics:diagnostic()].
103+
make_diagnostic({missing, Kind}, #{id := Id} = POI) ->
104+
Message = error_msg(Kind, Id),
69105
Severity = ?DIAGNOSTIC_ERROR,
70-
els_diagnostics:make_diagnostic(
71-
els_protocol:range(Range),
72-
Message,
73-
Severity,
74-
source()
75-
).
76-
77-
-spec has_definition(els_poi:poi(), els_dt_document:item()) -> boolean().
78-
has_definition(
79-
#{
80-
kind := application,
81-
id := {module_info, 0}
82-
},
83-
_
84-
) ->
106+
[
107+
els_diagnostics:make_diagnostic(
108+
els_protocol:range(range(Kind, POI)),
109+
Message,
110+
Severity,
111+
source()
112+
)
113+
];
114+
make_diagnostic(true, _) ->
115+
[].
116+
117+
-spec range(module | function, els_poi:poi()) -> els_poi:poi_range().
118+
range(module, #{data := #{mod_range := Range}}) ->
119+
Range;
120+
range(function, #{data := #{name_range := Range}}) ->
121+
Range;
122+
range(_, #{range := Range}) ->
123+
Range.
124+
125+
-spec error_msg(module | function, els_poi:poi_id()) -> binary().
126+
error_msg(module, {M, _F, _A}) ->
127+
els_utils:to_binary(io_lib:format("Cannot find module ~p", [M]));
128+
error_msg(function, Id) ->
129+
els_utils:to_binary(io_lib:format("Cannot find definition for function ~s", [id_str(Id)])).
130+
131+
-spec id_str(els_poi:poi_id()) -> binary().
132+
id_str(Id) ->
133+
case Id of
134+
{F, A} -> lists:flatten(io_lib:format("~p/~p", [F, A]));
135+
{M, F, A} -> lists:flatten(io_lib:format("~p:~p/~p", [M, F, A]))
136+
end.
137+
138+
-spec has_definition(els_poi:poi(), els_dt_document:item(), _) ->
139+
true | {missing, function | module}.
140+
has_definition(#{data := #{imported := true}}, _Document, _Opts) ->
141+
%% Call to a bif
85142
true;
86-
has_definition(
87-
#{
88-
kind := application,
89-
id := {module_info, 1}
90-
},
91-
_
92-
) ->
143+
has_definition(#{id := {module_info, 0}}, _, _) ->
93144
true;
94-
has_definition(
95-
#{
96-
kind := application,
97-
data := #{mod_is_variable := true}
98-
},
99-
_
100-
) ->
145+
has_definition(#{id := {module_info, 1}}, _, _) ->
101146
true;
102-
has_definition(
103-
#{
104-
kind := application,
105-
id := {Module, module_info, Arity}
106-
},
107-
_
108-
) when Arity =:= 0; Arity =:= 1 ->
109-
{ok, []} =/= els_dt_document_index:lookup(Module);
110-
has_definition(
111-
#{
112-
kind := application,
113-
id := {record_info, 2}
114-
},
115-
_
116-
) ->
147+
has_definition(#{data := #{mod_is_variable := true}}, _, _) ->
117148
true;
118-
has_definition(
119-
#{
120-
kind := application,
121-
id := {behaviour_info, 1}
122-
},
123-
_
124-
) ->
149+
has_definition(#{data := #{fun_is_variable := true}}, _, _) ->
125150
true;
126-
has_definition(
127-
#{
128-
kind := application,
129-
data := #{fun_is_variable := true}
130-
},
131-
_
132-
) ->
151+
has_definition(#{id := {Module, module_info, Arity}}, _, _) when Arity =:= 0; Arity =:= 1 ->
152+
case els_dt_document_index:lookup(Module) of
153+
{ok, []} ->
154+
{missing, module};
155+
{ok, _} ->
156+
true
157+
end;
158+
has_definition(#{id := {record_info, 2}}, _, _) ->
159+
true;
160+
has_definition(#{id := {behaviour_info, 1}}, _, _) ->
161+
true;
162+
has_definition(#{id := {lager, Level, Arity}}, _, _) ->
163+
lager_definition(Level, Arity);
164+
has_definition(#{id := {lists, append, 1}}, _, _) ->
165+
%% lists:append/1 isn't indexed for some reason
133166
true;
134167
has_definition(
168+
#{id := {F, A}} = POI,
169+
Document,
135170
#{
136-
kind := application,
137-
id := {lager, Level, Arity}
138-
},
139-
_
171+
%% Compiler already checks local function calls
172+
compiler_enabled := false
173+
}
140174
) ->
141-
lager_definition(Level, Arity);
142-
has_definition(POI, #{uri := Uri}) ->
143-
case els_code_navigation:goto_definition(Uri, POI) of
144-
{ok, _Defs} ->
175+
Uri = els_dt_document:uri(Document),
176+
MFA = {els_uri:module(Uri), F, A},
177+
case function_lookup(MFA) of
178+
true ->
145179
true;
146-
{error, _Error} ->
180+
false ->
181+
case els_code_navigation:goto_definition(Uri, POI) of
182+
{ok, _Defs} ->
183+
true;
184+
{error, _Error} ->
185+
{missing, function}
186+
end
187+
end;
188+
has_definition(#{id := {M, _F, _A} = MFA} = POI, _Document, _Opts) ->
189+
case function_lookup(MFA) of
190+
true ->
191+
true;
192+
false ->
193+
case els_utils:find_module(M) of
194+
{ok, Uri} ->
195+
case els_code_navigation:goto_definition(Uri, POI) of
196+
{ok, _Defs} ->
197+
true;
198+
{error, _Error} ->
199+
{missing, function}
200+
end;
201+
{error, _} ->
202+
{missing, module}
203+
end
204+
end;
205+
has_definition(_POI, #{uri := _Uri}, _Opts) ->
206+
true.
207+
208+
-spec function_lookup(mfa()) -> boolean().
209+
function_lookup(MFA) ->
210+
case els_db:lookup(els_dt_functions:name(), MFA) of
211+
{ok, []} ->
212+
false;
213+
{ok, _} ->
214+
true;
215+
_ ->
147216
false
148217
end.
149218

150219
-spec lager_definition(atom(), integer()) -> boolean().
151220
lager_definition(Level, Arity) when Arity =:= 1 orelse Arity =:= 2 ->
152-
lists:member(Level, lager_levels());
221+
case lists:member(Level, lager_levels()) of
222+
true ->
223+
true;
224+
false ->
225+
{missing, function}
226+
end;
153227
lager_definition(_, _) ->
154-
false.
228+
{missing, function}.
155229

156230
-spec lager_levels() -> [atom()].
157231
lager_levels() ->
158-
[debug, info, notice, warning, error, critical, alert, emergency].
232+
[debug, debug_unsafe, info, notice, warning, error, critical, alert, emergency].

‎apps/els_lsp/src/els_db.erl

+1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ tables() ->
3232
els_dt_document_index,
3333
els_dt_references,
3434
els_dt_signatures,
35+
els_dt_functions,
3536
els_docs_memo
3637
].
3738

‎apps/els_lsp/src/els_diagnostics.erl

+7-2
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,10 @@ run_diagnostics(Uri) ->
121121
ok = wait_for_indexing_job(Uri),
122122
[run_diagnostic(Uri, Id) || Id <- enabled_diagnostics()];
123123
false ->
124-
?LOG_INFO("Initial indexing is not done, skip running diagnostics."),
124+
?LOG_INFO(
125+
"Initial indexing is not done, skip running diagnostics for ~p",
126+
[els_uri:module(Uri)]
127+
),
125128
[]
126129
end.
127130

@@ -130,6 +133,8 @@ run_diagnostics(Uri) ->
130133
%%==============================================================================
131134
-spec is_initial_indexing_done() -> boolean().
132135
is_initial_indexing_done() ->
136+
%% Keep in sync with els_indexing
137+
Jobs = [<<"Applications">>, <<"OTP">>, <<"Dependencies">>],
133138
JobTitles = els_background_job:list_titles(),
134139
lists:all(
135140
fun(Job) ->
@@ -138,7 +143,7 @@ is_initial_indexing_done() ->
138143
JobTitles
139144
)
140145
end,
141-
[<<"Applications">>, <<"OTP">>, <<"Dependencies">>]
146+
Jobs
142147
).
143148

144149
-spec wait_for_indexing_job(uri()) -> ok.

0 commit comments

Comments
 (0)