search

bytecodealliance / sightglass

A benchmark suite and tool to compare different implementations of the same primitives.
Apache License 2.0
69 stars 33 forks source link

Weird file CI issues with benchmark_effect_size test #178

Open abrown opened 2 years ago

abrown commented 2 years ago

On Windows, the benchmark_effect_size test has started intermittently failing since #170. It is unclear to me what could have modified the execution of the code that copies the built engine to a location provided by tempfile::NamedTempFile. I added an assert in hopes that it would trigger the error sooner but the OS always thinks the file exists:

https://github.com/bytecodealliance/sightglass/blob/cd8c11b1da4c7ebe3b5b1c7fc10e3a613a9c3b7f/crates/cli/tests/tests.rs#L213

One example of this failure is here:

thread 'benchmark::benchmark_effect_size' panicked at 'Unexpected failure.
code--1073741819
...
command=`"D:\\a\\sightglass\\sightglass\\target\\debug\\sightglass-cli.exe" "benchmark" "--engine" "\\\\?\\D:\\a\\sightglass\\sightglass\\engines\\wasmtime\\engine.dll" "--engine" "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\.tmpaNVVum" "--processes" "1" "--iterations-per-process" "3" "../../benchmarks-next/noop/benchmark.wasm"`

If this random link is to be believed, the code referenced above is an access violation that might indicate referencing a null pointer, e.g. But why? The original engine library seems to run fine according to the logs; only the copied engine library has the problem.

abrown commented 2 years ago

@fitzgen, should we now consider this closed? If the CI issues with benchmark_effect_size are no longer a problem, maybe my "access violation" hypothesis was incorrect and it was the variance issue you saw?

fitzgen commented 2 years ago

Yeah sure, we can always reopen if we see it again.

abrown commented 1 year ago

I don't think this is resolved. @jlb6740 just triggered it again yesterday (logs). I'm going to disable the test on Windows until we figure out what is going on.