From 6972c08282f4e0bc911b44d886b648c8cd5cbe8e Mon Sep 17 00:00:00 2001 From: Tobias Pfeiffer Date: Sun, 13 Jan 2019 12:20:52 +0100 Subject: [PATCH] Provide a sorted order of scenarios for all formatters Before every formatter had to call `Statistics.sort` by themselves, which is unnecessary work for the formatters. I think it comes from a time where we had maps instead of lists. But now we can provide an order. This should also go towards some consistency in which scenarios are displayed. Moreover we can make `Statistics.sort` private limiting our possible interaction surface. --- lib/benchee/formatters/console.ex | 14 +++--- lib/benchee/statistics.ex | 32 ++++-------- test/benchee/formatters/console_test.exs | 62 ++++++++++++------------ test/benchee/statistics_test.exs | 47 +++++++++++++----- 4 files changed, 82 insertions(+), 73 deletions(-) diff --git a/lib/benchee/formatters/console.ex b/lib/benchee/formatters/console.ex index dec04dfa..600859c8 100644 --- a/lib/benchee/formatters/console.ex +++ b/lib/benchee/formatters/console.ex @@ -6,7 +6,7 @@ defmodule Benchee.Formatters.Console do @behaviour Benchee.Formatter - alias Benchee.{Statistics, Suite} + alias Benchee.Suite alias Benchee.Formatters.Console.{Memory, RunTime} def format(suite), do: format(suite, %{}) @@ -71,22 +71,22 @@ defmodule Benchee.Formatters.Console do scenarios |> Enum.reduce([], &update_grouped_list/2) - |> Enum.reverse() |> Enum.map(fn {input, scenarios} -> - scenarios - |> Statistics.sort() - |> generate_output(config, input) + generate_output(scenarios, config, input) end) end + # Normally one would prepend to lists and not append. In this case this lead to 2 + # `Enum.reverse` scattered around. As these lists are usually very small (mostly less + # than 10 elements) I opted for `++` here. defp update_grouped_list(scenario, grouped_scenarios) do case List.keyfind(grouped_scenarios, scenario.input_name, 0) do {_, group} -> - new_tuple = {scenario.input_name, [scenario | group]} + new_tuple = {scenario.input_name, group ++ [scenario]} List.keyreplace(grouped_scenarios, scenario.input_name, 0, new_tuple) _ -> - [{scenario.input_name, [scenario]} | grouped_scenarios] + grouped_scenarios ++ [{scenario.input_name, [scenario]}] end end diff --git a/lib/benchee/statistics.ex b/lib/benchee/statistics.ex index 49fc07f5..7e2bc874 100644 --- a/lib/benchee/statistics.ex +++ b/lib/benchee/statistics.ex @@ -4,7 +4,7 @@ defmodule Benchee.Statistics do times and then compute statistics like the average and the standard deviation. """ - alias Benchee.{Benchmark.Scenario, Conversion.Duration, Statistics, Suite, Utility.Parallel} + alias Benchee.{Benchmark.Scenario, Conversion.Duration, Suite, Utility.Parallel} alias Benchee.Statistics.Mode alias Benchee.Statistics.Percentile @@ -42,27 +42,6 @@ defmodule Benchee.Statistics do @type samples :: [number] - @doc """ - Sorts the given scenarios fastest to slowest by run_time average. - - ## Examples - - iex> scenario_1 = %Benchee.Benchmark.Scenario{run_time_statistics: %Statistics{average: 100.0}} - iex> scenario_2 = %Benchee.Benchmark.Scenario{run_time_statistics: %Statistics{average: 200.0}} - iex> scenario_3 = %Benchee.Benchmark.Scenario{run_time_statistics: %Statistics{average: 400.0}} - iex> scenarios = [scenario_2, scenario_3, scenario_1] - iex> Benchee.Statistics.sort(scenarios) - [%Benchee.Benchmark.Scenario{run_time_statistics: %Statistics{average: 100.0}}, - %Benchee.Benchmark.Scenario{run_time_statistics: %Statistics{average: 200.0}}, - %Benchee.Benchmark.Scenario{run_time_statistics: %Statistics{average: 400.0}}] - """ - @spec sort([%Scenario{}]) :: [%Scenario{}] - def sort(scenarios) do - Enum.sort_by(scenarios, fn %Scenario{run_time_statistics: %Statistics{average: average}} -> - average - end) - end - @doc """ Takes a job suite with job run times, returns a map representing the statistics of the job suite as follows: @@ -167,7 +146,7 @@ defmodule Benchee.Statistics do } end) - %Suite{suite | scenarios: scenarios_with_statistics} + %Suite{suite | scenarios: sort(scenarios_with_statistics)} end @doc """ @@ -347,4 +326,11 @@ defmodule Benchee.Statistics do %Suite{suite | scenarios: new_scenarios} end + + @spec sort([Scenario.t()]) :: [Scenario.t()] + defp sort(scenarios) do + Enum.sort_by(scenarios, fn scenario -> + {scenario.run_time_statistics.average, scenario.memory_usage_statistics.average} + end) + end end diff --git a/test/benchee/formatters/console_test.exs b/test/benchee/formatters/console_test.exs index 78bfe851..24df4e81 100644 --- a/test/benchee/formatters/console_test.exs +++ b/test/benchee/formatters/console_test.exs @@ -163,20 +163,6 @@ defmodule Benchee.Formatters.ConsoleTest do test "with multiple inputs and two jobs" do scenarios = [ - %Scenario{ - name: "Job", - input_name: "My Arg", - input: "My Arg", - run_time_statistics: %Statistics{ - average: 200.0, - ips: 5_000.0, - std_dev_ratio: 0.1, - median: 195.5, - percentiles: %{99 => 300.1}, - sample_size: 200 - }, - memory_usage_statistics: %Statistics{} - }, %Scenario{ name: "Other Job", input_name: "My Arg", @@ -193,14 +179,14 @@ defmodule Benchee.Formatters.ConsoleTest do }, %Scenario{ name: "Job", - input_name: "Other Arg", - input: "Other Arg", + input_name: "My Arg", + input: "My Arg", run_time_statistics: %Statistics{ - average: 400.0, - ips: 2_500.0, - std_dev_ratio: 0.15, - median: 395.0, - percentiles: %{99 => 500.1}, + average: 200.0, + ips: 5_000.0, + std_dev_ratio: 0.1, + median: 195.5, + percentiles: %{99 => 300.1}, sample_size: 200 }, memory_usage_statistics: %Statistics{} @@ -218,6 +204,20 @@ defmodule Benchee.Formatters.ConsoleTest do sample_size: 200 }, memory_usage_statistics: %Statistics{} + }, + %Scenario{ + name: "Job", + input_name: "Other Arg", + input: "Other Arg", + run_time_statistics: %Statistics{ + average: 400.0, + ips: 2_500.0, + std_dev_ratio: 0.15, + median: 395.0, + percentiles: %{99 => 500.1}, + sample_size: 200 + }, + memory_usage_statistics: %Statistics{} } ] @@ -242,29 +242,29 @@ defmodule Benchee.Formatters.ConsoleTest do test "with and without a tag" do scenarios = [ %Scenario{ - name: "job", + name: "job (improved)", input_name: @no_input, input: @no_input, run_time_statistics: %Statistics{ - average: 200.0, - ips: 5_000.0, + average: 100.0, + ips: 10_000.0, std_dev_ratio: 0.1, - median: 195.5, - percentiles: %{99 => 300.1}, + median: 90.0, + percentiles: %{99 => 200.1}, sample_size: 200 }, memory_usage_statistics: %Statistics{} }, %Scenario{ - name: "job (improved)", + name: "job", input_name: @no_input, input: @no_input, run_time_statistics: %Statistics{ - average: 100.0, - ips: 10_000.0, + average: 200.0, + ips: 5_000.0, std_dev_ratio: 0.1, - median: 90.0, - percentiles: %{99 => 200.1}, + median: 195.5, + percentiles: %{99 => 300.1}, sample_size: 200 }, memory_usage_statistics: %Statistics{} diff --git a/test/benchee/statistics_test.exs b/test/benchee/statistics_test.exs index 4e3140e7..f685ad57 100644 --- a/test/benchee/statistics_test.exs +++ b/test/benchee/statistics_test.exs @@ -164,6 +164,41 @@ defmodule Benchee.StatistcsTest do assert memory_stats.sample_size == 5 end + test "sorts them by their average run time fastest to slowest" do + fourth = %Scenario{name: "4", run_times: [400.1]} + second = %Scenario{name: "2", run_times: [200.0]} + third = %Scenario{name: "3", run_times: [400.0]} + first = %Scenario{name: "1", run_times: [100.0]} + scenarios = [fourth, third, second, first] + + sorted = Statistics.statistics(%Suite{scenarios: scenarios}).scenarios + + assert Enum.map(sorted, fn scenario -> scenario.name end) == ["1", "2", "3", "4"] + end + + test "sorts them by their average memory usage least to most" do + fourth = %Scenario{name: "4", memory_usages: [400.1]} + second = %Scenario{name: "2", memory_usages: [200.0]} + third = %Scenario{name: "3", memory_usages: [400.0]} + first = %Scenario{name: "1", memory_usages: [100.0]} + scenarios = [fourth, third, second, first] + + sorted = Statistics.statistics(%Suite{scenarios: scenarios}).scenarios + + assert Enum.map(sorted, fn scenario -> scenario.name end) == ["1", "2", "3", "4"] + end + + test "sorts them by their average run time using memory as a tie breaker" do + second = %Scenario{name: "2", run_times: [100.0], memory_usages: [100.0]} + third = %Scenario{name: "3", run_times: [100.0], memory_usages: [100.1]} + first = %Scenario{name: "1", run_times: [100.0], memory_usages: [99.9]} + scenarios = [third, second, first] + + sorted = Statistics.statistics(%Suite{scenarios: scenarios}).scenarios + + assert Enum.map(sorted, fn scenario -> scenario.name end) == ["1", "2", "3"] + end + defp stats_for(suite, job_name, input_name) do %Scenario{run_time_statistics: stats} = Enum.find(suite.scenarios, fn scenario -> @@ -197,16 +232,4 @@ defmodule Benchee.StatistcsTest do assert stats.mode == nil end end - - describe ".sort" do - test "sorts the benchmarks correctly and retains all data" do - fourth = %Scenario{run_time_statistics: %Statistics{average: 400.1}} - second = %Scenario{run_time_statistics: %Statistics{average: 200.0}} - third = %Scenario{run_time_statistics: %Statistics{average: 400.0}} - first = %Scenario{run_time_statistics: %Statistics{average: 100.0}} - scenarios = [fourth, second, third, first] - - assert Statistics.sort(scenarios) == [first, second, third, fourth] - end - end end