Sort component state fields in the error logs in the order from the most to the least important

.circleci/config.yml

@@ -154,6 +154,7 @@ jobs:
           at: .
       - run: cp -r benchmark/ ~/benchmark_backup/
       - run: cp mix.exs ~/benchmark_backup/
+      - run: docker pull membraneframeworklabs/docker_membrane
       - run: docker run -e MIX_ENV=benchmark -v ./:/root/app -v ~/results:/root/results -w /root/app membraneframeworklabs/docker_membrane mix do deps.get, deps.compile --force --all, run benchmark/run.exs /root/results/feature_branch_results
       - run: git checkout -f master
       - run: cp  ~/benchmark_backup/mix.exs ~/app

lib/membrane/core/bin/pad_controller.ex

@@ -51,6 +51,7 @@ defmodule Membrane.Core.Bin.PadController do
       case PadModel.get_data(state, pad_ref) do
         {:error, :unknown_pad} ->
           init_pad_data(pad_ref, pad_info, state)
+          |> Map.update!(:pad_refs, &[pad_ref | &1])
 
         # This case is for pads that were instantiated before the external link request,
         # that is in the internal link request (see `handle_internal_link_request/4`).
@@ -281,8 +282,10 @@ defmodule Membrane.Core.Bin.PadController do
   @spec handle_unlink(Pad.ref(), Core.Bin.State.t()) :: Core.Bin.State.t()
   def handle_unlink(pad_ref, state) do
     with {:ok, %{availability: :on_request}} <- PadModel.get_data(state, pad_ref) do
-      state = maybe_handle_pad_removed(pad_ref, state)
-      {pad_data, state} = PadModel.pop_data!(state, pad_ref)
+      {pad_data, state} =
+        maybe_handle_pad_removed(pad_ref, state)
+        |> Map.update!(:pad_refs, &List.delete(&1, pad_ref))
+        |> PadModel.pop_data!(pad_ref)
 
       if pad_data.endpoint do
         Message.send(pad_data.endpoint.pid, :handle_unlink, pad_data.endpoint.pad_ref)

lib/membrane/core/bin/state.ex

@@ -8,7 +8,7 @@ defmodule Membrane.Core.Bin.State do
   use Bunch
   use Bunch.Access
 
-  alias Membrane.{Child, Clock, Sync}
+  alias Membrane.{Child, Clock, Pad, Sync}
   alias Membrane.Core.Child.PadModel
   alias Membrane.Core.Parent.ChildLifeController
   alias Membrane.Core.Parent.{ChildrenModel, CrashGroup, Link}
@@ -20,6 +20,7 @@ defmodule Membrane.Core.Bin.State do
           children: ChildrenModel.children(),
           subprocess_supervisor: pid(),
           name: Membrane.Bin.name() | nil,
+          pad_refs: [Pad.ref()],
           pads_info: PadModel.pads_info() | nil,
           pads_data: PadModel.pads_data() | nil,
           parent_pid: pid,
@@ -55,6 +56,7 @@ defmodule Membrane.Core.Bin.State do
                 internal_state: nil,
                 children: %{},
                 name: nil,
+                pad_refs: [],
                 pads_info: nil,
                 pads_data: nil,
                 parent_pid: nil,

lib/membrane/core/child/pad_spec_handler.ex

@@ -21,7 +21,8 @@ defmodule Membrane.Core.Child.PadSpecHandler do
       | pads_info:
           get_pads(state)
           |> Map.new(),
-        pads_data: %{}
+        pads_data: %{},
+        pad_refs: []
     }
   end
 

lib/membrane/core/element/pad_controller.ex

@@ -228,7 +228,10 @@ defmodule Membrane.Core.Element.PadController do
       state = generate_eos_if_needed(pad_ref, state)
       state = maybe_handle_pad_removed(pad_ref, state)
       state = remove_pad_associations(pad_ref, state)
-      {pad_data, state} = PadModel.pop_data!(state, pad_ref)
+
+      {pad_data, state} =
+        Map.update!(state, :pad_refs, &List.delete(&1, pad_ref))
+        |> PadModel.pop_data!(pad_ref)
 
       with %{direction: :input, flow_control: :auto, other_effective_flow_control: :pull} <-
              pad_data do
@@ -314,7 +317,10 @@ defmodule Membrane.Core.Element.PadController do
       |> merge_pad_mode_data(endpoint.pad_props, other_pad_info, state)
       |> then(&struct!(Membrane.Element.PadData, &1))
 
