-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimizations #96
base: master
Are you sure you want to change the base?
Optimizations #96
Conversation
Triggers UBSAN failures. You should probably have that in your CI... |
Yes, the GitHub CI does not run any tests. It is on my TODO list, but haven't got the time to add that yet. Is UBSAN/ASAN triggered only with these changes? I experimented with this and I'm not sure I'm running it right, or that I'm interpreting the results the right way. For example, comparing a $ ./benchmark.sh ./old ./new in.bin 10
x before.txt
after.txt
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
| x |
|x x x x x x x * x |
| |________________________________M_____A________|_____________________________|_______________M__A________________________________________________| |
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
N Min Max Median Avg Stddev
x 10 1.406 1.434 1.417 1.4186 0.010297788
10 1.416 1.456 1.433 1.4339 0.012931014
Difference at 95.0% confidence
0.0153 /- 0.0109827
1.07853% /- 0.774195%
(Student's t, pooled s = 0.0116888) Doesn't this mean that the new version is actually slower? Compiled for release with gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0, running on a Ubuntu 22.04.4 LTS (via WSL2 on Windows 11 23H2), with a 13th Gen Intel(R) Core(TM) i7-1370P CPU. |
UBSAN was triggered with these changes, the previous code is clean. The branchless sign extension is not OK when the input is already 64-bit. That output means that it is indeed slower. I was using GCC 13 on Debian testing running on real hardware. It could also still be a measurement artifact. I was using 20 samples. |
What is the size of your Do you get the expected null result if On an AMD 5950x with Gentoo, GCC 13 and the same ~60 MB binary with 100 samples:
Old vs new code
The performance characteristics of modern systems are just completely f***** up. Maybe I should configure it for 99% confidence and always do more samples but that takes forever to run. |
I used a 58M file I obtained by concatenating a bunch of shellcodes together. If I use the same binary I get timings that are much closer: $ ./benchmark.sh ./old ./old in.bin 10
N Min Max Median Avg Stddev
x 10 1.456 1.518 1.502 1.4941 0.021855332
10 1.452 1.527 1.502 1.4937 0.02150478
No difference proven at 95.0% confidence But this is just a small sample. These kinds of changes are small enough that if we want to accurately measure them we probably need to build some micro benchmarks. |
I managed to fix UBSAN. It turns out that sometimes |
A few cleanups, a benchmark script and some optimizations.
Disassembling a ~60MB binary on my Intel i7-12700H laptop: