ci: consumable, contestant-focused benchmark reporting - #108
Open
dfa1 wants to merge 5 commits into
Open
Conversation
Step Summary alone requires a signed-in browser session to view and isn't retrievable via the API. Tee the comparison table into the job log too, and upload the raw JMH JSON + rendered table as a per-classifier artifact for offline diffing.
Windows' console code page isn't UTF-8 by default, so the '±' in the comparison table garbled to '?' on windows-x86_64 runs.
…cost The LTO-diff workflow only ever compared the same code built two ways. Add a contestants report (zstd-java byte[]/MemorySegment vs zstd-jni vs aircompressor) from the baseline build, plus reused-vs-fresh-per-call native context overhead via two new JMH benchmark methods per class. Tighten the JMH filter to anchor on the package-separator dot so CompressBenchmark no longer also matches MultiThreadCompressBenchmark, which has no zstdJni/aircompressor peers and only produced empty cells.
LTO was already investigated and rejected (CHANGELOG.md: regression on x86_64, and zig's Mach-O linker doesn't support it on macOS at all, issues #70/#77) - this was leftover machinery from that closed investigation, not something anyone runs against a real branch (experiment/lto-linux never existed). Renamed benchmark-lto.yml -> benchmark.yml since it's no longer LTO-specific, dropped compare-benchmarks.py (its only caller), and simplified back to a single checkout/build/run per platform.
LargeFileBenchmark only ever compared the mmap + MemorySegment path against zstd-jni's stream API, never against this library's own ZstdOutputStream - so the mmap path's gain over our own conventional stream API was invisible. Add zstdJavaStream (same buffered-read/ heap-write shape as zstdJniStream) to close that gap. Verified: compiles, runs clean at size=4194304, and the exact write()-loop-through-ZstdOutputStream pattern round-trips byte-for-byte through Zstd.decompress (checked standalone, not asserted in the benchmark itself - JMH benchmarks here aren't correctness tests, ZstdOutputStream's correctness is already covered by ZstdStreamTest).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
benchmark-lto.yml(compared the same code built two ways — an already-closed investigation, see CHANGELOG's rejected-LTO entry) withbenchmark.yml: aworkflow_dispatchjob that reports zstd-java (byte[]andMemorySegment) vs zstd-jni vs aircompressor on identical input, across all 4 supported platforms.CompressBenchmark/DecompressBenchmark), quantifying the cost of not following this library's reuse-the-context recommendation.gh run view --log) and uploads as a per-classifier artifact (raw JMH JSON + rendered markdown) viagh run download.compare-benchmarks.py's±mangled to�on Windows (default console code page isn't UTF-8) — same fix (sys.stdout.reconfigure(encoding="utf-8")) applied to the newformat-contestants.py.zstdJavaStreamtoLargeFileBenchmark— it previously compared the mmap+MemorySegmentpath only against zstd-jni's stream, never against this library's ownZstdOutputStream, so the mmap path's gain over our own conventional API was invisible.Verified
MultiThreadCompressBenchmarkleakage), log + artifact output both work, Windows±renders correctly, and results are directionally sane (zstd-java'sMemorySegmentpath leads compress on all 4 platforms; context reuse consistently wins, most sharply on Windows compress at -33%).zstdJavaStream's exact write-loop pattern round-tripped byte-for-byte throughZstd.decompressin a standalone check (ZstdOutputStreamitself is already covered byZstdStreamTest)../mvnw validate(checkstyle) andactionlintboth clean.Test plan
benchmark.ymlonmainonce merged (can'tworkflow_dispatcha renamed file until it exists on the default branch) to confirm the simplified single-checkout shape runs clean end-to-end.