Skip to content

Speed up do_format_decimal #4630

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.

Sign up for GitHub

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

Merged
vitaut merged 4 commits into from
Dec 25, 2025
Merged

Speed up do_format_decimal #4630

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a016694
Speed up do_format_decimal
user202729 Dec 13, 2025
4435698
Fix compiler warning
user202729 Dec 13, 2025
79d8430
Fix lint
user202729 Dec 14, 2025
e4e2e22
Avoid failure if sizeof(unsigned long long) > 8
user202729 Dec 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 38 additions & 5 deletions include/fmt/format.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1045,6 +1045,22 @@ inline auto digits2(size_t value) -> const char* {
return &data[value * 2];
}

// Given i in [0, 100), let x be the first 7 digits after
// the decimal point of i / 100 in base 2, the first 2 bytes
// after digits2_i(x) is the string representation of i.
inline auto digits2_i(size_t value) -> const char* {
alignas(2) static const char data[] =
Copy link
Copy Markdown
Contributor

@Skylion007 Skylion007 Dec 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to make this array constexpr? I think this might enable certain compiler to do static compile time bounds checks if it's constexpr, especially if the whole function is constexpr.

Actually, I see why this done given the C++14 compatibiltiy requirements? Frustrating though as they could be merged easier across translation units otherwise in C++17 or newer.

Copy link
Copy Markdown
Contributor

@Skylion007 Skylion007 Dec 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably mark as noexcept though like the following for consistency: 3269c1c#diff-bdc6f79e8e9f5b4331d66fb785636a87d29f55cf729865e13925b4209424c878R1033

Copy link
Copy Markdown
Contributor Author

@user202729 user202729 Dec 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copy the style of digits2 function where the data array is not constexpr. I don't know what the compatibility requirement is exactly. If this array can be changed to constexpr then similar arrays should be as well.

Edit: hm, maybe the noexcept wasn't in digits2 when I looked.

"00010203 0405060707080910 1112"
"131414151617 18192021 222324 "
"25262728 2930313232333435 3637"
"383939404142 43444546 474849 "
"50515253 5455565757585960 6162"
"636464656667 68697071 727374 "
"75767778 7980818282838485 8687"
"888989909192 93949596 979899 ";
return &data[value * 2];
}

template <typename Char> constexpr auto getsign(sign s) -> Char {
return static_cast<char>(((' ' << 24) | ('+' << 16) | ('-' << 8)) >>
(static_cast<int>(s) * 8));
Expand Down Expand Up @@ -1213,6 +1229,16 @@ FMT_CONSTEXPR20 FMT_INLINE void write2digits(Char* out, size_t value) {
*out = static_cast<Char>('0' + value % 10);
}

template <typename Char>
FMT_INLINE void write2digits_i(Char* out, size_t value) {
if (std::is_same<Char, char>::value && !FMT_OPTIMIZE_SIZE) {
memcpy(out, digits2_i(value), 2);
return;
}
*out++ = static_cast<Char>(digits2_i(value)[0]);
*out = static_cast<Char>(digits2_i(value)[1]);
}

// Formats a decimal unsigned integer value writing to out pointing to a buffer
// of specified size. The caller must ensure that the buffer is large enough.
template <typename Char, typename UInt>
Expand All @@ -1221,12 +1247,19 @@ FMT_CONSTEXPR20 auto do_format_decimal(Char* out, UInt value, int size)
FMT_ASSERT(size >= count_digits(value), "invalid digit count");
unsigned n = to_unsigned(size);
while (value >= 100) {
// Integer division is slow so do it for a group of two digits instead
// of for every digit. The idea comes from the talk by Alexandrescu
// "Three Optimization Tips for C++". See speed-test for a comparison.
n -= 2;
write2digits(out + n, static_cast<unsigned>(value % 100));
value /= 100;
if (!is_constant_evaluated() && sizeof(UInt) == 4) {
auto p = value * static_cast<uint64_t>((1ull << 39) / 100 + 1);
write2digits_i(out + n, p >> (39 - 7) & ((1 << 7) - 1));
value = static_cast<UInt>(p >> 39) +
(static_cast<UInt>(value >= (100u << 25)) << 25);
} else {
// Integer division is slow so do it for a group of two digits instead
// of for every digit. The idea comes from the talk by Alexandrescu
// "Three Optimization Tips for C++". See speed-test for a comparison.
write2digits(out + n, static_cast<unsigned>(value % 100));
value /= 100;
}
}
if (value >= 10) {
n -= 2;
Expand Down