Improve performance under batch operations

apps/remote_control/lib/lexical/remote_control/build.ex

@@ -9,7 +9,7 @@ defmodule Lexical.RemoteControl.Build do
   require Logger
   use GenServer
 
-  @tick_interval_millis 50
+  @timeout_interval_millis 50
 
   # Public interface
 
@@ -67,32 +67,30 @@ defmodule Lexical.RemoteControl.Build do
   @impl GenServer
   def handle_continue(:ensure_build_directory, %State{} = state) do
     State.ensure_build_directory(state)
-    schedule_tick()
     {:noreply, state}
   end
 
   @impl GenServer
   def handle_call({:force_compile_file, %Document{} = document}, _from, %State{} = state) do
     State.compile_file(state, document)
-    {:reply, :ok, state}
+    {:reply, :ok, state, @timeout_interval_millis}
   end
 
   @impl GenServer
   def handle_cast({:compile, force?}, %State{} = state) do
-    State.compile_project(state, force?)
-    {:noreply, state}
+    new_state = State.on_project_compile(state, force?)
+    {:noreply, new_state, @timeout_interval_millis}
   end
 
   @impl GenServer
   def handle_cast({:compile_file, %Document{} = document}, %State{} = state) do
     new_state = State.on_file_compile(state, document)
-    {:noreply, new_state}
+    {:noreply, new_state, @timeout_interval_millis}
   end
 
   @impl GenServer
-  def handle_info(:tick, %State{} = state) do
-    new_state = State.on_tick(state)
-    schedule_tick()
+  def handle_info(:timeout, %State{} = state) do
+    new_state = State.on_timeout(state)
     {:noreply, new_state}
   end
 
@@ -101,8 +99,4 @@ defmodule Lexical.RemoteControl.Build do
     Logger.warning("Undefined message: #{inspect(msg)}")
     {:noreply, project}
   end
-
-  defp schedule_tick do
-    Process.send_after(self(), :tick, @tick_interval_millis)
-  end
 end

apps/remote_control/lib/lexical/remote_control/build/state.ex

@@ -14,33 +14,47 @@ defmodule Lexical.RemoteControl.Build.State do
 
   use RemoteControl.Progress
 
-  defstruct project: nil, build_number: 0, uri_to_source_and_edit_time: %{}
+  defstruct project: nil,
+            build_number: 0,
+            uri_to_document: %{},
+            project_compile: :none
 
   def new(%Project{} = project) do
     %__MODULE__{project: project}
   end
 
-  def on_tick(%__MODULE__{} = state) do
-    {new_state, compiled_uris} =
-      Enum.reduce(state.uri_to_source_and_edit_time, {state, []}, fn
-        {uri, {document, edit_time}}, {state, compiled_uris} ->
-          if should_compile?(edit_time) do
-            new_state = increment_build_number(state)
-            compile_file(new_state, document)
-            {new_state, [uri | compiled_uris]}
-          else
-            {state, compiled_uris}
-          end
+  def on_timeout(%__MODULE__{} = state) do
+    new_state =
+      case state.project_compile do
+        :none -> state
+        :force -> compile_project(state, true)
+        :normal -> compile_project(state, false)
+      end
+
+    # We need to compile the individual documents even after the project is
+    # compiled because they might have unsaved changes, and we want that state
+    # to be the latest state of the project.
+    new_state =
+      Enum.reduce(new_state.uri_to_document, state, fn {_uri, document}, state ->
+        compile_file(state, document)
       end)
 
+    %__MODULE__{new_state | uri_to_document: %{}, project_compile: :none}
+  end
+
+  def on_file_compile(%__MODULE__{} = state, %Document{} = document) do
     %__MODULE__{
-      new_state
-      | uri_to_source_and_edit_time: Map.drop(state.uri_to_source_and_edit_time, compiled_uris)
+      state
+      | uri_to_document: Map.put(state.uri_to_document, document.uri, document)
     }
   end
 
-  def compile_scheduled?(%__MODULE__{} = state, uri) do
-    Map.has_key?(state.uri_to_source_and_edit_time, uri)
+  def on_project_compile(%__MODULE__{} = state, force?) do
+    if force? do
+      %__MODULE__{state | project_compile: :force}
+    else
+      %__MODULE__{state | project_compile: :normal}
+    end
   end
 
   def ensure_build_directory(%__MODULE__{} = state) do
@@ -65,7 +79,7 @@ defmodule Lexical.RemoteControl.Build.State do
     end
   end
 
-  def compile_project(%__MODULE__{} = state, initial?) do
+  defp compile_project(%__MODULE__{} = state, initial?) do
     state = increment_build_number(state)
     project = state.project
 
@@ -106,17 +120,12 @@ defmodule Lexical.RemoteControl.Build.State do
       RemoteControl.broadcast(diagnostics_message)
       Plugin.diagnose(project, state.build_number)
     end)
