Why would introducing a panic cause a 20% performance increase
I've been nerd-sniped and have been micro-optimizing the binary parser for Paradox games (EU4, CK3, HoI4, Vic3, etc). Think of it as json in binary form, except there is no distinction between objects and arrays and a container can switch back and forth from being an array to an object.
The core loop that parses all input is less than 200 lines and is extremely hot, and performs somewhat worse than anticipated.
Yesterday, to get a sense of branch coverage in benchmarks, I introduced a panic that won't be executed:
diff --git a/src/binary/tape.rs b/src/binary/tape.rs
index 9791629..671fa83 100644
--- a/src/binary/tape.rs
+++ b/src/binary/tape.rs
@@ -325,7 +325,7 @@ impl<'a, 'b> ParserState<'a, 'b> {
match self.token_tape.get_mut(parent_ind) {
Some(x @ BinaryToken::Array(_)) => *x = BinaryToken::Array(end_idx),
Some(x @ BinaryToken::Object(_)) => *x = BinaryToken::Object(end_idx),
- _ => return Err(self.end_location_error(data)),
+ _ => panic!("I make code go fast"),
}
// And restore state based on the next ancestor
And criterion reported a large throughput increase, up to 20% (YMMV).
In disbelief I tried this change on a different machine and saw consistently better performance there too.
How you can help
Do you know why adding this panic would cause such a change in performance? Is it a red herring?
I'm also soliciting general advice for optimizing the function in question. Whether it be specific changes to the code or tools to better understand what should be focused on.
To run the benchmarks:
cargo bench -- parse/binary
Note: due to benchmarks on smaller data not reflecting gains on real data, the benchmarks will download save files from the aforementioned games. The files are then cached locally (so just a heads up when profiling that the uncached run will capture decompression).
More information
If you're interested in helping, here is what I know so far.
Executing cargo-show-asm on the before and after doesn't show large difference in total lines emitted (1061 vs 1102):
cargo asm --lib jomini::binary::tape::BinaryTapeParser::parse_slice_into_tape
However, the diff of the before and after is large (1156 lines), which has tipped me off that introducing the panic causes the generated assembly of the entire function to vary wildly. The why and how, I do not know.
Viewing the profiled benchmark in kcachegrind has not been fruitful, or at least using it for micro-optimizing a single function like this isn't yielding insights. My kcachegrind skills may be lacking, though.
Running perf stat over one of the save files downloaded by the benchmark seems like it could be a good lead:
cargo build --no-default-features --release --bin tape_binary
sudo perf stat ./target/release/tape_binary < assets/saves/jomini/ck3-bin
As the number of reported cycles tends to decrease with the panic patch as well as the reported percentage of stalled-cycles-backend.
Any help on how to progress is greatly appreciated. Part of me thinks I'm capped by memory, but this panic patch has me second guessing this.