Skip to content

Commit

Permalink
Performance oriented fixes. (#841)
Browse files Browse the repository at this point in the history
* Perf: Precalculate contexts

We were calculating the context of the cursor once per suggestion,
this would take a couple dozen milliseconds, but for a module that had
a lot of functions, this would add up. Precalculating them saved a lot
of time.

* Use new Mix.Tasks.Format

Jose added a function that allows us to get a formatter without having
to lock on the build. This reduces the formatting time significantly,
and we no longer have that frustrating freeze on save.

---------

Co-authored-by: scohen <[email protected]>
  • Loading branch information
scohen and scohen authored Dec 11, 2024
1 parent bc36cc4 commit a9a806c
Show file tree
Hide file tree
Showing 11 changed files with 1,256 additions and 88 deletions.
1,047 changes: 1,047 additions & 0 deletions apps/common/lib/future/mix/tasks/format.ex

Large diffs are not rendered by default.

112 changes: 45 additions & 67 deletions apps/common/lib/lexical/ast/env.ex
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ defmodule Lexical.Ast.Env do
:suffix,
:position,
:position_module,
:zero_based_character
:zero_based_character,
:detected_contexts
]

@type t :: %__MODULE__{
Expand All @@ -33,7 +34,8 @@ defmodule Lexical.Ast.Env do
suffix: String.t(),
position: Position.t(),
position_module: String.t(),
zero_based_character: non_neg_integer()
zero_based_character: non_neg_integer(),
detected_contexts: %{atom() => boolean()}
}

@type token_value :: String.t() | charlist() | atom()
Expand Down Expand Up @@ -87,6 +89,8 @@ defmodule Lexical.Ast.Env do
zero_based_character: zero_based_character
}

env = detect_contexts(env)

{:ok, env}

_ ->
Expand All @@ -107,81 +111,55 @@ defmodule Lexical.Ast.Env do
end
end

@detectors %{
:alias => Detection.Alias,
:behaviour => {Detection.ModuleAttribute, [:behaviour]},
:bitstring => Detection.Bitstring,
:callback => {Detection.ModuleAttribute, [:callback]},
:comment => Detection.Comment,
:doc => {Detection.ModuleAttribute, [:doc]},
:function_capture => Detection.FunctionCapture,
:impl => {Detection.ModuleAttribute, [:impl]},
:import => Detection.Import,
:macrocallback => {Detection.ModuleAttribute, [:macrocallback]},
:moduledoc => {Detection.ModuleAttribute, [:moduledoc]},
:pipe => Detection.Pipe,
:require => Detection.Require,
:spec => Detection.Spec,
:string => Detection.String,
:struct_field_key => Detection.StructFieldKey,
:struct_field_value => Detection.StructFieldValue,
:struct_fields => Detection.StructFields,
:struct_reference => Detection.StructReference,
:type => Detection.Type,
:use => Detection.Use
}

def detect_contexts(%__MODULE__{} = env) do
detected_contexts =
Map.new(@detectors, fn
{context_name, {detector, extra_args}} ->
{context_name, apply(detector, :detected?, [env.analysis, env.position | extra_args])}

Check warning on line 142 in apps/common/lib/lexical/ast/env.ex

View workflow job for this annotation

GitHub Actions / Static analysis

Avoid `apply/2` and `apply/3` when the number of arguments is known.

{context_name, detector} ->
{context_name, detector.detected?(env.analysis, env.position)}
end)

%__MODULE__{env | detected_contexts: detected_contexts}
end

@spec in_context?(t, context_type) :: boolean()
# credo:disable-for-next-line Credo.Check.Refactor.CyclomaticComplexity
def in_context?(%__MODULE__{} = env, context_type) do
analysis = env.analysis
position = env.position

case context_type do
:alias ->
Detection.Alias.detected?(analysis, position)

:behaviour ->
Detection.ModuleAttribute.detected?(analysis, position, :behaviour)

:bitstring ->
Detection.Bitstring.detected?(analysis, position)

:callback ->
Detection.ModuleAttribute.detected?(analysis, position, :callback)

:comment ->
Detection.Comment.detected?(analysis, position)

