Closed RReverser closed 6 days ago
benchmarks please
Benchmarking failed. Please check the workflow run for details.
Benchmarking failed. Please check the workflow run for details.
benchmarks please
Benchmarking failed. Please check the workflow run for details.
@kazimuth FWIW this is marked as draft because it's not quite ready to look at. I only created a PR because without it our CI doesn't run on branches :)
Well, looks good to me so far :)
benchmarks please
Benchmarking failed. Please check the workflow run for details.
Benchmarking failed. Please check the workflow run for details.
Callgrind benchmark in progress...
Benchmarking failed. Please check the workflow run for details.
Benchmark in progress...
Benchmark in progress...
Benchmark in progress...
benchmarks please
Callgrind benchmark in progress...
Benchmark in progress...
The failure here is the one that will be fixed by #1987.
Internal tests failures appear to be spurious/unrelated, but let's wait to merge until they're fixed just in case.
There are some regexes here that need to be updated with new strings for this, to get the auto-generated comments fixed. This is, unfortunately, kind of a pain to test, so I can do it if needed.
Yeah if you could, please do.
Internal tests failures appear to be spurious/unrelated, but let's wait to merge until they're fixed just in case.
SGTM.
I hit the "Update branch" button because I believe it will make the internal tests pass.
It did, thanks.
Description of Changes
Turns out, there are some APIs that are not covered by any of our tests, except benchmarks.
Benchmarks are executed as both actual benchmarks and as integration tests for Rust, but not for C#. This PR fixes that to ensure that both Rust and C# are tested and measured on the CI from now on.
It also makes some adjustments to integration tests themselves, in particular fixes an issue where in integration tests reducers would be called but the response would never be read, so if a reducer panics, the tests would still succeed. Additionally, on failures the SpacetimeDB logs will be read and added to the error message so that you get a useful stacktrace from the module itself.
I also had to adjust some benchmark numbers (in particular, for
ia_loop
) since they were already very slow on CI for Rust, but become even more so for C#. The new loads are much lower, but should still be very usable for benchmark comparisons.The only remaining issue is that the benchmark names have changed, and the benchmark comparison tool tries to compare mismatched pairs - I'm not sure what the solution to this is. cc @kazimuth
API and ABI breaking changes
If this is an API or ABI breaking change, please apply the corresponding GitHub label.
Expected complexity level and risk
How complicated do you think these changes are? Grade on a scale from 1 to 5, where 1 is a trivial change, and 5 is a deep-reaching and complex change.
This complexity rating applies not only to the complexity apparent in the diff, but also to its interactions with existing and future code.
If you answered more than a 2, explain what is complex about the PR, and what other components it interacts with in potentially concerning ways.
Testing
Describe any testing you've done, and any testing you'd like your reviewers to do, so that you're confident that all the changes work as expected!