-  end
 
-  def on_file_compile(%__MODULE__{} = state, %Document{} = document) do
-    %__MODULE__{
-      state
-      | uri_to_source_and_edit_time:
-          Map.put(state.uri_to_source_and_edit_time, document.uri, {document, now()})
-    }
+    state
   end
 
   def compile_file(%__MODULE__{} = state, %Document{} = document) do
+    state = increment_build_number(state)
     project = state.project
 
     Build.with_lock(fn ->
@@ -169,6 +178,8 @@ defmodule Lexical.RemoteControl.Build.State do
       RemoteControl.broadcast(diagnostics)
       Plugin.diagnose(project, state.build_number, document)
     end)
+
+    state
   end
 
   def set_compiler_options do
@@ -201,15 +212,6 @@ defmodule Lexical.RemoteControl.Build.State do
     "Building #{Project.display_name(project)}"
   end
 
-  defp now do
-    System.system_time(:millisecond)
-  end
-
-  defp should_compile?(last_edit_time) do
-    millis_since_last_edit = now() - last_edit_time
-    millis_since_last_edit >= edit_window_millis()
-  end
-
   defp to_ms(microseconds) do
     microseconds / 1000
   end
@@ -218,10 +220,6 @@ defmodule Lexical.RemoteControl.Build.State do
     [columns: true, token_metadata: true]
   end
 
-  defp edit_window_millis do
-    Application.get_env(:remote_control, :edit_window_millis, 250)
-  end
-
   defp increment_build_number(%__MODULE__{} = state) do
     %__MODULE__{state | build_number: state.build_number + 1}
   end

apps/remote_control/test/lexical/remote_control/build/state_test.exs

@@ -6,7 +6,6 @@ defmodule Lexical.RemoteControl.Build.StateTest do
   alias Lexical.RemoteControl.Build.State
   alias Lexical.RemoteControl.Plugin
 
-  import Lexical.Test.EventualAssertions
   import Lexical.Test.Fixtures
 
   use ExUnit.Case, async: false
@@ -67,22 +66,88 @@ defmodule Lexical.RemoteControl.Build.StateTest do
     {:ok, document: document}
   end
 
-  describe "throttled compilation" do
-    setup [:with_metadata_project, :with_a_valid_document]
+  def with_patched_compilation(_) do
+    patch(Build.Document, :compile, :ok)
+    patch(Build.Project, :compile, :ok)
+    :ok
+  end
+
+  describe "throttled document compilation" do
+    setup [:with_metadata_project, :with_a_valid_document, :with_patched_compilation]
 
     test "it doesn't compile immediately", %{state: state, document: document} do