-    state = put_in(state, [:pads_data, endpoint.pad_ref], pad_data)
+    state =
+      state
+      |> put_in([:pads_data, endpoint.pad_ref], pad_data)
+      |> Map.update!(:pad_refs, &[endpoint.pad_ref | &1])
 
     :ok =
       AtomicDemand.set_sender_status(

lib/membrane/core/element/state.ex

@@ -19,6 +19,7 @@ defmodule Membrane.Core.Element.State do
           type: Element.type(),
           name: Element.name(),
           internal_state: Element.state() | nil,
+          pad_refs: [Pad.ref()] | nil,
           pads_info: PadModel.pads_info() | nil,
           pads_data: PadModel.pads_data() | nil,
           parent_pid: pid,
@@ -50,6 +51,7 @@ defmodule Membrane.Core.Element.State do
     :type,
     :name,
     :internal_state,
+    :pad_refs,
     :pads_info,
     :pads_data,
     :parent_pid,

lib/membrane/core/inspect.ex

@@ -0,0 +1,92 @@
+defmodule Membrane.Core.Inspect do
+  @moduledoc false
+
+  alias Inspect.Algebra
+  alias Membrane.Core.Component
+
+  @fields_order [
+    :module,
+    :name,
+    :pid,
+    :parent_pid,
+    :playback,
+    :type,
+    :internal_state,
+    :pad_refs,
+    :pads_info,
+    :children,
+    :links,
+    :crash_groups,
+    :pending_specs,
+    :synchronization,
+    :delayed_demands,
+    :effective_flow_control,
+    :initialized?,
+    :terminating?,
+    :setup_incomplete?,
+    :supplying_demand?,
+    :handling_action?,
+    :stalker,
+    :resource_guard,
+    :subprocess_supervisor,
+    :children_log_metadata,
+    :handle_demand_loop_counter,
+    :demand_size,
+    :pads_to_snapshot,
+    :playback_queue,
+    :pads_data
+  ]
+
+  @spec inspect(Component.state(), Inspect.Opts.t()) :: Inspect.Algebra.t()
+  def inspect(state, opts) do
+    field_doc_extractor =
+      fn field, opts ->
+        case Map.fetch(state, field) do
+          {:ok, value} ->
+            value_doc = Algebra.to_doc(value, opts)
+            Algebra.concat("#{Atom.to_string(field)}: ", value_doc)
+
+          :error ->
+            Algebra.empty()
+        end
+      end
+
+    Algebra.container_doc(
+      "%#{Kernel.inspect(state.__struct__)}{",
+      @fields_order,
+      "}",
+      opts,
+      field_doc_extractor,
+      break: :strict
+    )
+  end
+
+  @spec ensure_all_struct_fields_inspected!(module()) :: :ok | no_return()
+  def ensure_all_struct_fields_inspected!(state_module) do
+    state_module.__info__(:struct)
+    |> Enum.map(& &1.field)
+    |> Enum.reject(&(&1 in @fields_order))
+    |> case do
+      [] ->
+        :ok
+
+      fields ->
+        raise """
+        Fields #{inspect(fields)} from #{inspect(state_module)} structure are omitted in @fields_order module attribute in #{inspect(__MODULE__)}.\
+        Add them to @fields_order, to define where they will occur in the error logs.
+        """
+    end
+  end
+end
+
+[Pipeline, Bin, Element]
+|> Enum.map(fn component ->
+  state_module = Module.concat([Membrane.Core, component, State])
+
+  :ok = Membrane.Core.Inspect.ensure_all_struct_fields_inspected!(state_module)
+
+  defimpl Inspect, for: state_module do
+    @spec inspect(unquote(state_module).t(), Inspect.Opts.t()) :: Inspect.Algebra.t()
+    defdelegate inspect(state, opts), to: Membrane.Core.Inspect
+  end
+end)

test/membrane/core/element/pad_controller_test.exs

@@ -61,7 +61,9 @@ defmodule Membrane.Core.Element.PadControllerTest do
                  state
                )
 
-      assert %{new_state | pads_data: nil} == %{state | pads_data: nil}
+      assert Map.drop(new_state, [:pads_data, :pad_refs]) ==
+               Map.drop(state, [:pads_data, :pad_refs])
+
       assert PadModel.assert_instance(new_state, :input) == :ok
     end