Skip to content
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.

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

Optimizations for <format> #3826

Merged
merged 9 commits into from
Jul 14, 2023
Merged

Conversation

achabense
Copy link
Contributor

@achabense achabense commented Jun 23, 2023

  • (Cleanup) Remove two redundant parameters in a requires expression.
    They were meant for _Parse_align_callbacks(which is already checked).
  • Avoid usage of visit_format_arg in _Estimate_required_capacity.
    This change makes the method a lot more light-weight.
  • Avoid usage of toupper in _Buffer_to_uppercase and another place.
    I believe we don't need locale awareness in these places, and toupper cannot be inlined.
  • Avoid calling _Buffer_to_uppercase in _Write_integral when _Specs._Type is 'B'.
    In this case there are no letters in the buffer.

@achabense achabense requested a review from a team as a code owner June 23, 2023 13:54
@StephanTLavavej StephanTLavavej added performance Must go faster format C++20/23 format labels Jun 30, 2023
@achabense
Copy link
Contributor Author

benchmark:

Benchmark
#define _XKEYCHECK_H
#define private public
#include <format>
#undef private

#include <sstream>
#include "benchmark/benchmark.h"

auto _estimate_bef(const std::format_args& args) {
  return args._Estimate_required_capacity();
}

auto _estimate_now(const std::format_args& args) {
  using _CharType = char;
  size_t _Result = 0;

  for (size_t _Idx = 0; _Idx < args._Num_args; ++_Idx) {
    const auto _Packed_index = args._Index_array[_Idx];
    const auto _Arg_type = _Packed_index._Type();
    if (_Arg_type == std::_Basic_format_arg_type::_String_type) {
      const auto _Arg_storage = reinterpret_cast<const unsigned char*>(
                                    args._Index_array + args._Num_args) +
                                _Packed_index._Index;
      const auto _View =
          args._Get_value_from_memory<std::basic_string_view<_CharType>>(
              _Arg_storage);
      _Result += _View.size();
    } else if (_Arg_type == std::_Basic_format_arg_type::_CString_type) {
      _Result += 32;
    } else {
      _Result += 8;
    }
  };
  return _Result;
}

void BM_estimate_bef(benchmark::State& state) {
  std::string s("123456789");
  auto fmt_args = std::make_format_args(1, 2.0, "", s, 1.0f, 1u, '2', "1234");

  for (auto _ : state) {
    benchmark::DoNotOptimize(_estimate_bef(fmt_args));
  }
}

void BM_estimate_now(benchmark::State& state) {
  std::string s("123456789");
  auto fmt_args = std::make_format_args(1, 2.0, "", s, 1.0f, 1u, '2', "1234");

  for (auto _ : state) {
    benchmark::DoNotOptimize(_estimate_now(fmt_args));
  }
}

void _copy_toupper_lib(const char* source, char* dest, int size) {
  for (int i = 0; i < size; i++) {
    dest[i] = toupper(source[i]);
  }
}

void _copy_toupper_now(const char* source, char* dest, int size) {
  for (int i = 0; i < size; i++) {
    if (source[i] >= 'a' && source[i] <= 'z') {
      dest[i] = source[i] - ('a' - 'A');
    } else {
      dest[i] = source[i];
    }
  }
}

std::string source = []() {
  std::ostringstream is;
  for (int i = -30000; i < 30000; i++) {
    is << std::hex << i;
  }
  return is.str();
}();
std::string dest(source.size(), '0');

void BM_toupper_lib(benchmark::State& state) {
  for (auto _ : state) {
    _copy_toupper_lib(source.data(), dest.data(), source.size());
  }
}

void BM_toupper_now(benchmark::State& state) {
  for (auto _ : state) {
    _copy_toupper_now(source.data(), dest.data(), source.size());
  }
}

BENCHMARK(BM_estimate_bef);
BENCHMARK(BM_estimate_now);
BENCHMARK(BM_toupper_lib);
BENCHMARK(BM_toupper_now);

BENCHMARK_MAIN();
Result

image

@StephanTLavavej StephanTLavavej self-assigned this Jul 4, 2023
@StephanTLavavej
Copy link
Member

Looks perfect, thank you! 😻 🚀

@StephanTLavavej StephanTLavavej removed their assignment Jul 7, 2023
@StephanTLavavej StephanTLavavej self-assigned this Jul 13, 2023
@StephanTLavavej
Copy link
Member

I'm speculatively mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 5c91cbb into microsoft:main Jul 14, 2023
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks for identifying and improving these things in <format>! 🔍 ⚙️ 😻

@achabense achabense deleted the _For_fmt branch July 14, 2023 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format C++20/23 format performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants