error: switch to structured error types (internally) #453
+5,694
−3,919
Conversation
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
This reduces code size somewhat. There are probably more things like this, but I picked this one to test its effects. It got rid of a few LLVM lines from an LTO-compiled binary that uses Jiff. But not much.
This doesn't seem to have much impact as shown via a `cargo llvm-lines`. But this still seems like good sense to have.
Both of these functions are marked as `inline`, but they can be quite beefy. Instead, set the inlineable implementation as the common case, and put the rest behind a non-inlineable function.
These all generally call `BrokenDownTime::to_date`, which adds a fair bit of code. Since these are the uncommon case, let's not inline that goop.
I was playing with using this to avoid parametric polymorphism costs, but I couldn't find my way to a concrete benefit. Either way, this impl should exist for people that want to use it.
BurntSushi
force-pushed
the
ag/error-type-experiment
branch
from
43fe6f2 to
1d44e8d
Compare
This was an incredibly tedious and tortuous refactor. But this removes almost all of the "create ad hoc stringly-typed errors everywhere." This partially makes progress toward #418, but my initial impetus for doing this was to see if I could reduce binary size and improve compilation times. My general target was to see if I could reduce total LLVM lines. I tested this with [Biff] using this command in the root of the Biff repo: ``` cargo llvm-lines --profile release-lto ``` Before this change, Biff had 768,596 LLVM lines. With this change, it has 757,331 lines. So... an improvement, but a very modest one. What about compilation times? This does seem to translate to---also a modest---improvement. For compiling release builds of Biff. Before: ``` $ hyperfine -w1 --prepare 'cargo clean' 'cargo b -r' Benchmark 1: cargo b -r Time (mean ± σ): 7.776 s ± 0.052 s [User: 65.876 s, System: 2.621 s] Range (min … max): 7.690 s … 7.862 s 10 runs ``` After: ``` $ hyperfine -w1 --prepare 'cargo clean' 'cargo b -r' Benchmark 1: cargo b -r Time (mean ± σ): 7.591 s ± 0.067 s [User: 65.686 s, System: 2.564 s] Range (min … max): 7.504 s … 7.689 s 10 runs ``` What about dev builds? Before: ``` $ hyperfine -w1 --prepare 'cargo clean' 'cargo b' Benchmark 1: cargo b Time (mean ± σ): 4.074 s ± 0.022 s [User: 14.493 s, System: 1.818 s] Range (min … max): 4.037 s … 4.099 s 10 runs ``` After: ``` $ hyperfine -w1 --prepare 'cargo clean' 'cargo b' Benchmark 1: cargo b Time (mean ± σ): 4.541 s ± 0.027 s [User: 15.385 s, System: 2.081 s] Range (min … max): 4.503 s … 4.591 s 10 runs ``` Well... that's disappointing. A modest improvement to release builds, but a fairly large regression in dev builds. Maybe it's because of the additional hand-written impls for new structured error types? Bah. And binary size? Normal release builds (not LTO) of Biff that were stripped were 4,431,456 bytes before this change and 4,392,064 after. Hopefully this will unlock other improvements to justify doing this. Note also that this slims down a number of error messages. [Biff]: https://github.com/BurntSushi/biff
BurntSushi
force-pushed
the
ag/error-type-experiment
branch
from
1d44e8d to
17dd4ba
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This was an incredibly tedious and tortuous refactor. But this removes
almost all of the "create ad hoc stringly-typed errors everywhere."
This partially makes progress toward #418, but my initial impetus for
doing this was to see if I could reduce binary size and improve
compilation times. My general target was to see if I could reduce total
LLVM lines. I tested this with Biff using this command in the root of
the Biff repo:
Before this change, Biff had 768,596 LLVM lines. With this change, it
has 757,331 lines. So... an improvement, but a very modest one.
What about compilation times? This does seem to translate to---also a
modest---improvement. For compiling release builds of Biff. Before:
After:
What about dev builds? Before:
After:
Well... that's disappointing. A modest improvement to release builds,
but a fairly large regression in dev builds. Maybe it's because of the
additional hand-written impls for new structured error types? Bah.
And binary size? Normal release builds (not LTO) of Biff that were
stripped were 4,431,456 bytes before this change and 4,392,064 after.
Hopefully this will unlock other improvements to justify doing this.
Note also that this slims down a number of error messages.