The changes I have requested are minimal, so I will do them locally before pushing once the patches are approved. No need to post new revision.
On Tue, Jun 30, 2026 at 3:52 PM Tomasz Kaminski <[email protected]> wrote: > > > On Tue, Jun 30, 2026 at 3:26 PM Tomasz Kaminski <[email protected]> > wrote: > >> >> >> On Tue, Jun 30, 2026 at 3:10 PM Tomasz Kaminski <[email protected]> >> wrote: >> >>> >>> >>> On Tue, Jun 30, 2026 at 2:52 PM Anlai Lu <[email protected]> wrote: >>> >>>> This is v3 of the patch series. Performance data: see v2 cover letter. >>>> Results are unchanged -- same stack-buffer + __ostream_insert approach. >>>> >>>> Patch 1 is unchanged from v2. >>>> >>>> Changes in Patch 2 from v2: >>>> >>>> - Dropped the format string parameter from __chrono_write. >>>> _S_empty_fs() is used directly, per Tomasz Kaminski's suggestion. >>>> All chrono types now share the same two call forms: >>>> __detail::__chrono_write(__os, __arg); >>>> __detail::__chrono_write(__os, __arg, __os.getloc()); >>>> >>>> - Removed __detail::__empty_fmt: no longer needed. >>>> >>>> - All remaining operator<< that used basic_stringstream now use >>>> __chrono_write (month_day, month_day_last, month_weekday, >>>> month_weekday_last, year_month, year_month_day_last, >>>> year_month_weekday, year_month_weekday_last, sys_days, local_time). >>>> >>>> - The only types not converted: duration (must forward stream flags >>>> and precision) and local_info (uses __formatter_chrono_info). >>>> >>> I think for local_info the __chrono_write would also give a correct >>> result, >>> the formatter<local_info> uses __formatter_chrono_info and calls to >>> format_to >>> will pick it up. >>> >> However, now I wonder if it the output fits in 128B buffer, if it does I >> think the reduced >> duplication of the code would be beneficial (the _M_format_to overload >> for local_info) >> is esentially copy of operator<<. >> > However, with update to local_info we are no longer performing any > allocations in > operator<<, so this will be strictly be a regression in performance. So I > think you made > a right call, to not change it. It would be nice to put summary of that in > the commit message. > > >> Previously std::format("{}", t) format was implemented by delegating >> throu operator<<, >> and I have reimplemented all the formatters (outside duration) to be >> independed, >> some time ago. And then flipping the logic to use operator<< instead of >> formatter was >> on my TODO list. So thank you for picking this up. >> > >> >>> >>>> - Uses std::format_to_n (public API) instead of __do_vformat_to_n. >>>> >>>> Tested on x86_64-linux-gnu. libstdc++ testsuite (std/time/*) clean. >>>> All converted types verified byte-identical across C, en_US, de_DE, >>>> fr_FR, and zh_CN locales. >>>> >>> Do you see same kind of improvements as with your original patch? >>> >>>> >>>> Anlai Lu (2): >>>> libstdc++: Add stream state tests for chrono operator<< >>>> libstdc++: Use __chrono_write via _S_empty_fs for chrono ostream >>>> insertion >>>> >>>> libstdc++-v3/include/bits/chrono_io.h | 227 ++++-------------- >>>> .../testsuite/std/time/ostream_insert.cc | 163 +++++++++++++ >>>> 2 files changed, 212 insertions(+), 178 deletions(-) >>>> create mode 100644 libstdc++-v3/testsuite/std/time/ostream_insert.cc >>>> >>>> -- >>>> 2.34.1 >>>> >>>>
