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

Optimize %T in tm formatting #2500

Merged
merged 2 commits into from
Sep 15, 2021
Merged

Conversation

phprus
Copy link
Contributor

@phprus phprus commented Sep 13, 2021

Optimization %T in tm formatting like this: 67cb2da

And fix wchar_t tm formatting error:

/.../fmt/include/fmt/chrono.h:567:15: error: no match for ‘operator==’ (operand types are ‘fmt::v8::basic_string_view<wchar_t>’ and ‘fmt::v8::string_view {aka fmt::v8::basic_string_view<char>}’)
     if (specs == string_view("%F", 2))

Maybe I should add the use of write_digit2_separated for formats:

  • %Y-%m-%d
  • %H:%M:%S
  • %Y-%m-%d %H:%M:%S
  • %Y-%m-%dT%H:%M:%S

?

@phprus phprus force-pushed the optimize-tm-formatting-1 branch from 6c6239d to a9113e5 Compare September 13, 2021 07:59
@@ -563,7 +564,13 @@ template <typename Char> struct formatter<std::tm, Char> {
while (end != ctx.end() && *end != '}') ++end;
auto size = detail::to_unsigned(end - it);
specs = {it, size};
if (specs == string_view("%F", 2)) spec_ = spec::year_month_day;
// basic_string_view<>::compare isn't constexpr before C++17
Copy link

Choose a reason for hiding this comment

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

There is no basic_string_view<> before C++17.

Could it be that the internal replacement should be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Problem in std::char_traits<>::compare:

fmt/include/fmt/core.h

Lines 472 to 478 in 3d0c7ae

FMT_CONSTEXPR_CHAR_TRAITS auto compare(basic_string_view other) const -> int {
size_t str_size = size_ < other.size_ ? size_ : other.size_;
int result = std::char_traits<Char>::compare(data_, other.data_, str_size);
if (result == 0)
result = size_ == other.size_ ? 0 : (size_ < other.size_ ? -1 : 1);
return result;
}

constexpr std::char_traits<>::compare starts from C++17 only

Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@@ -1217,6 +1230,7 @@ class weekday {
};

class year_month_day {};
class hh_mm_ss {};
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! Removed.

@phprus phprus force-pushed the optimize-tm-formatting-1 branch from a9113e5 to 7407f76 Compare September 15, 2021 14:25
@vitaut vitaut merged commit d9fd695 into fmtlib:master Sep 15, 2021
@vitaut
Copy link
Contributor

vitaut commented Sep 15, 2021

Maybe I should add the use of write_digit2_separated for formats

I'm not sure if we should add more special cases but it would be nice to get rid of strftime completely in formatter::format by parsing the format string and using the optimization when possible. It would be not as fast as special casing but would be more generally applicable.

phprus added a commit to phprus/fmt that referenced this pull request Aug 22, 2022
… шаблон литерального оператора должен иметь список параметров шаблона, эквивалентный "<char ...>"

  template <detail_exported::fixed_string Str> constexpr auto operator""_a() {
                                                              ^

Signed-off-by: Vladislav Shchapov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants