From ba3e8f3bec4e00de82d901553e6a48fd84adc569 Mon Sep 17 00:00:00 2001 From: cpplearner Date: Sat, 11 Feb 2023 04:23:34 +0800 Subject: [PATCH 1/4] Optimize to_sys and to_local --- stl/inc/__msvc_tzdb.hpp | 10 ++- stl/inc/chrono | 136 ++++++++++++++++++---------------- stl/src/msvcp_atomic_wait.src | 1 + stl/src/tzdb.cpp | 17 ++++- 4 files changed, 95 insertions(+), 69 deletions(-) diff --git a/stl/inc/__msvc_tzdb.hpp b/stl/inc/__msvc_tzdb.hpp index 3948efa3e72..ff4267632b0 100644 --- a/stl/inc/__msvc_tzdb.hpp +++ b/stl/inc/__msvc_tzdb.hpp @@ -66,14 +66,20 @@ struct __std_tzdb_sys_info { const char* _Abbrev; }; +enum class __std_tzdb_sys_info_type { + _Full, + _Offset_only, + _Offset_and_range, +}; + _NODISCARD __std_tzdb_time_zones_info* __stdcall __std_tzdb_get_time_zones() noexcept; void __stdcall __std_tzdb_delete_time_zones(__std_tzdb_time_zones_info* _Info) noexcept; _NODISCARD __std_tzdb_current_zone_info* __stdcall __std_tzdb_get_current_zone() noexcept; void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* _Info) noexcept; -_NODISCARD __std_tzdb_sys_info* __stdcall __std_tzdb_get_sys_info( - const char* _Tz, size_t _Tz_len, __std_tzdb_epoch_milli _Local) noexcept; +_NODISCARD __std_tzdb_sys_info* __stdcall __std_tzdb_get_sys_info_v2( + const char* _Tz, size_t _Tz_len, __std_tzdb_epoch_milli _Local, __std_tzdb_sys_info_type _Type) noexcept; void __stdcall __std_tzdb_delete_sys_info(__std_tzdb_sys_info* _Info) noexcept; _NODISCARD __std_tzdb_leap_info* __stdcall __std_tzdb_get_leap_seconds( diff --git a/stl/inc/chrono b/stl/inc/chrono index e6bfa99bb77..91b912a8927 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -1765,70 +1765,12 @@ namespace chrono { template _NODISCARD sys_info get_info(const sys_time<_Duration>& _Sys) const { - return _Get_info(_Sys.time_since_epoch()); + return _Get_info(_Sys.time_since_epoch(), __std_tzdb_sys_info_type::_Full); } template _NODISCARD local_info get_info(const local_time<_Duration>& _Local) const { - local_info _Info{}; - const auto _Time_since_ep = _Local.time_since_epoch(); - _Info.first = _Get_info(_Time_since_ep); - - const sys_seconds _Local_sys{_CHRONO duration_cast(_Time_since_ep)}; - const auto _Curr_sys = _Local_sys - _Info.first.offset; - if (_Info.first.begin != _Min_seconds && _Curr_sys < _Info.first.begin + days{1}) { - // get previous transition information - _Info.second = get_info(_Info.first.begin - seconds{1}); - - const auto _Transition = _Info.first.begin; - const auto _Prev_sys = _Local_sys - _Info.second.offset; - if (_Curr_sys >= _Transition) { - if (_Prev_sys < _Transition) { - _Info.result = local_info::ambiguous; - _STD swap(_Info.first, _Info.second); - } else { - _Info.result = local_info::unique; - _Info.second = {}; - } - } else { - if (_Prev_sys >= _Transition) { - _Info.result = local_info::nonexistent; - _STD swap(_Info.first, _Info.second); - } else { - _Info.result = local_info::unique; - _Info.first = _STD move(_Info.second); - _Info.second = {}; - } - } - } else if (_Info.first.end != _Max_seconds && _Curr_sys > _Info.first.end - days{1}) { - // get next transition information - _Info.second = get_info(_Info.first.end + seconds{1}); - - const auto _Transition = _Info.first.end; - const auto _Next_sys = _Local_sys - _Info.second.offset; - if (_Curr_sys < _Transition) { - if (_Next_sys >= _Transition) { - _Info.result = local_info::ambiguous; - } else { - _Info.result = local_info::unique; - _Info.second = {}; - } - } else { - if (_Next_sys < _Transition) { - _Info.result = local_info::nonexistent; - } else { - _Info.result = local_info::unique; - _Info.first = _STD move(_Info.second); - _Info.second = {}; - } - } - } else { - // local time is contained inside of first transition boundaries by at least 1 day - _Info.result = local_info::unique; - _Info.second = {}; - } - - return _Info; + return _Get_local_info(_Local, __std_tzdb_sys_info_type::_Full); } template @@ -1846,7 +1788,7 @@ namespace chrono { template _NODISCARD sys_time> to_sys( const local_time<_Duration>& _Local, const choose _Choose) const { - const auto _Info = get_info(_Local); + const auto _Info = _Get_local_info(_Local, __std_tzdb_sys_info_type::_Offset_and_range); if (_Info.result == local_info::nonexistent) { return _Info.first.end; } @@ -1859,7 +1801,7 @@ namespace chrono { template _NODISCARD local_time> to_local(const sys_time<_Duration>& _Sys) const { - const auto _Info = get_info(_Sys); + const auto _Info = _Get_info(_Sys.time_since_epoch(), __std_tzdb_sys_info_type::_Offset_only); return local_time>{_Sys.time_since_epoch() + _Info.offset}; } @@ -1868,11 +1810,11 @@ namespace chrono { private: template - _NODISCARD sys_info _Get_info(const _Duration& _Dur) const { + _NODISCARD sys_info _Get_info(const _Duration& _Dur, __std_tzdb_sys_info_type _Type) const { using _Internal_duration = duration<__std_tzdb_epoch_milli, milli>; const auto _Internal_dur = _CHRONO duration_cast<_Internal_duration>(_Dur); const unique_ptr<__std_tzdb_sys_info, _Tzdb_deleter<__std_tzdb_sys_info>> _Info{ - __std_tzdb_get_sys_info(_Name.c_str(), _Name.length(), _Internal_dur.count())}; + __std_tzdb_get_sys_info_v2(_Name.c_str(), _Name.length(), _Internal_dur.count(), _Type)}; if (_Info == nullptr) { _Xbad_alloc(); } else if (_Info->_Err == __std_tzdb_error::_Win_error) { @@ -1897,7 +1839,71 @@ namespace chrono { .end = _End, .offset = _CHRONO duration_cast(_Internal_duration{_Info->_Offset}), .save = _CHRONO duration_cast(_Internal_duration{_Info->_Save}), - .abbrev = _Info->_Abbrev}; + .abbrev = _Info->_Abbrev ? _Info->_Abbrev : ""}; + } + + template + _NODISCARD local_info _Get_local_info( + const local_time<_Duration>& _Local, __std_tzdb_sys_info_type _Type) const { + local_info _Info{}; + const auto _Time_since_ep = _Local.time_since_epoch(); + _Info.first = _Get_info(_Time_since_ep, _Type); + + const sys_seconds _Local_sys{_CHRONO duration_cast(_Time_since_ep)}; + const auto _Curr_sys = _Local_sys - _Info.first.offset; + if (_Info.first.begin != _Min_seconds && _Curr_sys < _Info.first.begin + days{1}) { + // get previous transition information + _Info.second = _Get_info((_Info.first.begin - seconds{1}).time_since_epoch(), _Type); + + const auto _Transition = _Info.first.begin; + const auto _Prev_sys = _Local_sys - _Info.second.offset; + if (_Curr_sys >= _Transition) { + if (_Prev_sys < _Transition) { + _Info.result = local_info::ambiguous; + _STD swap(_Info.first, _Info.second); + } else { + _Info.result = local_info::unique; + _Info.second = {}; + } + } else { + if (_Prev_sys >= _Transition) { + _Info.result = local_info::nonexistent; + _STD swap(_Info.first, _Info.second); + } else { + _Info.result = local_info::unique; + _Info.first = _STD move(_Info.second); + _Info.second = {}; + } + } + } else if (_Info.first.end != _Max_seconds && _Curr_sys > _Info.first.end - days{1}) { + // get next transition information + _Info.second = _Get_info((_Info.first.end + seconds{1}).time_since_epoch(), _Type); + + const auto _Transition = _Info.first.end; + const auto _Next_sys = _Local_sys - _Info.second.offset; + if (_Curr_sys < _Transition) { + if (_Next_sys >= _Transition) { + _Info.result = local_info::ambiguous; + } else { + _Info.result = local_info::unique; + _Info.second = {}; + } + } else { + if (_Next_sys < _Transition) { + _Info.result = local_info::nonexistent; + } else { + _Info.result = local_info::unique; + _Info.first = _STD move(_Info.second); + _Info.second = {}; + } + } + } else { + // local time is contained inside of first transition boundaries by at least 1 day + _Info.result = local_info::unique; + _Info.second = {}; + } + + return _Info; } string _Name; diff --git a/stl/src/msvcp_atomic_wait.src b/stl/src/msvcp_atomic_wait.src index 0ea7194ccc4..771e0942c77 100644 --- a/stl/src/msvcp_atomic_wait.src +++ b/stl/src/msvcp_atomic_wait.src @@ -35,6 +35,7 @@ EXPORTS __std_tzdb_delete_time_zones __std_tzdb_get_current_zone __std_tzdb_get_leap_seconds + __std_tzdb_get_sys_info_v2 __std_tzdb_get_sys_info __std_tzdb_get_time_zones __std_wait_for_threadpool_work_callbacks diff --git a/stl/src/tzdb.cpp b/stl/src/tzdb.cpp index c43853bdc14..bfa59e228b2 100644 --- a/stl/src/tzdb.cpp +++ b/stl/src/tzdb.cpp @@ -483,8 +483,8 @@ void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* cons } } -[[nodiscard]] __std_tzdb_sys_info* __stdcall __std_tzdb_get_sys_info( - const char* _Tz, const size_t _Tz_len, __std_tzdb_epoch_milli _Sys) noexcept { +[[nodiscard]] __std_tzdb_sys_info* __stdcall __std_tzdb_get_sys_info_v2( + const char* _Tz, const size_t _Tz_len, __std_tzdb_epoch_milli _Sys, __std_tzdb_sys_info_type _Type) noexcept { // On exit--- // _Info == nullptr --> bad_alloc // _Info->_Err == _Win_error --> failed, call GetLastError() @@ -528,6 +528,10 @@ void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* cons return _Report_error(_Info, __std_tzdb_error::_Icu_error); } + if (_Type == __std_tzdb_sys_info_type::_Offset_only) { + return _Info.release(); + } + UDate _Transition{}; _Info->_Begin = __icu_ucal_getTimeZoneTransitionDate(_Cal.get(), UTimeZoneTransitionType::UCAL_TZ_TRANSITION_PREVIOUS_INCLUSIVE, &_Transition, &_UErr) @@ -545,6 +549,10 @@ void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* cons return _Report_error(_Info, __std_tzdb_error::_Icu_error); } + if (_Type == __std_tzdb_sys_info_type::_Offset_and_range) { + return _Info.release(); + } + int32_t _Abbrev_len{}; const auto _Abbrev = _Get_timezone_short_id(_Cal.get(), _Is_daylight, _Abbrev_len, _Info->_Err); if (_Abbrev == nullptr) { @@ -559,6 +567,11 @@ void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* cons return _Info.release(); } +__std_tzdb_sys_info* __stdcall __std_tzdb_get_sys_info( + const char* _Tz, const size_t _Tz_len, __std_tzdb_epoch_milli _Sys) noexcept { + return __std_tzdb_get_sys_info_v2(_Tz, _Tz_len, _Sys, __std_tzdb_sys_info_type::_Full); +} + void __stdcall __std_tzdb_delete_sys_info(__std_tzdb_sys_info* const _Info) noexcept { if (_Info) { delete[] _Info->_Abbrev; From 00337aabfc6c538478a0defbb1d40b5b40d2c575 Mon Sep 17 00:00:00 2001 From: cpplearner Date: Sun, 19 Mar 2023 12:20:02 +0800 Subject: [PATCH 2/4] Second try --- stl/inc/__msvc_tzdb.hpp | 4 ++-- stl/inc/chrono | 16 +++++++++++++++- stl/src/msvcp_atomic_wait.src | 1 - stl/src/tzdb.cpp | 17 ++++++++--------- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/stl/inc/__msvc_tzdb.hpp b/stl/inc/__msvc_tzdb.hpp index ff4267632b0..55adf4969d0 100644 --- a/stl/inc/__msvc_tzdb.hpp +++ b/stl/inc/__msvc_tzdb.hpp @@ -78,8 +78,8 @@ void __stdcall __std_tzdb_delete_time_zones(__std_tzdb_time_zones_info* _Info) n _NODISCARD __std_tzdb_current_zone_info* __stdcall __std_tzdb_get_current_zone() noexcept; void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* _Info) noexcept; -_NODISCARD __std_tzdb_sys_info* __stdcall __std_tzdb_get_sys_info_v2( - const char* _Tz, size_t _Tz_len, __std_tzdb_epoch_milli _Local, __std_tzdb_sys_info_type _Type) noexcept; +_NODISCARD __std_tzdb_sys_info* __stdcall __std_tzdb_get_sys_info( + const char* _Tz, size_t _Tz_len, __std_tzdb_epoch_milli _Local) noexcept; void __stdcall __std_tzdb_delete_sys_info(__std_tzdb_sys_info* _Info) noexcept; _NODISCARD __std_tzdb_leap_info* __stdcall __std_tzdb_get_leap_seconds( diff --git a/stl/inc/chrono b/stl/inc/chrono index 91b912a8927..76cf35b6436 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -1813,8 +1813,22 @@ namespace chrono { _NODISCARD sys_info _Get_info(const _Duration& _Dur, __std_tzdb_sys_info_type _Type) const { using _Internal_duration = duration<__std_tzdb_epoch_milli, milli>; const auto _Internal_dur = _CHRONO duration_cast<_Internal_duration>(_Dur); + + // TRANSITION, vNext + // Because the signature of __std_tzdb_get_sys_info cannot be changed, the option is encoded in the + // time zone name. In vNext, this should be a dedicated argument. + auto _Tz_arg = _Name; + if (_Type == __std_tzdb_sys_info_type::_Offset_only) { + _Tz_arg.push_back('!'); + } else if (_Type == __std_tzdb_sys_info_type::_Offset_and_range) { + _Tz_arg.push_back('#'); + } + + const auto _Tz_len = _Name.length(); + const unique_ptr<__std_tzdb_sys_info, _Tzdb_deleter<__std_tzdb_sys_info>> _Info{ - __std_tzdb_get_sys_info_v2(_Name.c_str(), _Name.length(), _Internal_dur.count(), _Type)}; + __std_tzdb_get_sys_info(_Tz_arg.c_str(), _Tz_len, _Internal_dur.count())}; + if (_Info == nullptr) { _Xbad_alloc(); } else if (_Info->_Err == __std_tzdb_error::_Win_error) { diff --git a/stl/src/msvcp_atomic_wait.src b/stl/src/msvcp_atomic_wait.src index 771e0942c77..0ea7194ccc4 100644 --- a/stl/src/msvcp_atomic_wait.src +++ b/stl/src/msvcp_atomic_wait.src @@ -35,7 +35,6 @@ EXPORTS __std_tzdb_delete_time_zones __std_tzdb_get_current_zone __std_tzdb_get_leap_seconds - __std_tzdb_get_sys_info_v2 __std_tzdb_get_sys_info __std_tzdb_get_time_zones __std_wait_for_threadpool_work_callbacks diff --git a/stl/src/tzdb.cpp b/stl/src/tzdb.cpp index bfa59e228b2..c72f3a0cd0e 100644 --- a/stl/src/tzdb.cpp +++ b/stl/src/tzdb.cpp @@ -483,8 +483,8 @@ void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* cons } } -[[nodiscard]] __std_tzdb_sys_info* __stdcall __std_tzdb_get_sys_info_v2( - const char* _Tz, const size_t _Tz_len, __std_tzdb_epoch_milli _Sys, __std_tzdb_sys_info_type _Type) noexcept { +[[nodiscard]] __std_tzdb_sys_info* __stdcall __std_tzdb_get_sys_info( + const char* _Tz, const size_t _Tz_len, __std_tzdb_epoch_milli _Sys) noexcept { // On exit--- // _Info == nullptr --> bad_alloc // _Info->_Err == _Win_error --> failed, call GetLastError() @@ -502,6 +502,8 @@ void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* cons return _Report_error(_Info, __std_tzdb_error::_Win_error); } + // TRANSITION, vNext + // Profiling shows that _Get_cal is a hot path. Its result should be cached (preferably in the time_zone object). const auto _Cal = _Get_cal(_Tz, _Tz_len, _Info->_Err); if (_Cal == nullptr) { return _Propagate_error(_Info); @@ -528,7 +530,9 @@ void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* cons return _Report_error(_Info, __std_tzdb_error::_Icu_error); } - if (_Type == __std_tzdb_sys_info_type::_Offset_only) { + // Additional options could be stored after the time zone name. _Tz[_Tz_len] might be the one-past-the-end element, + // but it's safe to read, because _Tz is known to be obtained from a std::string. + if (_Tz[_Tz_len] == '!') { return _Info.release(); } @@ -549,7 +553,7 @@ void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* cons return _Report_error(_Info, __std_tzdb_error::_Icu_error); } - if (_Type == __std_tzdb_sys_info_type::_Offset_and_range) { + if (_Tz[_Tz_len] == '#') { return _Info.release(); } @@ -567,11 +571,6 @@ void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* cons return _Info.release(); } -__std_tzdb_sys_info* __stdcall __std_tzdb_get_sys_info( - const char* _Tz, const size_t _Tz_len, __std_tzdb_epoch_milli _Sys) noexcept { - return __std_tzdb_get_sys_info_v2(_Tz, _Tz_len, _Sys, __std_tzdb_sys_info_type::_Full); -} - void __stdcall __std_tzdb_delete_sys_info(__std_tzdb_sys_info* const _Info) noexcept { if (_Info) { delete[] _Info->_Abbrev; From 614abe53707027c6486af455970eb2ae41e27bef Mon Sep 17 00:00:00 2001 From: cpplearner Date: Sun, 19 Mar 2023 14:07:15 +0800 Subject: [PATCH 3/4] Avoid magic characters --- stl/inc/__msvc_tzdb.hpp | 2 +- stl/inc/chrono | 9 ++------- stl/src/tzdb.cpp | 10 ++++++---- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/stl/inc/__msvc_tzdb.hpp b/stl/inc/__msvc_tzdb.hpp index 55adf4969d0..36e1906d2b1 100644 --- a/stl/inc/__msvc_tzdb.hpp +++ b/stl/inc/__msvc_tzdb.hpp @@ -66,7 +66,7 @@ struct __std_tzdb_sys_info { const char* _Abbrev; }; -enum class __std_tzdb_sys_info_type { +enum class __std_tzdb_sys_info_type : char { _Full, _Offset_only, _Offset_and_range, diff --git a/stl/inc/chrono b/stl/inc/chrono index 76cf35b6436..035f0b01db0 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -1815,14 +1815,9 @@ namespace chrono { const auto _Internal_dur = _CHRONO duration_cast<_Internal_duration>(_Dur); // TRANSITION, vNext - // Because the signature of __std_tzdb_get_sys_info cannot be changed, the option is encoded in the + // Because the signature of __std_tzdb_get_sys_info cannot be changed, _Type is encoded in the // time zone name. In vNext, this should be a dedicated argument. - auto _Tz_arg = _Name; - if (_Type == __std_tzdb_sys_info_type::_Offset_only) { - _Tz_arg.push_back('!'); - } else if (_Type == __std_tzdb_sys_info_type::_Offset_and_range) { - _Tz_arg.push_back('#'); - } + const string _Tz_arg = _Name + static_cast(_Type); const auto _Tz_len = _Name.length(); diff --git a/stl/src/tzdb.cpp b/stl/src/tzdb.cpp index c72f3a0cd0e..9f92763c856 100644 --- a/stl/src/tzdb.cpp +++ b/stl/src/tzdb.cpp @@ -502,6 +502,10 @@ void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* cons return _Report_error(_Info, __std_tzdb_error::_Win_error); } + // Get the option stored after the time zone name. If there's no option, _Tz[_Tz_len] is the null terminator in the + // std::string, and will be treated the same as __std_tzdb_sys_info_type::_Full. + const auto _Type = static_cast<__std_tzdb_sys_info_type>(_Tz[_Tz_len]); + // TRANSITION, vNext // Profiling shows that _Get_cal is a hot path. Its result should be cached (preferably in the time_zone object). const auto _Cal = _Get_cal(_Tz, _Tz_len, _Info->_Err); @@ -530,9 +534,7 @@ void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* cons return _Report_error(_Info, __std_tzdb_error::_Icu_error); } - // Additional options could be stored after the time zone name. _Tz[_Tz_len] might be the one-past-the-end element, - // but it's safe to read, because _Tz is known to be obtained from a std::string. - if (_Tz[_Tz_len] == '!') { + if (_Type == __std_tzdb_sys_info_type::_Offset_only) { return _Info.release(); } @@ -553,7 +555,7 @@ void __stdcall __std_tzdb_delete_current_zone(__std_tzdb_current_zone_info* cons return _Report_error(_Info, __std_tzdb_error::_Icu_error); } - if (_Tz[_Tz_len] == '#') { + if (_Type == __std_tzdb_sys_info_type::_Offset_and_range) { return _Info.release(); } From 0896b0ca1796036fb9b13cc2ad2f23ced59e501d Mon Sep 17 00:00:00 2001 From: Nicole Mazzuca Date: Mon, 27 Mar 2023 10:12:40 -0700 Subject: [PATCH 4/4] add comment to `__std_tzdb_sys_info_type` --- stl/inc/__msvc_tzdb.hpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/stl/inc/__msvc_tzdb.hpp b/stl/inc/__msvc_tzdb.hpp index 36e1906d2b1..137f17d908c 100644 --- a/stl/inc/__msvc_tzdb.hpp +++ b/stl/inc/__msvc_tzdb.hpp @@ -67,7 +67,11 @@ struct __std_tzdb_sys_info { }; enum class __std_tzdb_sys_info_type : char { - _Full, + // TRANSITION, ABI: In order to be compatible with existing object files which do not know about + // `__std_tzdb_sys_info_type`, the type is passed in the after-end byte of a string passed with its length to + // `__std_tzdb_get_sys_info`. Since older object files always pass the `.c_str()` of a `std::string` + // to that function, the after-end byte will always be '\0'. + _Full = '\0', _Offset_only, _Offset_and_range, };