:doc ->
Detection.ModuleAttribute.detected?(analysis, position, :doc)

:function_capture ->
Detection.FunctionCapture.detected?(analysis, position)

:impl ->
Detection.ModuleAttribute.detected?(analysis, position, :impl)

:import ->
Detection.Import.detected?(analysis, position)

:module_attribute ->
Detection.ModuleAttribute.detected?(analysis, position)

{:module_attribute, name} ->
Detection.ModuleAttribute.detected?(analysis, position, name)

:macrocallback ->
Detection.ModuleAttribute.detected?(analysis, position, :macrocallback)

:moduledoc ->
Detection.ModuleAttribute.detected?(analysis, position, :moduledoc)

:pipe ->
Detection.Pipe.detected?(analysis, position)

:require ->
Detection.Require.detected?(analysis, position)

:spec ->
Detection.Spec.detected?(analysis, position)

:string ->
Detection.String.detected?(analysis, position)

:struct_fields ->
Detection.StructFields.detected?(analysis, position)

:struct_field_key ->
Detection.StructFieldKey.detected?(analysis, position)

:struct_field_value ->
Detection.StructFieldValue.detected?(analysis, position)

:struct_reference ->
Detection.StructReference.detected?(analysis, position)

:type ->
Detection.Type.detected?(analysis, position)

:use ->
Detection.Use.detected?(analysis, position)
context_type ->
Map.get(env.detected_contexts, context_type)
end
end

Expand Down
17 changes: 17 additions & 0 deletions apps/remote_control/lib/lexical/remote_control.ex
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,23 @@ defmodule Lexical.RemoteControl do
end
end

def deps_paths do
case :persistent_term.get({__MODULE__, :deps_paths}, :error) do
:error ->
{:ok, deps_paths} =
RemoteControl.Mix.in_project(fn _ ->
Mix.Task.run("loadpaths")
Mix.Project.deps_paths()
end)

:persistent_term.put({__MODULE__, :deps_paths}, deps_paths)
deps_paths

deps_paths ->
deps_paths
end
end

def with_lock(lock_type, func) do
:global.trans({lock_type, self()}, func, [Node.self()])
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,10 @@ defmodule Lexical.RemoteControl.Build.Project do
Mix.Task.run(:loadconfig)
end

with_progress "mix deps.compile", fn ->
Mix.Task.run("deps.safe_compile", ~w(--skip-umbrella-children))
unless Elixir.Features.compile_keeps_current_directory?() do
with_progress "mix deps.compile", fn ->
Mix.Task.run("deps.safe_compile", ~w(--skip-umbrella-children))
end
end

with_progress "loading plugins", fn ->
Expand Down
41 changes: 33 additions & 8 deletions apps/remote_control/lib/lexical/remote_control/code_mod/format.ex
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,11 @@ defmodule Lexical.RemoteControl.CodeMod.Format do

{formatter_function, opts} =
if RemoteControl.project_node?() do
case RemoteControl.Mix.in_project(project, fetch_formatter) do
case mix_formatter_from_task(project, file_path) do
{:ok, result} ->
result

_error ->
:error ->
formatter_opts =
case find_formatter_exs(project, file_path) do
{:ok, opts} ->
Expand Down Expand Up @@ -211,13 +211,19 @@ defmodule Lexical.RemoteControl.CodeMod.Format do
end

defp do_find_formatter_exs(root_path, current_path) do
with :error <- formatter_exs_contents(current_path) do
parent =
current_path
|> Path.join("..")
|> Path.expand()
if File.exists?(current_path) do
with :error <- formatter_exs_contents(current_path) do
parent =
current_path
|> Path.join("..")
|> Path.expand()

do_find_formatter_exs(root_path, parent)
do_find_formatter_exs(root_path, parent)
end
else
# the current path doesn't exist, it doesn't make sense to keep looking
# for the .formatter.exs in its parents. Look for one in the root directory
do_find_formatter_exs(root_path, Path.join(root_path, ".formatter.exs"))
end
end

Expand All @@ -235,4 +241,23 @@ defmodule Lexical.RemoteControl.CodeMod.Format do
:error
end
end

