perf: profile the write path; buffer per-atom output (byte-identical) - #39
Merged
Merged
Conversation
Profiling the write path: our C writer already beats ASE's built-in extxyz writer by ~2.5x and extxyz-ng by ~1.5x, but writing a 200k-atom frame took ~266 ms vs ~33-50 ms to read it. The cost is the per-atom output loop, which did one fprintf per value (~15 per atom incl. separators/newline for a 7-column frame) -- each paying format-string parsing and a flockfile/funlockfile pair. A micro-benchmark showed the dominant cost is the %.8f float formatting itself, not I/O: buffering alone is ~1.14x while a custom integer formatter would be ~2x but cannot be byte-identical to printf's round-half-to-even (mismatches ~1 in 5M) without a fiddly correctly-rounded formatter. So this lands the safe, byte- identical win only: build each line in a growable memory buffer with snprintf and fwrite it in blocks, flushing at line boundaries. Same output bytes, far fewer locked stdio calls -- ~1.26x on a 200k write (266 -> 212 ms). Output is byte-identical (verified old-vs-new across signs, -0.0, rounding edges, ints/bools/strings, multi-column, and a custom format_dict); a golden + round-trip test locks it. The format_dict override (#22) path is unchanged. New benchmarks/bench_write.py reproduces the comparison vs ASE and extxyz-ng. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Please don't ping me in your AI slop PRs thanks |
Member
Author
Apologies. It's not exactly slop, but I take your point. 20k was referring to the system size I was benchmarking on, an accidental collision with your username. |
jameskermode
added a commit
that referenced
this pull request
Jun 10, 2026
Profiling (#39) showed the write path is bound by formatting the per-atom floats, not I/O: buffering alone was 1.14x while a custom formatter promised ~2x. This adds that formatter for the default "%16.8f", proven byte-identical to printf. A finite double is |v| = m*2^e exactly, and 10^8 = 2^8*5^8, so v*10^8 = m*390625*2^(e+8) is an exact rational. We round it to nearest (ties to even, matching printf's default) with integer-only arithmetic -- a 128-bit shift + half-bit test, no floating-point error -- then format the digits manually (no snprintf). It falls back to snprintf for non-finite / |v|>=1e15, for any custom format_dict (#22), and on compilers without __int128 (MSVC), so output is byte-identical everywhere. new libextxyz/fast_format.{c,h}; wired into the #39 buffered per-atom loop only when the float format is the default (fmt_f == NULL). Int/bool/string unchanged. Result: 200k-atom write 210 -> 84 ms (~2.4x over #39, ~3x over the original fprintf writer); now ~6x faster than ASE's writer and ~3x faster than extxyz-ng. Validation: libextxyz/test_fmt_float.c checks fmt_default_f16_8 == snprintf over 60M+ doubles incl. dense sweeps across round-half-even ties, subnormals, +/-0.0, and the fallback boundary (wired as a `meson test`); the naive round-half-up formatter fails it ~1 in 5M, the exact one passes 0/66M. tests/test_write_float_ format.py validates byte-identity end-to-end through the writer (default fast path == explicit %16.8f snprintf path) over 300k random + crafted-edge values. #39 golden/round-trip + #22 precision stay green; leaks 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jameskermode
added a commit
that referenced
this pull request
Jun 10, 2026
Profiling (#39) showed the write path is bound by formatting the per-atom floats, not I/O: buffering alone was 1.14x while a custom formatter promised ~2x. This adds that formatter for the default "%16.8f", proven byte-identical to printf. A finite double is |v| = m*2^e exactly, and 10^8 = 2^8*5^8, so v*10^8 = m*390625*2^(e+8) is an exact rational. We round it to nearest (ties to even, matching printf's default) with integer-only arithmetic -- a 128-bit shift + half-bit test, no floating-point error -- then format the digits manually (no snprintf). It falls back to snprintf for non-finite / |v|>=1e15, for any custom format_dict (#22), and on compilers without __int128 (MSVC), so output is byte-identical everywhere. new libextxyz/fast_format.{c,h}; wired into the #39 buffered per-atom loop only when the float format is the default (fmt_f == NULL). Int/bool/string unchanged. Result: 200k-atom write 210 -> 84 ms (~2.4x over #39, ~3x over the original fprintf writer); now ~6x faster than ASE's writer and ~3x faster than extxyz-ng. Validation: libextxyz/test_fmt_float.c checks fmt_default_f16_8 == snprintf over 60M+ doubles incl. dense sweeps across round-half-even ties, subnormals, +/-0.0, and the fallback boundary (wired as a `meson test`); the naive round-half-up formatter fails it ~1 in 5M, the exact one passes 0/66M. tests/test_write_float_ format.py validates byte-identity end-to-end through the writer (default fast path == explicit %16.8f snprintf path) over 300k random + crafted-edge values. #39 golden/round-trip + #22 precision stay green; leaks 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Profiling the write path (the ask)
We'd optimized reads but never looked at writes. Benchmark (
benchmarks/bench_write.py, new), 200k-atom frame:extxyzcextxyzpluginnp.savetxt)¹ measured on a species+pos frame; extxyz-ng can't read the
forces+pbc-bool fixtures, so it'sn/ain the table for those.So we already beat ASE (2.4×) and extxyz-ng (1.5×) — but writing 200k atoms took ~266 ms vs ~33–50 ms to read. The per-atom loop did one
fprintfper value (~15 per atom incl. separators/newline), each paying format-string parsing + aflockfile/funlockfilepair.A C micro-benchmark (200k×6 floats) split the cost:
fprintf: 179 mssnprintf→ line buffer +fwrite: 157 ms (1.14×)i.e. the bottleneck is the
%.8fformatting itself, not I/O. The 2× lever needs a custom float formatter, which can't be byte-identical to printf (round-half-to-even — mismatches ~1 in 5M) without a fiddly correctly-rounded implementation. Given we already lead both competitors, we opted for the safe, byte-identical win only.This change — buffering (byte-identical)
Build each atom line in a growable memory buffer with
snprintf,fwriteit in blocks (flush at line boundaries). Same output bytes, far fewer locked stdio calls: ~1.26× on a 200k write (266 → 212 ms).-0.0, rounding edges, ints/bools/strings, multi-column, and a customformat_dict. Golden + round-trip + block-boundary tests lock it (tests/test_write_buffer.py).format_dictoverride (Precision specification when writing to extxyz #22) path unchanged;extxyz_write_llABI unchanged (Fortran/CI unaffected).Verification
pytest tests/(49, incl.test_write_precision.py) +python/ase-extxyz/tests/(38) green.leaks --atExit= 0 on the read→write C driver (the buffer is one alloc/free).benchmarks/bench_write.pyreproduces the comparison (times extxyz-ng ifEXTXYZ_NG_PYTHONis set).🤖 Generated with Claude Code