Skip to content

Commit

Permalink
Replacing strftime with std::time_put (#2550)
Browse files Browse the repository at this point in the history 
* Fix unicode test

* Add xchar support to chrono formatter

* Replace strftime with std::time_put

* Add std::locale support to std::tm formatter

* Use predefined names and formats for C-locale

* Performance improvement

* Make locale-independent and C locale formats consistent among platforms
  • Loading branch information
@phprus
phprus authored Oct 30, 2021
1 parent 90034e4 commit 1031eed
Show file tree
Hide file tree
Showing 4 changed files with 247 additions and 141 deletions.
Loading

2 comments on commit 1031eed

@alexezeder
Copy link
Contributor

@alexezeder alexezeder commented on 1031eed Nov 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Looks like this commit introduced performance regressions for some chrono types, like timezone and almost everything locale-specific.
@phprus, maybe this information can be handy to you, idk. Perhaps you are aware of this, I just didn't pay much attention to the chrono formatting.

@phprus
Copy link
Contributor Author

@phprus phprus commented on 1031eed Nov 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yes you are right. This is due to multiple std::time_put calls.

The use of a single call to time_put in such cases (locales) was discussed (#2541 (comment)) but rejected (#2541 (comment)).

It is possible in libstdc++ to use non-standard features that will reduce the overhead, but this was rejected (#2577).

I'll do a PR using libstdc++ non-standard features to test the performance. I can also try reusing the ostringstream object. But I'm not sure if this will help.

UPDATE 1:

Timezones are a separate big problem. There is no cross-platform API for getting this information in C/C++. I am currently researching this issue.

There is also a hypothesis that the performance regression was due to fair use of locales. The old version did not use locales.

Please sign in to comment.