Skip to content

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
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
@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