Skip to content

Commit 6dd944a

Browse files
zaewcclaude
andcommitted
Keep the digit-separator feature off the default hot path
PR fastfloat#369's "performance-neutral" claim did not hold: the default (no-separator) parse was ~70% larger in the hot frame (parse_number_string inlined into from_chars: 833 -> 1414 x86/arm64 instructions), because 1. parse_number_string is force-inlined (always_inline), and the runtime dispatch inlined BOTH the has_separator==true and ==false instantiations into the default caller -- dragging the entire separator-aware scanner into the hot frame; and 2. the runtime separator branch survived in from_chars() even though a default-constructed parse_options provably has no separator, because the thin from_chars_caller forwarder was not inlined, so the compile-time '\0' was lost across the call boundary and the branch (plus two calls) could not be folded away. Fixes: * FASTFLOAT_NOINLINE + a cold parse_number_string_with_separator trampoline so the separator instantiation stays strictly out of line; the default caller only ever materializes the has_separator==false code. * Force-inline from_chars_caller::call (all three specializations) so the separator branch constant-folds away on the common from_chars() path. Net: the default hot frame returns to 844 instructions (vs 833 on main; the +11 is the unrelated parse_number_string refactor and is unchanged by this commit). The separator path is unaffected and all tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1d35d26 commit 6dd944a

2 files changed

Lines changed: 26 additions & 11 deletions

File tree

include/fast_float/float_common.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,12 @@ using parse_options = parse_options_t<char>;
217217
#define fastfloat_really_inline inline __attribute__((always_inline))
218218
#endif
219219

220+
#ifdef FASTFLOAT_VISUAL_STUDIO
221+
#define FASTFLOAT_NOINLINE __declspec(noinline)
222+
#else
223+
#define FASTFLOAT_NOINLINE __attribute__((noinline, cold))
224+
#endif
225+
220226
// Branch-probability hint marking the rare slow-path branches as cold, so the
221227
// optimizer keeps the out-of-line slow-path re-parse off the hot path (and does
222228
// not duplicate the force-inlined hot scanner into the caller, which bloated

include/fast_float/parse_number.h

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ fastfloat_really_inline bool rounds_to_nearest() noexcept {
139139

140140
template <typename T> struct from_chars_caller {
141141
template <typename UC>
142-
FASTFLOAT_CONSTEXPR20 static from_chars_result_t<UC>
142+
fastfloat_really_inline FASTFLOAT_CONSTEXPR20 static from_chars_result_t<UC>
143143
call(UC const *first, UC const *last, T &value,
144144
parse_options_t<UC> options) noexcept {
145145
return from_chars_advanced(first, last, value, options);
@@ -149,7 +149,7 @@ template <typename T> struct from_chars_caller {
149149
#ifdef __STDCPP_FLOAT32_T__
150150
template <> struct from_chars_caller<std::float32_t> {
151151
template <typename UC>
152-
FASTFLOAT_CONSTEXPR20 static from_chars_result_t<UC>
152+
fastfloat_really_inline FASTFLOAT_CONSTEXPR20 static from_chars_result_t<UC>
153153
call(UC const *first, UC const *last, std::float32_t &value,
154154
parse_options_t<UC> options) noexcept {
155155
// if std::float32_t is defined, and we are in C++23 mode; macro set for
@@ -166,7 +166,7 @@ template <> struct from_chars_caller<std::float32_t> {
166166
#ifdef __STDCPP_FLOAT64_T__
167167
template <> struct from_chars_caller<std::float64_t> {
168168
template <typename UC>
169-
FASTFLOAT_CONSTEXPR20 static from_chars_result_t<UC>
169+
fastfloat_really_inline FASTFLOAT_CONSTEXPR20 static from_chars_result_t<UC>
170170
call(UC const *first, UC const *last, std::float64_t &value,
171171
parse_options_t<UC> options) noexcept {
172172
// if std::float64_t is defined, and we are in C++23 mode; macro set for
@@ -289,23 +289,32 @@ from_chars_advanced(parsed_number_string_t<UC> &pns, T &value) noexcept {
289289
return answer;
290290
}
291291

292+
template <bool bjf, typename UC>
293+
FASTFLOAT_NOINLINE FASTFLOAT_CONSTEXPR20 parsed_number_string_t<UC>
294+
parse_number_string_with_separator(UC const *first, UC const *last,
295+
parse_options_t<UC> options,
296+
bool store_spans) noexcept {
297+
return parse_number_string<bjf, true, UC>(first, last, options, store_spans);
298+
}
299+
292300
// Runtime -> compile-time dispatch over both boolean knobs of
293301
// parse_number_string. basic_json_fmt was already dispatched this way; the
294302
// digit separator is selected here too so that the separator-aware code paths
295-
// stay confined to the (cold) has_separator==true instantiation. Callers that
296-
// never set a separator -- the overwhelming majority -- run the
303+
// stay confined to the (cold, out-of-line) has_separator==true instantiation.
304+
// Callers that never set a separator -- the overwhelming majority -- run the
297305
// has_separator==false instantiation, which is byte-for-byte the original
298-
// separator-free parser.
306+
// separator-free parser; the separator check is a single predictable branch
307+
// into cold code.
299308
template <typename UC>
300309
fastfloat_really_inline FASTFLOAT_CONSTEXPR20 parsed_number_string_t<UC>
301310
parse_number_string_options(UC const *first, UC const *last,
302311
parse_options_t<UC> options, bool bjf,
303312
bool store_spans) noexcept {
304-
if (options.digit_separator != UC('\0')) {
305-
return bjf ? parse_number_string<true, true, UC>(first, last, options,
306-
store_spans)
307-
: parse_number_string<false, true, UC>(first, last, options,
308-
store_spans);
313+
if fastfloat_unlikely(options.digit_separator != UC('\0')) {
314+
return bjf ? parse_number_string_with_separator<true, UC>(
315+
first, last, options, store_spans)
316+
: parse_number_string_with_separator<false, UC>(
317+
first, last, options, store_spans);
309318
}
310319
return bjf ? parse_number_string<true, false, UC>(first, last, options,
311320
store_spans)

0 commit comments

Comments
 (0)