Skip to content

Commit 6589729

Browse files
mordantemordante
committed
[libc++][format] Improves parsing speed.
A format string like "{}" is quite common. In this case avoid parsing the format-spec when it's not present. Before the parsing was always called, therefore some refactoring is done to make sure the formatters work properly when their parse member isn't called. From the wording it's not entirely clear whether this optimization is allowed [tab:formatter] ``` and the range [pc.begin(), pc.end()) from the last call to f.parse(pc). ``` Implies there's always a call to `f.parse` even when the format-spec isn't present. Therefore this optimization isn't done for handle classes; it's unclear whether that would break user defined formatters. The improvements give a small reduciton is code size: 719408 12472 488 732368 b2cd0 before 718824 12472 488 731784 b2a88 after The performance benefits when not using a format-spec are: ``` Comparing ./formatter_int.libcxx.out-baseline to ./formatter_int.libcxx.out Benchmark Time CPU Time Old Time New CPU Old CPU New ---------------------------------------------------------------------------------------------------------------------------------------------------- BM_Basic<uint32_t> -0.0688 -0.0687 67 62 67 62 BM_Basic<int32_t> -0.1105 -0.1107 73 65 73 65 BM_Basic<uint64_t> -0.1053 -0.1049 95 85 95 85 BM_Basic<int64_t> -0.0889 -0.0888 93 85 93 85 BM_BasicLow<__uint128_t> -0.0655 -0.0655 96 90 96 90 BM_BasicLow<__int128_t> -0.0693 -0.0694 97 90 97 90 BM_Basic<__uint128_t> -0.0359 -0.0359 256 247 256 247 BM_Basic<__int128_t> -0.0414 -0.0414 239 229 239 229 ``` For the cases where a format-spec is used the results remain similar, some are faster some are slower, differing per run. Reviewed By: ldionne, #libc Differential Revision: https://reviews.llvm.org/D129426
1 parent fd67992 commit 6589729

File tree

6 files changed

+8
-30
lines changed

6 files changed

+8
-30
lines changed

‎libcxx/include/__format/formatter_bool.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT formatter<bool, _CharT>
4747

