From 8a171303915e51cadc48140cede4bac588e2b9fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 16 Apr 2021 12:31:25 +0200 Subject: [PATCH] Improvements to runtime initialization (#201) 1. Do not use nouse_stdio as it causes slowdowns when IEx is also running 2. Reduce the amount of generated random atoms by using the child_node as the name of the parent process 3. Do not pass quoted strings nor newlines to Windows to eval, use argv instead Closes #194. --- lib/livebook/runtime/elixir_standalone.ex | 15 +++--- lib/livebook/runtime/mix_standalone.ex | 19 ++++--- lib/livebook/runtime/standalone_init.ex | 65 ++++++++++------------- 3 files changed, 45 insertions(+), 54 deletions(-) diff --git a/lib/livebook/runtime/elixir_standalone.ex b/lib/livebook/runtime/elixir_standalone.ex index ad7707e3029..7c700719bde 100644 --- a/lib/livebook/runtime/elixir_standalone.ex +++ b/lib/livebook/runtime/elixir_standalone.ex @@ -31,13 +31,13 @@ defmodule Livebook.Runtime.ElixirStandalone do @spec init() :: {:ok, t()} | {:error, String.t()} def init() do parent_node = node() - child_node = random_node_name() - parent_process_name = random_process_name() + child_node = child_node_name(parent_node) + + Utils.temporarily_register(self(), child_node, fn -> + argv = [parent_node] - Utils.temporarily_register(self(), parent_process_name, fn -> with {:ok, elixir_path} <- find_elixir_executable(), - eval <- child_node_ast(parent_node, parent_process_name) |> Macro.to_string(), - port <- start_elixir_node(elixir_path, child_node, eval), + port = start_elixir_node(elixir_path, child_node, child_node_eval_string(), argv), {:ok, primary_pid} <- parent_init_sequence(child_node, port) do runtime = %__MODULE__{ node: child_node, @@ -52,13 +52,12 @@ defmodule Livebook.Runtime.ElixirStandalone do end) end - defp start_elixir_node(elixir_path, node_name, eval) do + defp start_elixir_node(elixir_path, node_name, eval, argv) do # Here we create a port to start the system process in a non-blocking way. Port.open({:spawn_executable, elixir_path}, [ :binary, - :nouse_stdio, :hide, - args: elixir_flags(node_name) ++ ["--eval", eval] + args: elixir_flags(node_name) ++ ["--eval", eval, "--" | Enum.map(argv, &to_string/1)] ]) end end diff --git a/lib/livebook/runtime/mix_standalone.ex b/lib/livebook/runtime/mix_standalone.ex index 4fdeb3229fb..f7dfc23a3a0 100644 --- a/lib/livebook/runtime/mix_standalone.ex +++ b/lib/livebook/runtime/mix_standalone.ex @@ -46,15 +46,16 @@ defmodule Livebook.Runtime.MixStandalone do spawn_link(fn -> parent_node = node() - child_node = random_node_name() - parent_process_name = random_process_name() + child_node = child_node_name(parent_node) + + Utils.temporarily_register(self(), child_node, fn -> + argv = [parent_node] - Utils.temporarily_register(self(), parent_process_name, fn -> with {:ok, elixir_path} <- find_elixir_executable(), :ok <- run_mix_task("deps.get", project_path, output_emitter), :ok <- run_mix_task("compile", project_path, output_emitter), - eval <- child_node_ast(parent_node, parent_process_name) |> Macro.to_string(), - port <- start_elixir_mix_node(elixir_path, child_node, eval, project_path), + eval = child_node_eval_string(), + port = start_elixir_mix_node(elixir_path, child_node, eval, argv, project_path), {:ok, primary_pid} <- parent_init_sequence(child_node, port, output_emitter) do runtime = %__MODULE__{ node: child_node, @@ -86,14 +87,16 @@ defmodule Livebook.Runtime.MixStandalone do end end - defp start_elixir_mix_node(elixir_path, node_name, eval, project_path) do + defp start_elixir_mix_node(elixir_path, node_name, eval, argv, project_path) do # Here we create a port to start the system process in a non-blocking way. Port.open({:spawn_executable, elixir_path}, [ :binary, :stderr_to_stdout, :hide, - args: elixir_flags(node_name) ++ ["-S", "mix", "run", "--eval", eval], - cd: project_path + cd: project_path, + args: + elixir_flags(node_name) ++ + ["-S", "mix", "run", "--eval", eval, "--" | Enum.map(argv, &to_string/1)] ]) end end diff --git a/lib/livebook/runtime/standalone_init.ex b/lib/livebook/runtime/standalone_init.ex index db81fc69c70..72a18f7eb0c 100644 --- a/lib/livebook/runtime/standalone_init.ex +++ b/lib/livebook/runtime/standalone_init.ex @@ -11,22 +11,9 @@ defmodule Livebook.Runtime.StandaloneInit do @doc """ Returns a random name for a dynamically spawned node. """ - @spec random_node_name() :: atom() - def random_node_name() do - Utils.node_from_name("livebook_runtime_#{Utils.random_short_id()}") - end - - @doc """ - Returns random name to register a process under. - - We have to pass parent process pid to the new Elixir node. - The node receives code to evaluate as string, so we cannot - directly embed the pid there, but we can temporarily register - the process under a random name and pass this name to the child node. - """ - @spec random_process_name() :: atom() - def random_process_name() do - :"livebook_parent_process_name_#{Utils.random_short_id()}" + @spec child_node_name(atom()) :: atom() + def child_node_name(parent) do + :"#{Utils.random_short_id()}-#{parent}" end @doc """ @@ -124,31 +111,33 @@ defmodule Livebook.Runtime.StandaloneInit do loop.(loop) end + # Note Windows does not handle escaped quotes and newlines the same way as Unix, + # so the string cannot have constructs newlines nor strings. That's why we pass + # the parent node name as ARGV and write the code avoiding newlines. + @child_node_eval_string """ + [parent_node] = System.argv();\ + init_ref = make_ref();\ + parent_process = {node(), String.to_atom(parent_node)};\ + send(parent_process, {:node_started, init_ref, node(), self()});\ + receive do {:node_initialized, ^init_ref} ->\ + manager_ref = Process.monitor(Livebook.Runtime.ErlDist.Manager);\ + receive do {:DOWN, ^manager_ref, :process, _object, _reason} -> :ok end;\ + after 10_000 ->\ + :timeout;\ + end\ + """ + + if @child_node_eval_string =~ "\n" do + raise "invalid @child_node_eval_string, newline found: #{inspect(@child_node_eval_string)}" + end + @doc """ Performs the child side of the initialization contract. This function returns AST that should be evaluated in primary - process on the newly spawned child node. + process on the newly spawned child node. The executed code expects + the parent_node on ARGV. The process on the parent node is assumed + to have the same name as the child node. """ - def child_node_ast(parent_node, parent_process_name) do - # This is the primary process, so as soon as it finishes, the runtime terminates. - quote do - # Initiate communication with the parent process (on the parent node). - init_ref = make_ref() - parent_process = {unquote(parent_process_name), unquote(parent_node)} - send(parent_process, {:node_started, init_ref, node(), self()}) - - receive do - {:node_initialized, ^init_ref} -> - manager_ref = Process.monitor(Livebook.Runtime.ErlDist.Manager) - - # Wait until the Manager process terminates. - receive do - {:DOWN, ^manager_ref, :process, _object, _reason} -> :ok - end - after - 10_000 -> :timeout - end - end - end + def child_node_eval_string(), do: @child_node_eval_string end