defp mix_formatter_from_task(%Project{} = project, file_path) do
try do
root_path = Project.root_path(project)
deps_paths = RemoteControl.deps_paths()

formatter_and_opts =
Mix.Tasks.Future.Format.formatter_for_file(file_path,
root: root_path,
deps_paths: deps_paths,
plugin_loader: fn plugins -> Enum.filter(plugins, &Code.ensure_loaded?/1) end
)

{:ok, formatter_and_opts}
rescue
_ ->
:error
end
end
end
17 changes: 14 additions & 3 deletions apps/remote_control/lib/lexical/remote_control/completion.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ defmodule Lexical.RemoteControl.Completion do
alias Lexical.RemoteControl.Completion.Candidate

import Document.Line
import Lexical.Logging

def elixir_sense_expand(%Env{} = env) do
{doc_string, position} = strip_struct_operator(env)
Expand All @@ -19,11 +20,21 @@ defmodule Lexical.RemoteControl.Completion do
if String.trim(hint) == "" do
[]
else
{_formatter, opts} = Format.formatter_for_file(env.project, env.document.path)
{_formatter, opts} =
timed_log("formatter for file", fn ->
Format.formatter_for_file(env.project, env.document.path)
end)

locals_without_parens = Keyword.fetch!(opts, :locals_without_parens)

for suggestion <- ElixirSense.suggestions(doc_string, line, character),
candidate = from_elixir_sense(suggestion, locals_without_parens),
for suggestion <-
timed_log("ES suggestions", fn ->
ElixirSense.suggestions(doc_string, line, character)
end),
candidate =
timed_log("from_elixir_sense", fn ->
from_elixir_sense(suggestion, locals_without_parens)
end),
candidate != nil do
candidate
end
Expand Down
41 changes: 39 additions & 2 deletions apps/server/lib/lexical/server/code_intelligence/completion.ex
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ defmodule Lexical.Server.CodeIntelligence.Completion do
case Env.new(project, analysis, position) do
{:ok, env} ->
completions = completions(project, env, context)
Logger.info("Emitting completions: #{inspect(completions)}")
log_candidates(completions)
maybe_to_completion_list(completions)

{:error, _} = error ->
Expand All @@ -45,6 +45,19 @@ defmodule Lexical.Server.CodeIntelligence.Completion do
end
end

defp log_candidates(candidates) do
log_iolist =
Enum.reduce(candidates, ["Emitting Completions: ["], fn %Completion.Item{} = completion,
acc ->
name = Map.get(completion, :name) || Map.get(completion, :label)
kind = completion |> Map.get(:kind, :unknown) |> to_string()

[acc, [kind, ":", name], " "]
end)

Logger.info([log_iolist, "]"])
end

defp completions(%Project{} = project, %Env{} = env, %Completion.Context{} = context) do
prefix_tokens = Env.prefix_tokens(env, 1)

Expand Down Expand Up @@ -152,7 +165,7 @@ defmodule Lexical.Server.CodeIntelligence.Completion do
%Env{} = env,
%Completion.Context{} = context
) do
Logger.info("Local completions are #{inspect(local_completions)}")
debug_local_completions(local_completions)

for result <- local_completions,
displayable?(project, result),
Expand All @@ -163,6 +176,30 @@ defmodule Lexical.Server.CodeIntelligence.Completion do
end
end

defp debug_local_completions(completions) do
completions_by_type =
Enum.group_by(completions, fn %candidate_module{} ->
candidate_module
|> Atom.to_string()
|> String.split(".")
|> List.last()
|> String.downcase()
end)

log_iodata =
Enum.reduce(completions_by_type, ["Local completions are: ["], fn {type, completions},
acc ->
names =
Enum.map_join(completions, ", ", fn candidate ->
Map.get(candidate, :name) || Map.get(candidate, :detail)
end)

[acc, [type, ": (", names], ") "]
end)

Logger.info([log_iodata, "]"])
end

defp to_completion_item(candidate, env) do
candidate
|> Translatable.translate(Builder, env)
Expand Down
Loading

0 comments on commit a9a806c

Please sign in to comment.