[libc++][mingw] Remove setlocale from snprintf_l

[alvinhochun]

Summary

The previous code used __libcpp_locale_guard, which caused severe
performance issues with certain usage of streams because setlocale is
_that_ slow on Windows. This change speeds up number-to-string
conversion using ostringstream by a factor of 5x to 100x.

Fixes https://github.com/llvm/llvm-project/issues/56202

[alvinhochun]

This is an alternative of D59525 and D59727 for mingw. I think this implementation using _vsnprintf_s_l and _vscprintf_l should replicate the expected behaviour of snprintf_l, with NULL terminator and such. @mstorsjo should be able to confirm whether These two functions are available on both UCRT and MSVCRT configuration of mingw.

In case of insufficient buffer, this implementation has to process the format string twice, but doing this is still much faster than having to call setlocale multiple times.

I have not run any of the libcxx test suites on Windows and I don't have a build setup to do so.

I did test the same benchmark in https://github.com/llvm/llvm-project/issues/56202#issuecomment-1166229891, first built using the vanilla llvm-mingw-20220323-ucrt-x86_64 native toolchain and ran, then ran with a replacement libc++.dll built from llvmorg-14.0.0 with this patch applied. The performance difference is significant.

Without patch:

ns/op	op/s	err%	total	benchmark
--------------------:	--------------------:	--------:	----------:	:----------
89.32	11,195,659.27	0.8%	2.39	[ref] osstream with filler string
28,366.48	35,252.88	0.7%	2.42	osstream << int (not C locale)
1,045.82	956,183.67	0.7%	2.41	osstream << int (is C locale)
9.08	110,145,251.38	0.4%	2.42	std::to_string(int)
4.65	215,153,197.41	0.4%	2.40	std::to_chars(int)
28,814.25	34,705.05	1.0%	2.42	osstream << double (not C locale)
1,368.22	730,877.02	1.4%	2.29	osstream << double (is C locale)
348.63	2,868,353.50	1.8%	2.41	std::to_string(double)
32.64	30,638,029.72	0.4%	2.42	std::to_chars(double)

With patch:

ns/op	op/s	err%	total	benchmark
--------------------:	--------------------:	--------:	----------:	:----------
90.23	11,083,182.99	0.9%	2.39	[ref] osstream with filler string
222.78	4,488,729.77	1.0%	2.43	osstream << int (not C locale)
223.61	4,472,112.67	1.1%	2.42	osstream << int (is C locale)
8.82	113,315,748.74	1.6%	2.38	std::to_string(int)
4.76	209,994,184.81	2.0%	2.44	std::to_chars(int)
482.47	2,072,672.21	1.4%	2.41	osstream << double (not C locale)
477.90	2,092,487.71	0.7%	2.42	osstream << double (is C locale)
321.76	3,107,894.84	0.3%	2.42	std::to_string(double)
32.55	30,721,425.55	0.6%	2.46	std::to_chars(double)

[alvinhochun]

long double is an issue though...

[mstorsjo]

In D128600#3610680, @alvinhochun wrote:

long double is an issue though...

Yup, long double on x86 is a tricky thing. In the CI configuration, we build with -DLIBCXX_EXTRA_SITE_DEFINES="__USE_MINGW_ANSI_STDIO=1", which makes sure that both the library is built with this define, and the define is set whenever any user of the library includes libc++ headers (I think libstdc++ does the same, where including libstdc++ also enables the same for your code).

(This isn't the configuration I built it for llvm-mingw releases though, but maybe I should.)

So for this case, one (messy) possible way forward could be to use the modern locale APIs if __USE_MINGW_ANSI_STDIO isn't set (or is 0) which would help for the actual llvm-mingw releases for now, while using the slow approach when x86 long doubles are needed. (That codepath wouldn't be covered by CI unfortunately then, though.)

I guess the more complete/proper way forward would make a mingw-w64 specific implementation of these (or some similar) functions too, if __USE_MINGW_ANSI_STDIO is set to 1. (No idea offhand how hard that would be though...)

[alvinhochun]

In D128600#3611043, @mstorsjo wrote:

I guess the more complete/proper way forward would make a mingw-w64 specific implementation of these (or some similar) functions too, if __USE_MINGW_ANSI_STDIO is set to 1. (No idea offhand how hard that would be though...)

Hypothetically, if this means implementing custom formatting routines, wouldn't it be better to just use the same implementation for both msvc and mingw-w64?

(I may experiment with this for a bit, perhaps it can reuse some parts of the std::to_chars implementation...)

[mstorsjo]

In D128600#3611520, @alvinhochun wrote:

In D128600#3611043, @mstorsjo wrote:

I guess the more complete/proper way forward would make a mingw-w64 specific implementation of these (or some similar) functions too, if __USE_MINGW_ANSI_STDIO is set to 1. (No idea offhand how hard that would be though...)

Hypothetically, if this means implementing custom formatting routines, wouldn't it be better to just use the same implementation for both msvc and mingw-w64?

I didn’t mean implementing formatters from scratch (fwiw, libc++ does have that already for the modern std::format stuff too). Mingw-w64 already has got its own formatters that are used when __USE_MINGW_ANSI_STDIO is defined - I meant extending that with an entry point that takes a locale parameter. (Not sure if that’s straightforward or requires touching everything though.)

But on the other hand, now that libc++ does have its own formatters, I guess it could be useful to use that for these older formatting cases too, instead of relying on the libc?

[mstorsjo]

Libcxx wants to finish all outstanding reviews for the migration to github PRs. I guess we should abandon this one for now then, and then reopen as PR when we've got more time to spend effort on it? (It's annoying as this solution is 95% there for all practical cases, but handling the long double case is really annoying...)

[alvinhochun]

Yeah, I don't have time to work on this now, and I don't have a good idea how to solve this either.

libcxx/src/support/win32/locale_win32.cpp

int snprintf_l(char *ret, size_t n, locale_t loc, const char *format, ...)
     va_list ap;
     va_start( ap, format );
 #if defined(_LIBCPP_MSVCRT)
     // FIXME: Remove usage of internal CRT function and globals.
     int result = __stdio_common_vsprintf(
         _CRT_INTERNAL_LOCAL_PRINTF_OPTIONS | _CRT_INTERNAL_PRINTF_STANDARD_SNPRINTF_BEHAVIOR,
         ret, n, format, loc, ap);
 #else
-    __libcpp_locale_guard __current(loc);
-    _LIBCPP_DIAGNOSTIC_PUSH
-    _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wformat-nonliteral")
-    int result = vsnprintf( ret, n, format, ap );
-    _LIBCPP_DIAGNOSTIC_POP
+    int result;
+    if (n != 0) {
+      va_list ap_copy;
+      va_copy(ap_copy, ap);
+      result = _vsnprintf_s_l(ret, n, _TRUNCATE, format, loc, ap_copy);
+      va_end(ap_copy);
+    }
+    if (n == 0 || result < 0)
+      result = _vscprintf_l(format, loc, ap);
 #endif
     va_end(ap);
     return result;
 }
 
 int asprintf_l( char **ret, locale_t loc, const char *format, ... )
 {
     va_list ap;