Skip to content

Commit

Permalink
Adjust tests for Windows + get rid off subtle time scaling bug
Browse files Browse the repository at this point in the history
We didn't scale our measured native time for determine_n to
nanoseconds until now 😱
Impact is little, as to the best of my knowledge it only affects
Windows systems and then only significantly with few runs... still
this shouldn't have happend.
  • Loading branch information
PragTob committed Mar 19, 2019
1 parent 0097083 commit 43d8b7d
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 5 deletions.
10 changes: 9 additions & 1 deletion lib/benchee/benchmark/repeated_measurement.ex
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ defmodule Benchee.Benchmark.RepeatedMeasurement do
run_time = measure_iteration(scenario, scenario_context, collector)

if run_time >= @minimum_execution_time do
{num_iterations, adjust_for_iterations(run_time, num_iterations)}
{num_iterations, report_time(run_time, num_iterations)}
else
if fast_warning, do: printer.fast_warning()

Expand All @@ -52,6 +52,14 @@ defmodule Benchee.Benchmark.RepeatedMeasurement do
end
end

# we need to convert the time here since we measure native time to see when we have enough
# repetitions but the first time is used in the actual samples
defp report_time(measurement, num_iterations) do
measurement
|> :erlang.convert_time_unit(:native, :nanosecond)
|> adjust_for_iterations(num_iterations)
end

defp adjust_for_iterations(measurement, 1), do: measurement
defp adjust_for_iterations(measurement, num_iterations), do: measurement / num_iterations

Expand Down
11 changes: 9 additions & 2 deletions test/benchee/benchmark/repeated_measurement_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,12 @@ defmodule Bencheee.Benchmark.RepeatedMeasurementTest do
determine_n_times(scenario, @scenario_context, false, FakeCollector)

assert num_iterations == 10

# 50 adjusted by the 10 iteration factor
assert time == 5
# we need to use convert_time_unit cos windows is funny, see:
# https://twitter.com/PragTob/status/1108024728734838784
expected_time = :erlang.convert_time_unit(5, :native, :nanosecond)
assert_in_delta time, expected_time, 1

# 1 initial + 10 more after repeat
assert_received_exactly_n_times(:called, 11)
Expand All @@ -54,7 +58,10 @@ defmodule Bencheee.Benchmark.RepeatedMeasurementTest do
determine_n_times(scenario, @scenario_context, false, FakeCollector)

assert num_iterations == 1
assert time == 10

# Why erlang time conversion? See test above.
expected_time = :erlang.convert_time_unit(10, :native, :nanosecond)
assert_in_delta time, expected_time, 1

# 1 initial + 10 more after repeat
assert_received_exactly_n_times(:called, 1)
Expand Down
3 changes: 1 addition & 2 deletions test/benchee_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ defmodule BencheeTest do

assert output =~ ~r/fast/
assert output =~ ~r/unreliable/
assert output =~ ~r/^Constant\s+\d+.+\s+[0-2]\.\d+ ns/m
end

@tag :needs_fast_function_repetition
Expand Down Expand Up @@ -804,7 +803,7 @@ defmodule BencheeTest do
end
end

@slower_regex "\\s+- \\d+\\.\\d+x slower - \\+\\d+\\.\\d+.+"
@slower_regex "\\s+- \\d+\\.\\d+x slower - \\+\\d+(\\.\\d+)?.+"
defp readme_sample_asserts(output, tag_string \\ "") do
assert output =~ "warmup: 5 ms"
assert output =~ "time: 10 ms"
Expand Down

0 comments on commit 43d8b7d

Please sign in to comment.