Optimize tm formatting #2602

phprus · 2021-11-14T16:40:31Z

Optimize tm formatting for:

C-locale and alternative representation.
%z format (timezone offset from UTC).

vitaut · 2021-11-17T14:48:36Z

Thanks for the PR. Could you provide any benchmark results showing the effects of this change?

vitaut · 2021-11-17T14:50:38Z

include/fmt/chrono.h

-  void on_utc_offset() { format_localized('z'); }
+  void on_utc_offset() {
+#if defined(_WIN32)
+    _tzset();


We shouldn't call _tzset or tzset because it changes global state.

_tzset / tzset is used internally in std::time_put.

Without calling them, it is impossible to get the timezone correctly (if the timezone is not stored in struct std::tm).

Windows: https://docs.microsoft.com/en-us/cpp/c-runtime-library/daylight-dstbias-timezone-and-tzname?view=msvc-140 (Remarks section)
Solaris: https://docs.oracle.com/cd/E86824_01/html/E54766/ctime-3c.html

On Solaris and Windows tzset is threadsafe.

@vitaut, What do you think about this?

I think we should limit system-specific logic here and suggest just detecting if tm_gmtoff is available and otherwise go through format_localized as before. The presence of tm_gmtoff can be detected in a portable way using SFINAE.

phprus · 2021-11-17T15:08:29Z

Benchmark: https://github.com/phprus/fmt-bench/tree/optimize-tm-formatting-5

*BSD, Apple, glibc: Acceleration 10-20 times (%z).
This is std::put_time and ostrstream overhead.

phprus · 2021-11-17T16:05:50Z

cc @alexezeder

phprus · 2021-11-17T16:10:57Z

Other formatters (%EY, ...) on all OS with C-locale: 10-20 times

alexezeder · 2021-11-17T18:53:07Z

cc @alexezeder

I'm not 100% sure why I was mentioned here, but here are the results on my machine. 😏

Currently, there is no option to run benchmarks for PRs in fmt_bnchmrk_gnrtr (I doubt it would be implemented someday). But the only one runner there uses the same version of GCC - 11.

GCC 11 + libstdc++ (C++20)

Benchmark	old time	new time
FMTFormatter_z	261 ns	26.3 ns
FMTFormatterCompile_z	246 ns	11.6 ns
FMTFormatter_Y	263 ns	26.1 ns
FMTFormatterCompile_Y	244 ns	10.5 ns

Raw output

cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=20 ..

$ ./fmt_test_old
----------------------------------------------------------------
Benchmark                      Time             CPU   Iterations
----------------------------------------------------------------
FMTFormatter_z               261 ns          261 ns      2692446
FMTFormatterCompile_z        246 ns          246 ns      2851517
FMTFormatter_Y               263 ns          263 ns      2662867
FMTFormatterCompile_Y        244 ns          244 ns      2867342

$ ./fmt_test_new
----------------------------------------------------------------
Benchmark                      Time             CPU   Iterations
----------------------------------------------------------------
FMTFormatter_z              26.3 ns         26.3 ns     26136296
FMTFormatterCompile_z       11.6 ns         11.6 ns     61292230
FMTFormatter_Y              26.1 ns         26.1 ns     26920284
FMTFormatterCompile_Y       10.5 ns         10.5 ns     67130168

Clang 13 + libc++ (C++20)

Benchmark	old time	new time
FMTFormatter_z	110 ns	30.6 ns
FMTFormatterCompile_z	90.0 ns	10.4 ns
FMTFormatter_Y	110 ns	30.5 ns
FMTFormatterCompile_Y	88.4 ns	12.7 ns

Raw output

CC='clang' CXX='clang++' CXXFLAGS='-stdlib=libc++' cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_STANDARD=20 ..

$ ./fmt_test_old
----------------------------------------------------------------
Benchmark                      Time             CPU   Iterations
----------------------------------------------------------------
FMTFormatter_z               110 ns          110 ns      6403692
FMTFormatterCompile_z       90.0 ns         90.0 ns      7797814
FMTFormatter_Y               110 ns          110 ns      6353394
FMTFormatterCompile_Y       88.4 ns         88.4 ns      7909662

$ ./fmt_test_new
----------------------------------------------------------------
Benchmark                      Time             CPU   Iterations
----------------------------------------------------------------
FMTFormatter_z              30.6 ns         30.6 ns     23115238
FMTFormatterCompile_z       10.4 ns         10.4 ns     67346905
FMTFormatter_Y              30.5 ns         30.4 ns     22898049
FMTFormatterCompile_Y       12.7 ns         12.7 ns     55214092

phprus · 2021-11-17T19:08:28Z

@alexezeder, I mentioned you since you wrote a comment about performance regression (1031eed#commitcomment-60128058)

This PR improves performance in those cases (C-locale and timezone offset).

vitaut · 2021-11-21T15:04:23Z

include/fmt/chrono.h

-  void on_utc_offset() { format_localized('z'); }
+  void on_utc_offset() {
+#if defined(_WIN32)
+    _tzset();


I think we should limit system-specific logic here and suggest just detecting if tm_gmtoff is available and otherwise go through format_localized as before. The presence of tm_gmtoff can be detected in a portable way using SFINAE.

vitaut · 2021-11-21T15:06:49Z

include/fmt/chrono.h

+    if (!is_classic_ && ns != numeric_system::standard)
+      return format_localized('Y', 'E');
    write_year(tm_year());


Let's swap the branches to get rid of negation:

if (is_classic_ || ns == numeric_system::standard) return write_year(tm_year()); format_localized('Y', 'E');

and the same throughout the PR.

tm_gmtoff and SFINAE - Done.
Solaris specific code is removed.
Windows specific code I suggest to include in fmt, because Windows is more popular.

I was wrong.
Inside ucrt, the internal function __tzset () is used, which is called only once. When using _tzset (), performance is 10 times slower.

I've added a workaround, but I'm not sure if I need to switch to custom code.

Perf with workaround:

2021-11-22T00:37:43+05:00 Running fmt_test_old Run on (2 X 2494 MHz CPU s) CPU Caches: L1 Data 32 KiB (x2) L1 Instruction 32 KiB (x2) L2 Unified 4096 KiB (x2) L3 Unified 16384 KiB (x1) ------------------------------------------------------------------ Benchmark Time CPU Iterations ------------------------------------------------------------------ FMTFormatter_z 1384 ns 1360 ns 448000 FMTFormatterCompile_z 1321 ns 1318 ns 497778 FMTFormatter_z_strftime 312 ns 314 ns 2240000 FMTFormatter_z_time_put 1217 ns 1221 ns 640000 FMTFormatter_Y 1385 ns 1381 ns 497778 FMTFormatterCompile_Y 1300 ns 1311 ns 560000 FMTFormatter_Y_strftime 301 ns 300 ns 2240000 FMTFormatter_Y_time_put 1214 ns 1221 ns 640000 2021-11-22T00:37:50+05:00 Running fmt_test_new Run on (2 X 2494 MHz CPU s) CPU Caches: L1 Data 32 KiB (x2) L1 Instruction 32 KiB (x2) L2 Unified 4096 KiB (x2) L3 Unified 16384 KiB (x1) ------------------------------------------------------------------ Benchmark Time CPU Iterations ------------------------------------------------------------------ FMTFormatter_z 123 ns 123 ns 5600000 FMTFormatterCompile_z 71.1 ns 69.8 ns 8960000 FMTFormatter_z_strftime 312 ns 314 ns 2240000 FMTFormatter_z_time_put 1214 ns 1221 ns 640000 FMTFormatter_Y 121 ns 120 ns 5600000 FMTFormatterCompile_Y 67.9 ns 68.4 ns 11200000 FMTFormatter_Y_strftime 303 ns 305 ns 2357895 FMTFormatter_Y_time_put 1203 ns 1200 ns 560000

vitaut · 2021-11-21T15:07:46Z

test/chrono-test.cc

@@ -49,7 +49,14 @@ std::string system_strftime(const std::string& format, const std::tm* timeptr,
  os.imbue(loc);
  facet.put(os, os, ' ', timeptr, format.c_str(),
            format.c_str() + format.size());
+#ifdef _WIN32
+  // Workaround to early ucrt bug


Early version.

I have not tested all versions. But in the latest version of the Windows SDK I have installed with MSVS, '%z' returns '+0000'. In the old version: '-0000'.

If remove platform-dependent code, this change will not be necessary.

Since we are keeping this, let's tweak it a bit for clarity:

// Workaround a bug in older versions of ucrt.

BTW is "ucrt" the correct term here? Is it Microsoft C runtime library? Maybe calling it Microsoft CRT here would be clearer?

vitaut · 2021-11-21T15:20:41Z

@phprus, thanks for the benchmark. Could you add strftime there for comparison?

phprus · 2021-11-21T17:50:32Z

@vitaut, I will do a performance test (on Windows and with raw strftime, std::time_put) in the coming days.

phprus · 2021-11-21T19:54:41Z

@vitaut, benchmark updated: https://github.com/phprus/fmt-bench/tree/optimize-tm-formatting-5

include/fmt/chrono.h

vitaut · 2021-11-24T00:07:51Z

test/chrono-test.cc

@@ -49,7 +49,14 @@ std::string system_strftime(const std::string& format, const std::tm* timeptr,
  os.imbue(loc);
  facet.put(os, os, ' ', timeptr, format.c_str(),
            format.c_str() + format.size());
+#ifdef _WIN32
+  // Workaround to early ucrt bug


Since we are keeping this, let's tweak it a bit for clarity:

// Workaround a bug in older versions of ucrt.

BTW is "ucrt" the correct term here? Is it Microsoft C runtime library? Maybe calling it Microsoft CRT here would be clearer?

vitaut · 2021-11-24T00:57:10Z

benchmark updated

Awesome, thanks!

phprus · 2021-11-25T14:17:04Z

Sorry for delay.

UCRT is are official name for Microsoft C Runtime start from Visual Studio 2015 (https://docs.microsoft.com/en-us/cpp/porting/upgrade-your-code-to-the-universal-crt?view=msvc-170)

I replaced the abbreviation UCRT to another version of the name: Universal CRT.

vitaut · 2021-11-25T14:52:59Z

Merged, thank you for another great PR!

vitaut reviewed Nov 17, 2021

View reviewed changes

vitaut requested changes Nov 21, 2021

View reviewed changes

Use predefined formats for C-locale

19e3e2f

phprus force-pushed the optimize-tm-formatting-5 branch from 76da43b to 2597fc2 Compare November 21, 2021 17:43

phprus force-pushed the optimize-tm-formatting-5 branch from 2597fc2 to 0b7c3db Compare November 21, 2021 19:44

vitaut requested changes Nov 24, 2021

View reviewed changes

Add platform-specific 'z' formatter

d7b90d7

phprus force-pushed the optimize-tm-formatting-5 branch from 0b7c3db to d7b90d7 Compare November 25, 2021 14:24

vitaut approved these changes Nov 25, 2021

View reviewed changes

vitaut merged commit 5abe9e8 into fmtlib:master Nov 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize tm formatting #2602

Optimize tm formatting #2602

phprus commented Nov 14, 2021

vitaut commented Nov 17, 2021

vitaut Nov 17, 2021

phprus Nov 17, 2021

phprus Nov 20, 2021

vitaut Nov 21, 2021

phprus commented Nov 17, 2021

phprus commented Nov 17, 2021

phprus commented Nov 17, 2021

alexezeder commented Nov 17, 2021

phprus commented Nov 17, 2021

vitaut Nov 21, 2021

vitaut Nov 21, 2021 •

edited

Loading

phprus Nov 21, 2021

phprus Nov 21, 2021

phprus Nov 21, 2021

vitaut Nov 21, 2021

phprus Nov 21, 2021

vitaut Nov 24, 2021

vitaut commented Nov 21, 2021

phprus commented Nov 21, 2021

phprus commented Nov 21, 2021

vitaut Nov 24, 2021

vitaut commented Nov 24, 2021

phprus commented Nov 25, 2021

vitaut commented Nov 25, 2021

Optimize tm formatting #2602

Optimize tm formatting #2602

Conversation

phprus commented Nov 14, 2021

vitaut commented Nov 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phprus commented Nov 17, 2021

phprus commented Nov 17, 2021

phprus commented Nov 17, 2021

alexezeder commented Nov 17, 2021

GCC 11 + libstdc++ (C++20)

Clang 13 + libc++ (C++20)

phprus commented Nov 17, 2021

Choose a reason for hiding this comment

vitaut Nov 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitaut commented Nov 21, 2021

phprus commented Nov 21, 2021

phprus commented Nov 21, 2021

Choose a reason for hiding this comment

vitaut commented Nov 24, 2021

phprus commented Nov 25, 2021

vitaut commented Nov 25, 2021

vitaut Nov 21, 2021 •

edited

Loading