-      new_state =
-        state
-        |> State.on_file_compile(document)
-        |> State.on_tick()
+      State.on_file_compile(state, document)
+
+      refute_called(Build.Document.compile(document))
+      refute_called(Build.Project.compile(_, _))
+    end
+
+    test "it compiles files when on_timeout is called", %{state: state, document: document} do
+      state
+      |> State.on_file_compile(document)
+      |> State.on_timeout()
+
+      assert_called(Build.Document.compile(document))
+      refute_called(Build.Project.compile(_, _))
+    end
+  end
+
+  describe "throttled project compilation" do
+    setup [:with_metadata_project, :with_a_valid_document, :with_patched_compilation]
+
+    test "doesn't compile immediately if forced", %{state: state} do
+      State.on_project_compile(state, true)
+      refute_called(Build.Project.compile(_, _))
+    end
+
+    test "doesn't compile immediately", %{state: state} do
+      State.on_project_compile(state, false)
+      refute_called(Build.Project.compile(_, _))
+    end
 
-      assert State.compile_scheduled?(new_state, document.uri)
+    test "compiles if force is true after on_timeout is called", %{state: state} do
+      state
+      |> State.on_project_compile(true)
+      |> State.on_timeout()
+
+      assert_called(Build.Project.compile(_, true))
     end
 
-    test "it compiles after a timeout", %{state: state, document: document} do
-      state = State.on_file_compile(state, document)
+    test "compiles after on_timeout is called", %{state: state} do
+      state
+      |> State.on_project_compile(false)
+      |> State.on_timeout()
+
+      assert_called(Build.Project.compile(_, false))
+    end
+  end
+
+  describe "mixed compilation" do
+    setup [:with_metadata_project, :with_a_valid_document, :with_patched_compilation]
+
+    test "doesn't compile if both documents and projects are added", %{
+      state: state,
+      document: document
+    } do
+      state
+      |> State.on_project_compile(false)
+      |> State.on_file_compile(document)
+
+      refute_called(Build.Document.compile(_))
+      refute_called(Build.Project.compile(_, _))
+    end
 
-      refute_eventually(State.compile_scheduled?(State.on_tick(state), document.uri), 500)
+    test "compiles when on_timeout is called if both documents and projects are added", %{
+      state: state,
+      document: document
+    } do
+      state
+      |> State.on_project_compile(false)
+      |> State.on_file_compile(document)
+      |> State.on_timeout()
+
+      assert_called(Build.Document.compile(_))
+      assert_called(Build.Project.compile(_, _))
     end
   end
 end

apps/server/lib/lexical/convertibles/lexical.plugin.diagnostic.result.ex

@@ -41,12 +41,16 @@ defimpl Lexical.Convertible, for: Lexical.Plugin.V1.Diagnostic.Result do
     Conversions.to_lsp(range)
   end
 
-  defp lsp_range(%Diagnostic.Result{} = diagnostic) do
-    with {:ok, document} <- Document.Store.open_temporary(diagnostic.uri) do
+  defp lsp_range(%Diagnostic.Result{uri: uri} = diagnostic) when is_binary(uri) do
+    with {:ok, document} <- Document.Store.open_temporary(uri) do
       position_to_range(document, diagnostic.position)
     end
   end
 
+  defp lsp_range(%Diagnostic.Result{}) do
+    {:error, :no_uri}
+  end
+
   defp position_to_range(%Document{} = document, {start_line, start_column, end_line, end_column}) do
     start_pos = Position.new(document, start_line, max(start_column, 1))
     end_pos = Position.new(document, end_line, max(end_column, 1))

projects/lexical_shared/lib/lexical/document/store.ex

@@ -268,7 +268,7 @@ defmodule Lexical.Document.Store do
 
   @spec open_temporary(Lexical.uri() | Path.t(), timeout()) ::
           {:ok, Document.t()} | {:error, term()}
-  def open_temporary(uri, timeout \\ 5000) do
+  def open_temporary(uri, timeout \\ 5000) when is_binary(uri) do
     ProcessCache.trans(uri, 50, fn ->
       GenServer.call(name(), {:open_temporarily, uri, timeout})
     end)