Add performance test #515

varsill · 2023-01-24T12:57:48Z

No description provided.

…viour. Simplify run.exs

FelonEkonom · 2023-02-27T15:56:10Z

benchmark/metric/message_queues_length.ex

+    queues_lengths_from_multiple_tries
+    |> Enum.zip()
+    |> Enum.map(&Tuple.to_list(&1))
+    |> Enum.map(&(Enum.sum(&1) / length(&1)))


Why do you divide expressions like this by 1_000_00 in other places (eg. in InProgressMemory), but not there?

I express InProgressMemory in MB, and in case of queue's length I don't think it's worth to use any unit prefix :D

FelonEkonom · 2023-02-28T09:52:41Z

benchmark/run.exs

+            [first_branch | rest_of_branches] = group_of_branches
+
+            branch_ending_with_newly_added_element =
+              first_branch
+              |> via_in(Pad.ref(:input, 0))
+              |> child(
+                {:filter, current_level.level, filter_no},
+                %BranchedFilter{
+                  number_of_reductions: reductions,
+                  generator: prepare_generator(params[:max_random]),
+                  dispatcher: prepare_dispatcher()
+                },
+                get_if_exists: true
+              )
+
+            unfinished_branches =
+              Enum.map(0..(output_pads - 1), fn pad_no ->
+                get_child({:filter, current_level.level, filter_no})
+                |> via_out(Pad.ref(:output, pad_no))
+              end)
+
+            newly_finished_branches =
+              Enum.with_index(rest_of_branches, 1)
+              |> Enum.map(fn {branch, branch_index_in_group} ->
+                branch
+                |> via_in(Pad.ref(:input, branch_index_in_group))
+                |> get_child({:filter, current_level.level, filter_no})
+              end)
+
+            {unfinished_branches,
+             newly_finished_branches ++ [branch_ending_with_newly_added_element]}


Suggested change

[first_branch | rest_of_branches] = group_of_branches

branch_ending_with_newly_added_element =

first_branch

|> via_in(Pad.ref(:input, 0))

|> child(

{:filter, current_level.level, filter_no},

%BranchedFilter{

number_of_reductions: reductions,

generator: prepare_generator(params[:max_random]),

dispatcher: prepare_dispatcher()

},

get_if_exists: true

)

unfinished_branches =

Enum.map(0..(output_pads - 1), fn pad_no ->

get_child({:filter, current_level.level, filter_no})

|> via_out(Pad.ref(:output, pad_no))

end)

newly_finished_branches =

Enum.with_index(rest_of_branches, 1)

|> Enum.map(fn {branch, branch_index_in_group} ->

branch

|> via_in(Pad.ref(:input, branch_index_in_group))

|> get_child({:filter, current_level.level, filter_no})

end)

{unfinished_branches,

newly_finished_branches ++ [branch_ending_with_newly_added_element]}

newly_finished_branches =

Enum.with_index(group_of_branches)

|> Enum.map(fn {branch, branch_index_in_group ->

branch

|> via_in(Pad.ref(:input, branch_index_in_group))

|> child(

{:filter, current_level.level, filter_no},

%BranchedFilter{

number_of_reductions: reductions,

generator: prepare_generator(params[:max_random]),

dispatcher: prepare_dispatcher()

},

get_if_exists: true

)

end)

unfinished_branches =

Enum.map(0..(output_pads - 1), fn pad_no ->

get_child({:filter, current_level.level, filter_no})

|> via_out(Pad.ref(:output, pad_no))

end)

{unfinished_branches, newly_finished_branches}

~~good spot ;)~~
[EDIT] Well, in fact it's not possible to write it that way. In the children specification (by which I mean the argument of the spec action) there cannot be duplicated children - and what get_if_exists does is that it simply does not allow to spawn children, that are already in the parent's state (that is - the children that were spawned in the previous spec actions). Briefly speaking, the specification must be "correct" as get_if_exists won't remove duplicates in the specification itself.

get_if_exists cannot affect the duplicates in the specification because we would have some kind of a undefined behaviour. Consider the following scenario:

spec = [ {child(:x, X, get_if_exists?: true), group: {:first_group, :temporary}}, {child(:x, X, get_if_exists?: true), group: {:second_group, :temporary}} ]

To which children group should the child :x belong in case get_if_exists?: true would remove the duplicates from the spec?

FelonEkonom · 2023-02-28T09:53:57Z

benchmark/run.exs

+        if rem(how_many_output_pads, input_pads) != 0,
+          do: raise("Wrong branched pipeline specification!")
+
+        {unfinished_branches_list, newly_finished_branches_list} =


Suggested change

{unfinished_branches_list, newly_finished_branches_list} =

{unfinished_branches_lists, newly_finished_branches_lists} =

FelonEkonom · 2023-02-28T09:59:37Z

benchmark/run/branched_filter.ex

