Build with -O3 -fno-strict-overflow; drop -march=native - #8
Merged
Conversation
Steelmanning -march=native turned up the opposite of what we assumed, plus a bigger problem underneath it. -march=native is slower. ~12% behind plain -O3 on Zen 4, worst on the numeric loops it should help most: -16% integer, -21% float. Two runs, 21 interleaved passes, best-of. Microbenchmarks are the friendliest case it gets, so there's no reason to expect a win on real work. Dropped. The bigger find: the other four definitions were building unoptimized. Leaving RUBY_CFLAGS empty doesn't mean "use configure's default" — ruby-build exports it as CFLAGS, which supersedes configure's optflags in the compile line, so an empty value is -O0. Measured 2.2-3.5x slower across method calls, integer math, string building, hash churn and array ops. rbconfig still reports optflags: -O3 in that state, which is why it went unnoticed; the timings are what to trust. Unintended fallout from b719789, which made RUBY_CFLAGS="" unconditional where it had previously only been set under clang. Turning optimization on activates two things -O0 was silently masking, and neither is caught by "does it compile and run": -fno-strict-overflow is mandatory, not caution. These sources assume signed overflow wraps; GCC treats that as UB and exploits it from -O2 up. 1.8.7 built at -O3 without it compiles clean, runs, loads every stdlib — and evaluates 2**64 to 0, typed Fixnum. Silent wrong arithmetic. Confirmed -O2 breaks it too, and that the flag costs nothing measurable (within noise on 2.7.8, two of five benchmarks nominally faster with it). 1.9.3 additionally caps _FORTIFY_SOURCE at 2 under GCC. Ubuntu's GCC raises it to 3 whenever optimizing, and level 3's object-size inference aborts the build with "*** buffer overflow detected ***" while running the freshly built miniruby. Not a hardening regression: fortify does nothing without optimization, so at -O0 there was none at all. Scoped to 1.9.3; the rest build fine at 3. So test/build now asserts overflow arithmetic. The existing checks would have passed a Ruby computing 2**64 == 0 — verified by building the unsafe variant and watching it clear ruby --version, openssl, digest and zlib before the new assertion caught it. Failures also dump ruby-build's own log now, and strip curl's progress meter: several definitions send compiler output to that log rather than stdout, and diagnosing the 1.9.3 abort meant reproducing it by hand because the container took the log with it. Full matrix green, 12/12 on Arch and Ubuntu Noble.
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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
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.
Steelmanning
-march=nativeturned up the opposite of what we assumed, plus a bigger problem underneath it.-march=native is slower
~12% behind plain
-O3on Zen 4 (Ryzen 9 7950X), worst on exactly the numeric loops it should help. Two independent runs, 21 interleaved passes, best-of:-O3 -march=native-O3Microbenchmarks are the friendliest case
-march=nativegets — tight numeric loops are where wider instructions should pay off. It loses there, so there's no reason to expect a win on real work. Consistency and performance point the same way.The bigger find: four definitions were building unoptimized
Leaving
RUBY_CFLAGSempty doesn't mean "use configure's default". ruby-build exports it asCFLAGS, which supersedes configure's ownoptflagsin the compile line, so an empty value builds at-O0:-O0→-O3-O0→-O3RbConfig::CONFIG["optflags"]still reports-O3in that state, which is why it went unnoticed — the flag string lies, the timings don't. Unintended fallout from b719789, which madeRUBY_CFLAGS=""unconditional where it had previously only been set under clang.Two hazards that -O0 was masking
Turning optimization on activates machinery
-O0silently disables. Neither is caught by "does it compile and run":-fno-strict-overflowis mandatory, not caution. These sources assume signed overflow wraps; GCC treats that as UB and exploits it from-O2up. 1.8.7 built at-O3without it compiles cleanly, runs, loads every stdlib — and evaluates2**64to0, typedFixnum. Silent wrong arithmetic.-O2breaks it too, so this isn't-O3aggressiveness. The flag costs nothing measurable: within noise on 2.7.8, two of five benchmarks nominally faster with it.1.9.3 caps
_FORTIFY_SOURCEat 2 under GCC. Ubuntu's GCC raises it to 3 whenever optimizing, and level 3's object-size inference aborts the build with*** buffer overflow detected ***while running the freshly built miniruby. Not a hardening regression — fortify does nothing without optimization, so at-O0there was none at all; level 2 is strictly more checking than today. Scoped to 1.9.3, since the rest build fine at 3. I tested disabling fortify outright first and preferred this.test/build now asserts arithmetic
The existing checks would have passed a Ruby computing
2**64 == 0. Verified the new one earns its place by building the unsafe variant deliberately — it clearedruby --version, openssl, digest and zlib before the assertion caught2**64 wrong: 0.Failures also dump ruby-build's own log and strip curl's progress meter. Several definitions send compiler output to that log rather than stdout, and the container is
--rm, so diagnosing the 1.9.3 abort meant reproducing it by hand — the reported tail was entirely progress bars.Verification
Full matrix green, 12/12 across Arch and Ubuntu Noble, including the
ubuntu-noble 1.9.3-p551case that this change initially broke.Benchmarks are microbenchmarks on one machine (Zen 4, GCC 16 on Arch / GCC 13.3 on Ubuntu). They measure interpreter CPU paths; real dev work is diluted by I/O and GC, so expect the
-O0→-O3win to land smaller than 2-3× in a Rails suite — but in the same direction, and the correctness findings are independent of workload.