perf(stdlib): use io_lib.format for string.format float conversion#319
Conversation
Replace the :erlang.float_to_binary + expand_float/2 post-processing chain in format_spec_float/2 with a single native :io_lib.format/2 call, and route the %e/%g mantissa through the same path. Precision-0 gets a dedicated round-half-to-even clause (io_lib raises on ~.*f with P=0 on OTP 29), matching C Lua; 0/0 (the :nan atom) now formats as "nan" instead of raising. Deletes expand_float/2, round_mantissa/2 and normalize_mantissa/2. Plan: B14 Closes #311
davydog187
left a comment
There was a problem hiding this comment.
Automated review (skeptical senior pass). Verdict: clean-with-nits.
I fetched the PR head (816514d), read the migrated string.ex, and sanity-checked :io_lib.format output against PUC-Lua (/opt/homebrew/bin/lua, 5.5 — identical printf delegation for %f/%e/%g) for the rounding-sensitive cases. The conversion is byte-for-byte correct across everything I tried.
Correctness — verified against C Lua (all pass)
%.0fround-half-to-even (round_half_even/1,string.ex:487):2.5→2,0.5→0,1.5→2,3.5→4,4.5→4,-2.5→-2,2.4→2,2.6→3,1000000.5→1000000— all match C Lua. The.5ties are exactly representable in IEEE754 sofrac == 0.5is exact; no FP-precision trap. Above 2^52 there is no.5tie sofracis0.0— safe.- Negative-rounds-to-zero sign:
%.0fof-0.4 → "-0",-0.6 → "-1"match C Lua (sign preserved for negative non-zero inputs). - Large precision (
fixed_float/2,:io_lib.format(~c"~.*f", ...)):%.20fof1.0 → 1.00000000000000000000,0.1 → 0.10000000000000000555— match C Lua exactly. %e/%gmantissa viamantissa_with_carry/3(string.ex:529): verified%.6e 1.0 → 1.000000e+00, the 9.99→10 carry (%.2e 9.999999 → 1.00e+01,%.6e 9.9999999 → 1.000000e+01),%.0e, and1.5e-10 → 1.500000e-10— all match. 2-digit exponent padding (e+00) correctly retained;:io_lib's single-digit exponents are not used. Good call keepingformat_scientific_str/2.%gtrailing-zero stripping + boundary:100000.0→100000,1000000.0→1e+06,0.0001→0.0001,0.00001→1e-05,0.1→0.1,1.0→1,99999.99→100000(carry),1234567.0→1.23457e+06— all match C Lua. C's%gtrailing-zero strip is preserved viastrip_trailing_zeros{,_scientific}/1.- nan:
0/0surfaces as the:nanatom (executor.ex:2559) and the newformat_spec_float(:nan, _)clause returns"nan", matching C Lua; no longer raises. Verified the VM produces:nanfor0/0. - inf: this VM clamps
1/0 → 1.0e308/-1/0 → -1.0e308(executor.ex:2560-2561,math.huge-consistent), so a non-finite value never reaches the formatter.%fof1.0e308produces identical bytes (316 chars) to C Lua's finite output. The C-Luainf/-infdivergence is a pre-existing VM-wide design decision, correctly scoped out.
Findings
-
[minor]
-0.0input loses its sign (fixed_float/2,string.ex:469&474).sign = if float_val < 0.0— but-0.0 < 0.0isfalseon the BEAM, so%fof literal-0.0yields0.000000and%.0fyields0. C Lua emits-0.000000/-0, and the oldfloat_to_binarypath did preserve it (float_to_binary(-0.0) → "-0.0"). This is a genuine regression vs both C Lua and prior behavior, though narrow: it only triggers when the input is exactly-0.0(e.g.string.format("%f", -0.0)or-1*0.0); negative values that round to zero (-0.4) are handled correctly. No existing or new test covers-0.0, so the suite stays green. Cheap fix: detect negative-zero via something likefloat_val < 0.0 or (float_val == 0.0 and 1.0/float_val < 0.0)(or match the sign bit). Worth a one-line test pinningstring.format("%f", -0.0) == "-0.000000"either way. -
[nit] Plan id
B14used as commit SCOPE.chore(B14): start plan/chore(B14): mark plan as reviewviolate the repo rule that commit subjects use the affected subsystem as scope, never the plan id. The substantiveperf(stdlib): ...commit is correctly scoped and keepsPlan: B14in the body — these are just the housekeeping commits, but the scope convention still applies.
Convention / scope checks (clean)
- No dangling callers of
expand_float/2,round_mantissa/2, ornormalize_mantissa/2— all three fully removed. - No plan-id (
B14) leakage in source or test files. - No
Co-Authored-ByAI trailer. - Scope confined to the plan file,
string.ex, andstring_test.exs— no stray edits into sibling-PR (#316/#317) territory. - New tests added per the issue: precision-0 sign-boundary set,
%.20flarge precision, and0/0 → "nan". (Note: large-precision is covered only for1.0; an irrational like%.20fof0.1would be a stronger pin, and a-0.0case is the gap called out above.)
Nothing here is a blocker. Recommend addressing the -0.0 sign before merge (or consciously documenting it as an accepted edge), and renaming the chore(B14) commit scopes.
Derive the printed sign from the IEEE-754 sign bit instead of `< 0.0`. On the BEAM `-0.0 < 0.0` is false, so the old check dropped the sign of exact negative zero, yielding "0.000000" where C printf / PUC-Lua give "-0.000000". Applied to the %f, %e/%E, and %g/%G paths. Plan: B14 Addresses the PR #319 review finding on fixed_float/2 negative-zero sign.
|
Fixed the negative-zero sign loss in
|
use io_lib.format for string.format float conversion
Plan:
.agents/plans/B14-string-format-iolib-float.mdCloses #311
Goal
Cut the per-call cost of
string.formatfloat specifiers by delegating%f(and the%e/%gmantissa) to a single native:io_lib.format/2call, replacing the:erlang.float_to_binary+expand_float/2post-processing chain. Correctness against C Lua is the gate.Success criteria
format_spec_float/2uses:io_lib.format(~c"~.*f", [P, abs(val)])with sign reapplied separately, plus an explicit precision-0 path. Verified: newfixed_float/2clauses.expand_float/2deleted;round_mantissa/2andnormalize_mantissa/2removed (folded intomantissa_with_carry/3). Verified:mix compile --warnings-as-errorsclean, no unused-function warnings.%frounding matches C Lua (round-half-to-even) at the.5boundary:%.0fof2.5→2,0.5→0,1.5→2,3.5→4,-2.5→-2. Verified: new unit test asserts each against/opt/homebrew/bin/lua.%fof0/0→"nan"no longer raises. Verified: new unit test. (1/0never reaches as infinity in this VM — see Out of scope.)test/lua/vm/string_test.exsandtest/language/stdlib/string_test.exspass unchanged (no test encoded the old half-away precision-0 behavior). Verified: both files green.%.20flarge precision, andnan. Verified:describe "string.format float rounding and non-finite values".mix compile --warnings-as-errorsclean.mix testfull suite green, no regression (2117 passed).mix test --only lua53pass count unchanged (17 passed, 12 skipped — same as main baseline).Changes
Verification output (tail)
Out of scope
%e/%gexponent assembly —:io_lib's single-digit exponents are not C-compatible;format_scientific_str/2's 2-digit padding is kept.1/0/-1/0"inf" formatting: this VM clamps1/0to the finite float1.0e308rather than IEEE infinity, so a non-finite path is never reached. Recorded in the plan's Discoveries.apply_width_flags/2; sibling-issue (perf(stdlib): parse string.format flags into bitmask, integer specifier #309/perf(stdlib): defer string.format width padding to iolist #310) work outside float conversion.benchmarks/string_format.exsrequires:luerl, which is unavailable in this worktree's deps.