+    how_many_buffers_to_output = state.generator.(length(state.buffers))
+
+    if how_many_buffers_to_output > 0 do
+      output_pads = Map.keys(ctx.pads) |> Enum.filter(&match?({Membrane.Pad, :output, _}, &1))


Suggested change

output_pads = Map.keys(ctx.pads) |> Enum.filter(&match?({Membrane.Pad, :output, _}, &1))

output_pads = Map.keys(ctx.pads) |> Enum.filter(&match?(Pad.ref(:output, _id), &1))

FelonEkonom · 2023-02-28T10:01:03Z

benchmark/run/branched_filter.ex

    state.workload_simulation.()
-    {[forward: buffer], state}
+    how_many_buffers_to_output = state.generator.(length(state.buffers))


Just a code formatting suggestion

Suggested change

how_many_buffers_to_output = state.generator.(length(state.buffers))

how_many_buffers_to_output = length(state.buffers) |> state.generator.()

done here and in the linear_filter.ex

FelonEkonom · 2023-02-28T10:09:31Z

benchmark/run/branched_filter.ex

+    {actions, rest_of_buffers} =
+      Enum.zip(output_pads, how_many_buffers_per_pad)
+      |> Enum.map_reduce(buffers, fn {pad, how_many_buffers_per_pad}, buffers_left ->
+        {buffers_for_this_pad, rest_buffers} = Enum.split(buffers_left, how_many_buffers_per_pad)
+        action = {:buffer, {pad, buffers_for_this_pad}}
+        {action, rest_buffers}
+      end)
+
+    if rest_of_buffers != [] do
+      raise("The dispatcher function is working improperly!")
+    end
+
+    actions


Second argument of Enum.zip/2 should be enumerable, but how_many_buffers_per_pad sounds like it is a number

Suggested change

{actions, rest_of_buffers} =

Enum.zip(output_pads, how_many_buffers_per_pad)

|> Enum.map_reduce(buffers, fn {pad, how_many_buffers_per_pad}, buffers_left ->

{buffers_for_this_pad, rest_buffers} = Enum.split(buffers_left, how_many_buffers_per_pad)

action = {:buffer, {pad, buffers_for_this_pad}}

{action, rest_buffers}

end)

if rest_of_buffers != [] do

raise("The dispatcher function is working improperly!")

end

actions

Enum.chunk_every(buffers, how_many_buffers_per_pad)

|> Enum.zip(output_pads)

|> Enum.map(fn {buffers_chunk, pad} -> {:buffer, {pad, buffers_chunk}} end)

Well, how_many_buffers_per_pad is not an integer - it's a list of integers which sums to the number of buffers to be dispatched. [1, 4, 3] is a valid how_many_buffers_per_pad list for 3 pads and 8 buffers, and such a list means, that one buffer should be sent through the first pad, 4 buffers should be sent through the second pad and 3 buffers should be sent through the third pad.

FelonEkonom · 2023-02-28T10:24:26Z

benchmark/run/pipeline.ex

@@ -4,13 +4,17 @@ defmodule Benchmark.Run.Pipeline do

  @impl true
  def handle_init(_ctx, options) do
-    {[spec: options[:spec]], %{monitoring_process: options[:monitoring_process]}}
+    {[spec: options[:spec]], %{monitoring_process: options[:monitoring_process], memory: []}}


Field memory in state seems to be unused

indeed, thanks for spotting

mat-hek · 2023-03-03T13:13:32Z

benchmark/run.exs

+      child(%Membrane.Testing.Source{
+        output:
+          {1,
+           fn state, _size ->
+             if state < params[:number_of_buffers] do
+               {[
+                  buffer:
+                    {:output,
+                     %Membrane.Buffer{payload: :crypto.strong_rand_bytes(params[:buffer_size])}},
+                  redemand: :output
+                ], state + 1}
+             else
+               {[end_of_stream: :output], state}
+             end
+           end}
+      }),


too much nesting

mat-hek · 2023-03-03T13:15:28Z

benchmark/run.exs