4848
_LIBCPP_HIDE_FROM_ABI auto format(bool __value, auto& __ctx) const -> decltype(__ctx.out()) {
4949
switch (__parser_.__type_) {
50+
case __format_spec::__type::__default:
5051
case __format_spec::__type::__string:
5152
return __formatter::__format_bool(__value, __ctx, __parser_.__get_parsed_std_specifications(__ctx));
5253

‎libcxx/include/__format/formatter_char.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT __formatter_char {
4141
}
4242

4343
_LIBCPP_HIDE_FROM_ABI auto format(_CharT __value, auto& __ctx) const -> decltype(__ctx.out()) {
44-
if (__parser_.__type_ == __format_spec::__type::__char)
44+
if (__parser_.__type_ == __format_spec::__type::__default || __parser_.__type_ == __format_spec::__type::__char)
4545
return __formatter::__format_char(__value, __ctx.out(), __parser_.__get_parsed_std_specifications(__ctx));
4646

4747
if constexpr (sizeof(_CharT) <= sizeof(int))

‎libcxx/include/__format/formatter_integral.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,6 @@ _LIBCPP_HIDE_FROM_ABI auto __format_integer(
207207
char* __end,
208208
const char* __prefix,
209209
int __base) -> decltype(__ctx.out()) {
210-
_LIBCPP_ASSERT(
211-
__specs.__alignment_ != __format_spec::__alignment::__default,
212-
"the caller should adjust the default to the value required by the type");
213-
214210
char* __first = __formatter::__insert_sign(__begin, __negative, __specs.__std_.__sign_);
215211
if (__specs.__std_.__alternate_form_ && __prefix)
216212
while (*__prefix)
@@ -280,6 +276,7 @@ _LIBCPP_HIDE_FROM_ABI auto __format_integer(
280276
return __formatter::__format_integer(
281277
__value, __ctx, __specs, __negative, __array.begin(), __array.end(), __value != 0 ? "0" : nullptr, 8);
282278
}
279+
case __format_spec::__type::__default:
283280
case __format_spec::__type::__decimal: {
284281
array<char, __formatter::__buffer_size<decltype(__value), 10>()> __array;
285282
return __formatter::__format_integer(

‎libcxx/include/__format/formatter_output.h

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,11 @@ struct _LIBCPP_TYPE_VIS __padding_size_result {
5959
_LIBCPP_HIDE_FROM_ABI constexpr __padding_size_result
6060
__padding_size(size_t __size, size_t __width, __format_spec::__alignment __align) {
6161
_LIBCPP_ASSERT(__width > __size, "don't call this function when no padding is required");
62-
_LIBCPP_ASSERT(__align != __format_spec::__alignment::__default,
63-
"the caller should adjust the default to the value required by the type");
6462
_LIBCPP_ASSERT(__align != __format_spec::__alignment::__zero_padding,
6563
"the caller should have handled the zero-padding");
6664

6765
size_t __fill = __width - __size;
6866
switch (__align) {
69-
case __format_spec::__alignment::__default:
7067
case __format_spec::__alignment::__zero_padding:
7168
__libcpp_unreachable();
7269

@@ -81,6 +78,7 @@ __padding_size(size_t __size, size_t __width, __format_spec::__alignment __align
8178
size_t __after = __fill - __before;
8279
return {__before, __after};
8380
}
81+
case __format_spec::__alignment::__default:
8482
case __format_spec::__alignment::__right:
8583
return {__fill, 0};
8684
}
@@ -91,9 +89,6 @@ template <class _OutIt, class _CharT>
9189
_LIBCPP_HIDE_FROM_ABI _OutIt __write_using_decimal_separators(_OutIt __out_it, const char* __begin, const char* __first,
9290
const char* __last, string&& __grouping, _CharT __sep,
9391
__format_spec::__parsed_specifications<_CharT> __specs) {
94-
_LIBCPP_ASSERT(__specs.__alignment_ != __format_spec::__alignment::__default,
95-
"the caller should adjust the default to the value required by the type");
96-
9792
int __size = (__first - __begin) + // [sign][prefix]
9893
(__last - __first) + // data
9994
(__grouping.size() - 1); // number of separator characters

‎libcxx/include/__format/parser_std_format_spec.h

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,18 +1040,10 @@ _LIBCPP_HIDE_FROM_ABI constexpr void __process_display_type_char(__parser<_CharT
10401040
__format_spec::__process_display_type_bool_string(__parser);
10411041
}
10421042

1043-
template <class _CharT>
1044-
_LIBCPP_HIDE_FROM_ABI constexpr void __process_display_type_integer(__parser<_CharT>& __parser) {
1045-
if (__parser.__alignment_ == __alignment::__default)
1046-
__parser.__alignment_ = __alignment::__right;
1047-
}
1048-
10491043
template <class _CharT>
10501044
_LIBCPP_HIDE_FROM_ABI constexpr void __process_parsed_bool(__parser<_CharT>& __parser) {
10511045
switch (__parser.__type_) {
10521046
case __format_spec::__type::__default:
1053-
__parser.__type_ = __format_spec::__type::__string;
1054-
[[fallthrough]];
10551047
case __format_spec::__type::__string:
10561048
__format_spec::__process_display_type_bool_string(__parser);
10571049
break;
@@ -1062,7 +1054,6 @@ _LIBCPP_HIDE_FROM_ABI constexpr void __process_parsed_bool(__parser<_CharT>& __p
10621054
case __format_spec::__type::__decimal:
10631055
case __format_spec::__type::__hexadecimal_lower_case:
10641056
case __format_spec::__type::__hexadecimal_upper_case:
1065-
__process_display_type_integer(__parser);
10661057
break;
10671058

10681059
default:
@@ -1074,8 +1065,6 @@ template <class _CharT>
10741065
_LIBCPP_HIDE_FROM_ABI constexpr void __process_parsed_char(__parser<_CharT>& __parser) {
10751066
switch (__parser.__type_) {
10761067
case __format_spec::__type::__default:
1077-
__parser.__type_ = __format_spec::__type::__char;
1078-
[[fallthrough]];
10791068
case __format_spec::__type::__char:
10801069
__format_spec::__process_display_type_char(__parser);
10811070
break;
@@ -1086,7 +1075,6 @@ _LIBCPP_HIDE_FROM_ABI constexpr void __process_parsed_char(__parser<_CharT>& __p
10861075
case __format_spec::__type::__decimal:
10871076
case __format_spec::__type::__hexadecimal_lower_case:
10881077
case __format_spec::__type::__hexadecimal_upper_case:
1089-
__format_spec::__process_display_type_integer(__parser);
10901078
break;
10911079

10921080
default:
@@ -1098,15 +1086,12 @@ template <class _CharT>
10981086
_LIBCPP_HIDE_FROM_ABI constexpr void __process_parsed_integer(__parser<_CharT>& __parser) {
10991087
switch (__parser.__type_) {
11001088
case __format_spec::__type::__default:
1101-
__parser.__type_ = __format_spec::__type::__decimal;
1102-
[[fallthrough]];
11031089
case __format_spec::__type::__binary_lower_case:
11041090
case __format_spec::__type::__binary_upper_case:
11051091
case __format_spec::__type::__octal:
11061092
case __format_spec::__type::__decimal:
11071093
case __format_spec::__type::__hexadecimal_lower_case:
11081094
case __format_spec::__type::__hexadecimal_upper_case:
1109-
__format_spec::__process_display_type_integer(__parser);
11101095
break;
11111096

11121097
case __format_spec::__type::__char:
@@ -1120,8 +1105,6 @@ _LIBCPP_HIDE_FROM_ABI constexpr void __process_parsed_integer(__parser<_CharT>&
11201105

11211106
template <class _CharT>
11221107
_LIBCPP_HIDE_FROM_ABI constexpr void __process_parsed_floating_point(__parser<_CharT>& __parser) {
1123-
__format_spec::__process_display_type_integer(__parser);
1124-
11251108
switch (__parser.__type_) {
11261109
case __format_spec::__type::__default:
11271110
// When no precision specified then it keeps default since that

‎libcxx/include/format

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,7 @@ __handle_replacement_field(const _CharT* __begin, const _CharT* __end,
376376
__format::__parse_number_result __r =
377377
__format::__parse_arg_id(__begin, __end, __parse_ctx);
378378

379+
bool __parse = *__r.__ptr == _CharT(':');
379380
switch (*__r.__ptr) {
380381
case _CharT(':'):
381382
// The arg-id has a format-specifier, advance the input to the format-spec.
@@ -395,7 +396,7 @@ __handle_replacement_field(const _CharT* __begin, const _CharT* __end,
395396
if (__type == __arg_t::__handle)
396397
__ctx.__handle(__r.__value).__parse(__parse_ctx);
397398
else
398-
__format::__compile_time_visit_format_arg(__parse_ctx, __ctx, __type);
399+
__format::__compile_time_visit_format_arg(__parse_ctx, __ctx, __type);
399400
} else
400401
_VSTD::visit_format_arg(
401402
[&](auto __arg) {
@@ -405,7 +406,8 @@ __handle_replacement_field(const _CharT* __begin, const _CharT* __end,
405406
__arg.format(__parse_ctx, __ctx);
406407
else {
407408
formatter<decltype(__arg), _CharT> __formatter;
408-
__parse_ctx.advance_to(__formatter.parse(__parse_ctx));
409+
if (__parse)
410+
__parse_ctx.advance_to(__formatter.parse(__parse_ctx));
409411
__ctx.advance_to(__formatter.format(__arg, __ctx));
410412
}
411413
},

0 commit comments

Comments
 (0)