Improve performance of crossref diagnostics

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

@@ -14,6 +14,7 @@ main() ->
   unknown_module:module_info(module),
   ?MODULE:behaviour_info(callbacks),
   lager:debug("log message", []),
+  lager:debug_unsafe("log message", []),
   lager:info("log message", []),
   lager:notice("log message", []),
   lager:warning("log message", []),
@@ -23,6 +24,7 @@ main() ->
   lager:emergency("log message", []),
 
   lager:debug("log message"),
+  lager:debug_unsafe("log message", []),
   lager:info("log message"),
   lager:notice("log message"),
   lager:warning("log message"),

apps/els_lsp/src/els_crossref_diagnostics.erl

@@ -22,6 +22,7 @@
 %% Includes
 %%==============================================================================
 -include("els_lsp.hrl").
+-include_lib("kernel/include/logger.hrl").
 %%==============================================================================
 %% Callback Functions
 %%==============================================================================
@@ -32,19 +33,41 @@ is_default() ->
 
 -spec run(uri()) -> [els_diagnostics:diagnostic()].
 run(Uri) ->
-    case els_utils:lookup_document(Uri) of
-        {error, _Error} ->
-            [];
-        {ok, Document} ->
-            POIs = els_dt_document:pois(Document, [
-                application,
-                implicit_fun,
-                import_entry,
-                export_entry,
-                nifs_entry
-            ]),
-            [make_diagnostic(POI) || POI <- POIs, not has_definition(POI, Document)]
-    end.
+    EnabledDiagnostics = els_diagnostics:enabled_diagnostics(),
+    CompilerEnabled = lists:member(<<"compiler">>, EnabledDiagnostics),
+    Start = erlang:monotonic_time(millisecond),
+    Res =
+        case els_utils:lookup_document(Uri) of
+            {error, _Error} ->
+                [];
+            {ok, Document} ->
+                POIs = els_dt_document:pois(Document, kinds()),
+                Opts = #{
+                    compiler_enabled => CompilerEnabled
+                },
+                {Diags, _Cache} =
+                    lists:mapfoldl(
+                        fun(#{id := Id} = POI, Cache) ->
+                            case find_in_cache(Id, Cache) of
+                                {ok, HasDef} ->
+                                    {[make_diagnostic(HasDef, POI)], Cache};
+                                error ->
+                                    HasDef = has_definition(POI, Document, Opts),
+                                    {
+                                        make_diagnostic(HasDef, POI),
+                                        update_cache(HasDef, Id, Cache)
+                                    }
+                            end
+                        end,
+                        #{},
+                        POIs
+                    ),
+                lists:flatten(Diags)
+        end,
+    End = erlang:monotonic_time(millisecond),
+    Duration = End - Start,
+    ?LOG_DEBUG("Crossref done for ~p [duration: ~p ms]", [els_uri:module(Uri), Duration]),
+    Res.
 
 -spec source() -> binary().
 source() ->
@@ -53,106 +76,155 @@ source() ->
 %%==============================================================================
 %% Internal Functions
 %%==============================================================================
--spec make_diagnostic(els_poi:poi()) -> els_diagnostics:diagnostic().
-make_diagnostic(#{range := Range, id := Id}) ->
-    Function =
-        case Id of
-            {F, A} -> lists:flatten(io_lib:format("~p/~p", [F, A]));
-            {M, F, A} -> lists:flatten(io_lib:format("~p:~p/~p", [M, F, A]))
-        end,
-    Message = els_utils:to_binary(
-        io_lib:format(
-            "Cannot find definition for function ~s",
-            [Function]
-        )
-    ),
+-spec update_cache(true | {missing, function | module}, els_poi:poi_id(), map()) -> map().
+update_cache({missing, module}, {M, _F, _A}, Cache) ->
+    %% Cache missing module to avoid repeated lookups
+    Cache#{M => missing};
+update_cache(HasDef, Id, Cache) ->
+    Cache#{Id => HasDef}.
+
+-spec find_in_cache(els_poi:poi_id(), map()) -> _.
+find_in_cache({M, _F, _A}, Cache) when is_map_key(M, Cache) ->
+    {ok, {missing, module}};
+find_in_cache(Id, Cache) ->
+    maps:find(Id, Cache).
+
+-spec kinds() -> [els_poi:poi_kind()].
+kinds() ->
+    [
+        application,
+        implicit_fun,
+        import_entry,
+        export_entry,
+        nifs_entry
+    ].
+
+-spec make_diagnostic(_, els_poi:poi()) -> [els_diagnostics:diagnostic()].
+make_diagnostic({missing, Kind}, #{id := Id} = POI) ->
+    Message = error_msg(Kind, Id),
     Severity = ?DIAGNOSTIC_ERROR,
-    els_diagnostics:make_diagnostic(
-        els_protocol:range(Range),
-        Message,
-        Severity,
-        source()
-    ).
-
--spec has_definition(els_poi:poi(), els_dt_document:item()) -> boolean().
-has_definition(
-    #{
-        kind := application,
-        id := {module_info, 0}
-    },
-    _
-) ->
+    [
+        els_diagnostics:make_diagnostic(
+            els_protocol:range(range(Kind, POI)),
+            Message,
+            Severity,
+            source()
+        )
+    ];
+make_diagnostic(true, _) ->
+    [].
+
+-spec range(module | function, els_poi:poi()) -> els_poi:poi_range().
+range(module, #{data := #{mod_range := Range}}) ->
+    Range;
+range(function, #{data := #{name_range := Range}}) ->
+    Range;
+range(_, #{range := Range}) ->
+    Range.
+
+-spec error_msg(module | function, els_poi:poi_id()) -> binary().
+error_msg(module, {M, _F, _A}) ->
+    els_utils:to_binary(io_lib:format("Cannot find module ~p", [M]));
+error_msg(function, Id) ->
+    els_utils:to_binary(io_lib:format("Cannot find definition for function ~s", [id_str(Id)])).
+
+-spec id_str(els_poi:poi_id()) -> string().
+id_str(Id) ->
+    case Id of
+        {F, A} -> lists:flatten(io_lib:format("~p/~p", [F, A]));
+        {M, F, A} -> lists:flatten(io_lib:format("~p:~p/~p", [M, F, A]))
+    end.
+
+-spec has_definition(els_poi:poi(), els_dt_document:item(), _) ->
+    true | {missing, function | module}.
+has_definition(#{data := #{imported := true}}, _Document, _Opts) ->
+    %% Call to a bif
     true;
-has_definition(
-    #{
-        kind := application,
-        id := {module_info, 1}
-    },
-    _
-) ->
+has_definition(#{id := {module_info, 0}}, _, _) ->
     true;
-has_definition(
-    #{
-        kind := application,
-        data := #{mod_is_variable := true}
-    },
-    _
-) ->
+has_definition(#{id := {module_info, 1}}, _, _) ->
     true;
-has_definition(
-    #{
-        kind := application,
-        id := {Module, module_info, Arity}
-    },
-    _
-) when Arity =:= 0; Arity =:= 1 ->
-    {ok, []} =/= els_dt_document_index:lookup(Module);
-has_definition(
-    #{
-        kind := application,
-        id := {record_info, 2}
-    },
-    _
-) ->
+has_definition(#{data := #{mod_is_variable := true}}, _, _) ->
     true;
-has_definition(
-    #{
-        kind := application,
-        id := {behaviour_info, 1}
-    },
-    _
-) ->
+has_definition(#{data := #{fun_is_variable := true}}, _, _) ->
     true;
-has_definition(
-    #{
-        kind := application,
-        data := #{fun_is_variable := true}
-    },
-    _
-) ->
+has_definition(#{id := {Module, module_info, Arity}}, _, _) when Arity =:= 0; Arity =:= 1 ->
+    case els_dt_document_index:lookup(Module) of
+        {ok, []} ->
+            {missing, module};
+        {ok, _} ->
+            true
+    end;
+has_definition(#{id := {record_info, 2}}, _, _) ->
+    true;
+has_definition(#{id := {behaviour_info, 1}}, _, _) ->
+    true;
+has_definition(#{id := {lager, Level, Arity}}, _, _) ->
+    lager_definition(Level, Arity);
+has_definition(#{id := {lists, append, 1}}, _, _) ->
+    %% lists:append/1 isn't indexed for some reason
     true;
 has_definition(
+    #{id := {F, A}} = POI,
+    Document,
     #{
-        kind := application,
-        id := {lager, Level, Arity}
-    },
-    _
+        %% Compiler already checks local function calls
+        compiler_enabled := false
+    }
 ) ->
-    lager_definition(Level, Arity);
-has_definition(POI, #{uri := Uri}) ->
-    case els_code_navigation:goto_definition(Uri, POI) of
-        {ok, _Defs} ->
+    Uri = els_dt_document:uri(Document),
+    MFA = {els_uri:module(Uri), F, A},
+    case function_lookup(MFA) of
+        true ->
+            true;
+        false ->
+            case els_code_navigation:goto_definition(Uri, POI) of
+                {ok, _Defs} ->
+                    true;
+                {error, _Error} ->
+                    {missing, function}
+            end
+    end;
+has_definition(#{id := {M, _F, _A} = MFA} = POI, _Document, _Opts) ->
+    case function_lookup(MFA) of
+        true ->
             true;
-        {error, _Error} ->
-            false
+        false ->
+            case els_utils:find_module(M) of
+                {ok, Uri} ->
+                    case els_code_navigation:goto_definition(Uri, POI) of
+                        {ok, _Defs} ->
+                            true;
+                        {error, _Error} ->
+                            {missing, function}
+                    end;
+                {error, _} ->
+                    {missing, module}
+            end
+    end;
+has_definition(_POI, #{uri := _Uri}, _Opts) ->
+    true.
+
+-spec function_lookup(mfa()) -> boolean().
+function_lookup(MFA) ->
+    case els_db:lookup(els_dt_functions:name(), MFA) of
+        {ok, []} ->
+            false;
+        {ok, _} ->
+            true
     end.
 
 -spec lager_definition(atom(), integer()) -> boolean().
 lager_definition(Level, Arity) when Arity =:= 1 orelse Arity =:= 2 ->
-    lists:member(Level, lager_levels());
+    case lists:member(Level, lager_levels()) of
+        true ->
+            true;
+        false ->
+            {missing, function}
+    end;
 lager_definition(_, _) ->
-    false.
+    {missing, function}.
 
 -spec lager_levels() -> [atom()].
 lager_levels() ->
-    [debug, info, notice, warning, error, critical, alert, emergency].
+    [debug, debug_unsafe, info, notice, warning, error, critical, alert, emergency].

apps/els_lsp/src/els_db.erl

@@ -32,6 +32,7 @@ tables() ->
         els_dt_document_index,
         els_dt_references,
         els_dt_signatures,
+        els_dt_functions,
         els_docs_memo
     ].
 

apps/els_lsp/src/els_diagnostics.erl

@@ -121,7 +121,10 @@ run_diagnostics(Uri) ->
             ok = wait_for_indexing_job(Uri),
             [run_diagnostic(Uri, Id) || Id <- enabled_diagnostics()];
         false ->
-            ?LOG_INFO("Initial indexing is not done, skip running diagnostics."),
+            ?LOG_INFO(
+                "Initial indexing is not done, skip running diagnostics for ~p",
+                [els_uri:module(Uri)]
+            ),
             []
     end.
 
@@ -130,6 +133,8 @@ run_diagnostics(Uri) ->
 %%==============================================================================
 -spec is_initial_indexing_done() -> boolean().
 is_initial_indexing_done() ->
+    %% Keep in sync with els_indexing
+    Jobs = [<<"Applications">>, <<"OTP">>, <<"Dependencies">>],
     JobTitles = els_background_job:list_titles(),
     lists:all(
         fun(Job) ->
@@ -138,7 +143,7 @@ is_initial_indexing_done() ->
                 JobTitles
             )
         end,
-        [<<"Applications">>, <<"OTP">>, <<"Dependencies">>]
+        Jobs
     ).
 
 -spec wait_for_indexing_job(uri()) -> ok.