+    }
+
+    final_level =
+      Enum.reduce(struct, initial_level, fn {input_pads, output_pads}, current_level ->


body of the lambda in this reduce is giant, maybe we can split it to some private functions

mat-hek · 2023-03-03T13:21:14Z

benchmark/run/reductions.ex

+    task =
+      Task.async(fn ->
+        Enum.each(1..n, fn _x -> test_function() end)
+        :erlang.process_info(self())[:reductions]


process_info/1 (without options) is only for debugging, probably for performance reasons

Suggested change

:erlang.process_info(self())[:reductions]

Process.info(self(), :reductions) |> elem(1)

benchmark/run.exs

Co-authored-by: Feliks Pobiedziński <38541925+FelonEkonom@users.noreply.github.com>

Implement MVP of performance test

07c9e6e

varsill added the no-changelog This label has to be added if changes from the PR are not meant to be placed in the CHANGELOG.md label Jan 24, 2023

varsill added 28 commits January 24, 2023 15:07

Format the performance_test.exs

3e812e2

Implement the test in form of a mix task

6c67e61

Add CircleCI config

5399aed

Improve config

a024d83

Updated config.yml

2ea4c37

Updated config.yml

c453cda

Updated config.yml

ea4ba1d

Updated config.yml

759aefe

Add functionality to compare results of performance test

bfa957b

Merge branch 'performance_test' of https://github.com/membraneframewo…

62d9852

…rk/membrane_core into performance_test

Add missing changes in benchmark.ex

903d983

Fix bug with do_loop/3

72bcf7a

Fix bug in circleci config

2de851c

Add persistent results

7035019

bugfix with volume config

4be3c33

bugfix with volume config

6b9b841

Delete the results of the test. Add documentation

d64a1fe

Remove dependency to Membrane.Testing from the benchmark

458ec1a

Reimplement the benchmark test in form of .exs scripts

dd0aadc

Improve the steps description in circleci config

0009fb3

Improve the steps description in circleci config

43144cf

Improve the steps description in circleci config

9c6edde

Update tests

6e785c5

Update tests

5211cc6

Remove cache spec

9e999b0

Merge branch 'master' into performance_test

ba02b70

Update config

44c15a9

Merge branch 'performance_test' of https://github.com/membraneframewo…

5d283da

…rk/membrane_core into performance_test

varsill added 6 commits February 17, 2023 18:23

Add start_meassurement and stop_meassurement callbacks in Metric beha…

17ffc5d

…viour. Simplify run.exs

improve formatting

5b7b158

change the worsening factor

d5b98f9

Change the order of meassurements

0ac8e83

Change the order of meassurements

1680af9

Format

ddb8393

varsill requested a review from FelonEkonom February 27, 2023 09:44

varsill added 8 commits February 27, 2023 10:46

Change the queue lenght worsening factor:

6af563b

Improve the reduction calculation

d4d8b92

Improve the reduction calculation

7c54403

Reduce the number of reductions

864cd4b

Reduce the number of reductions

1020ab7

Reduce the number of reductions

8c74174

Change the function used to simulate workload

e85595e

Change number of retries

9864034

FelonEkonom requested changes Feb 28, 2023

View reviewed changes

varsill added 3 commits February 28, 2023 14:58

implement reviewer's suggestions

358124d

implement further reviewer's suggestions

478f806

Revoke change concerning with_branches pipeline generation

2e713d7

mat-hek requested changes Mar 3, 2023

View reviewed changes

implement reviewer's suggestions

2247a3e

varsill requested review from mat-hek and FelonEkonom March 6, 2023 12:33

Remove test case

386946e

FelonEkonom approved these changes Mar 6, 2023

View reviewed changes

benchmark/run.exs Outdated Show resolved Hide resolved

varsill and others added 2 commits March 6, 2023 14:13

Update benchmark/run.exs

65f69c7

Co-authored-by: Feliks Pobiedziński <38541925+FelonEkonom@users.noreply.github.com>

Merge branch 'master' into performance_test

3f1672c

mat-hek approved these changes Mar 13, 2023

View reviewed changes

varsill merged commit 209552c into master Mar 13, 2023

varsill deleted the performance_test branch March 13, 2023 12:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add performance test #515

Add performance test #515

varsill commented Jan 24, 2023

FelonEkonom Feb 27, 2023

varsill Feb 28, 2023

FelonEkonom Feb 28, 2023

varsill Feb 28, 2023 •

edited

Loading

FelonEkonom Feb 28, 2023

varsill Feb 28, 2023

FelonEkonom Feb 28, 2023

varsill Feb 28, 2023

FelonEkonom Feb 28, 2023

varsill Feb 28, 2023

FelonEkonom Feb 28, 2023

varsill Feb 28, 2023 •

edited

Loading

FelonEkonom Feb 28, 2023

varsill Feb 28, 2023

mat-hek Mar 3, 2023

varsill Mar 6, 2023

mat-hek Mar 3, 2023

varsill Mar 7, 2023

mat-hek Mar 3, 2023

varsill Mar 6, 2023

	{unfinished_branches_list, newly_finished_branches_list} =
	{unfinished_branches_lists, newly_finished_branches_lists} =

	output_pads = Map.keys(ctx.pads) \|> Enum.filter(&match?({Membrane.Pad, :output, _}, &1))
	output_pads = Map.keys(ctx.pads) \|> Enum.filter(&match?(Pad.ref(:output, _id), &1))

	how_many_buffers_to_output = state.generator.(length(state.buffers))
	how_many_buffers_to_output = length(state.buffers) \|> state.generator.()

	:erlang.process_info(self())[:reductions]
	Process.info(self(), :reductions) \|> elem(1)

Add performance test #515

Add performance test #515

Conversation

varsill commented Jan 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

varsill Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

varsill Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

varsill Feb 28, 2023 •

edited

Loading

varsill Feb 28, 2023 •

edited

Loading