Skip to content

<chrono>: Optimize to_sys and to_local #3579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 28, 2023
Merged

<chrono>: Optimize to_sys and to_local #3579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ba3e8f3
Optimize to_sys and to_local
cpplearner Feb 10, 2023
00337aa
Second try
cpplearner Mar 19, 2023
614abe5
Avoid magic characters
cpplearner Mar 19, 2023
0896b0c
add comment to `__std_tzdb_sys_info_type`
strega-nil Mar 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions stl/inc/__msvc_tzdb.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@ struct __std_tzdb_sys_info {
const char* _Abbrev;
};

enum class __std_tzdb_sys_info_type : char {
// 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,
};

_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;

Expand Down
145 changes: 80 additions & 65 deletions stl/inc/chrono
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1765,70 +1765,12 @@ namespace chrono {

template <class _Duration>
_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 <class _Duration>
_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<sys_seconds::duration>(_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 <class _Duration>
Expand All @@ -1846,7 +1788,7 @@ namespace chrono {
template <class _Duration>
_NODISCARD sys_time<common_type_t<_Duration, seconds>> 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;
}
Expand All @@ -1859,7 +1801,7 @@ namespace chrono {

template <class _Duration>
_NODISCARD local_time<common_type_t<_Duration, seconds>> 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<common_type_t<_Duration, seconds>>{_Sys.time_since_epoch() + _Info.offset};
}

Expand All @@ -1868,11 +1810,20 @@ namespace chrono {

private:
template <class _Duration>
_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);

// TRANSITION, vNext
// 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.
const string _Tz_arg = _Name + static_cast<char>(_Type);

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(_Name.c_str(), _Name.length(), _Internal_dur.count())};
__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) {
Expand All @@ -1897,7 +1848,71 @@ namespace chrono {
.end = _End,
.offset = _CHRONO duration_cast<seconds>(_Internal_duration{_Info->_Offset}),
.save = _CHRONO duration_cast<minutes>(_Internal_duration{_Info->_Save}),
.abbrev = _Info->_Abbrev};
.abbrev = _Info->_Abbrev ? _Info->_Abbrev : ""};
}

template <class _Duration>
_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<sys_seconds::duration>(_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;
Expand Down
14 changes: 14 additions & 0 deletions stl/src/tzdb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,12 @@ 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);
if (_Cal == nullptr) {
return _Propagate_error(_Info);
Expand All @@ -528,6 +534,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)
Expand All @@ -545,6 +555,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();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Noting that if _Type \notin {0, 1, 2}, you'll get the full info; this means that future extensions will get all of the info, which seems reasonable, and means it'll be easy to extend. (no change requested, this is great!)


int32_t _Abbrev_len{};
const auto _Abbrev = _Get_timezone_short_id(_Cal.get(), _Is_daylight, _Abbrev_len, _Info->_Err);
if (_Abbrev == nullptr) {
Expand Down