bazelbuild / rules_scala

Scala rules for Bazel
Apache License 2.0
363 stars 274 forks source link

jmh code might be unreproducible #699

Open ittaiz opened 5 years ago

ittaiz commented 5 years ago

One of the travis builds failed on the reproducibility test without apparent relation to the change (I think). Makes me think maybe it just surfaced there: https://travis-ci.org/bazelbuild/rules_scala/jobs/497218553

< 47c3d415c9e5dbe424c573d08f40da1e  bazel-bin/test/jmh/test_benchmark_compiled_benchmark_lib.jar
< 47c3d415c9e5dbe424c573d08f40da1e  bazel-bin/test/jmh/test_benchmark.runfiles/io_bazel_rules_scala/test/jmh/test_benchmark_compiled_benchmark_lib.jar
1501a1500
> 4f5fa74aa94da701176f042a1e47c841  bazel-bin/test/jmh/test_benchmark_codegen_resources.jar
5393a5393,5394
> d9a26e038170d9e26c593555af311b79  bazel-bin/test/jmh/test_benchmark_compiled_benchmark_lib.jar
> d9a26e038170d9e26c593555af311b79  bazel-bin/test/jmh/test_benchmark.runfiles/io_bazel_rules_scala/test/jmh/test_benchmark_compiled_benchmark_lib.jar
7596d7596
< ef6ee9a44b47c199f47ee962eaf89e7f  bazel-bin/test/jmh/test_benchmark_codegen_resources.jar

@johnynek wdyt?

ittaiz commented 5 years ago

mainly since the PR passed and then I pushed two commits and it broke: https://github.com/bazelbuild/rules_scala/pull/698/commits/b7979bac2d9f2809849ef81085e8b24d1a753d49 https://github.com/bazelbuild/rules_scala/pull/698/commits/a530f8b29b201561624127c7b0bea9c9b658dc61 They seem really unrelated

ittaiz commented 5 years ago

indeed passed after rerun without code changing

johnynek commented 5 years ago

yeah, it looks like it is unreproducible. Fortunately, we never depend on a benchmark, we only ever run them, so I guess it is not totally critical to fix, but it would be nice to.

Surprising this pops up now years later.

ittaiz commented 5 years ago

yeah, very surprising. I agree about the lack of urgency. maybe we should exclude it from the reproducibility test for now?

johnynek commented 5 years ago

I just noticed that the shas are always the same:

< 47c3d415c9e5dbe424c573d08f40da1e  bazel-bin/test/jmh/test_benchmark_compiled_benchmark_lib.jar
< 47c3d415c9e5dbe424c573d08f40da1e  bazel-bin/test/jmh/test_benchmark.runfiles/io_bazel_rules_scala/test/jmh/test_benchmark_compiled_benchmark_lib.jar
1620a1619
> 4f5fa74aa94da701176f042a1e47c841  bazel-bin/test/jmh/test_benchmark_codegen_resources.jar
5836a5836,5837
> d9a26e038170d9e26c593555af311b79  bazel-bin/test/jmh/test_benchmark_compiled_benchmark_lib.jar
> d9a26e038170d9e26c593555af311b79  bazel-bin/test/jmh/test_benchmark.runfiles/io_bazel_rules_scala/test/jmh/test_benchmark_compiled_benchmark_lib.jar
6954d6954
< ef6ee9a44b47c199f47ee962eaf89e7f  bazel-bin/test/jmh/test_benchmark_codegen_resources.jar

that's the linux build. Note, they are reproducibly wrong. The first hash above matches the one in the original issue, as does the second.

This suggests it should be debuggable.

benjaminp commented 5 years ago

This seems to be because the JMH generator holds the benchmarks list in a hashmap, which results in the benchmark set being emitted in a non-deterministic order.

johnynek commented 5 years ago

Nice find. Would be interesting to see if they would take a PR to make it a sorted set or a list. Did you find a line number somewhere to look at?

benjaminp commented 5 years ago

Thereabouts https://hg.openjdk.java.net/code-tools/jmh/file/99d7b73cf1e3/jmh-core/src/main/java/org/openjdk/jmh/generators/core/BenchmarkGenerator.java#l123

On Thu, Jul 18, 2019, at 21:23, P. Oscar Boykin wrote:

Nice find. Would be interesting to see if they would take a PR to make it a sorted set or a list. Did you find a line number somewhere to look at?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/issues/699?email_source=notifications&email_token=AABVSTRD6S4MBSJN6Q4IHC3QAE6SLA5CNFSM4GZJKUP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2KQ4EA#issuecomment-513084944, or mute the thread https://github.com/notifications/unsubscribe-auth/AABVSTVAXD3GRMVWDJVAKUDQAE6SLANCNFSM4GZJKUPQ.