From fad780afc21a16a58667fa03f3c2f736b4f0314c Mon Sep 17 00:00:00 2001 From: YexuanXiao Date: Tue, 27 May 2025 09:20:25 +0800 Subject: [PATCH 1/8] Use `lower_bound` instead of `find_if` to lookup `time_zone`s. --- benchmarks/src/locate_zone.cpp | 19 +++++++++++++++++++ stl/inc/chrono | 5 +++-- 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 benchmarks/src/locate_zone.cpp diff --git a/benchmarks/src/locate_zone.cpp b/benchmarks/src/locate_zone.cpp new file mode 100644 index 00000000000..1bc0d7f3fd0 --- /dev/null +++ b/benchmarks/src/locate_zone.cpp @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "benchmark/benchmark.h" +#include + +void locate_zone(benchmark::State& state) { + auto&& tzdb = std::chrono::get_tzdb(); + for (auto _ : state) { + for (std::size_t i = 0; i != tzdb.zones.size(); ++i) { + auto* res = tzdb.locate_zone(tzdb.zones[i].name()); + benchmark::DoNotOptimize(res); + } + } +} + +BENCHMARK(locate_zone); + +BENCHMARK_MAIN(); diff --git a/stl/inc/chrono b/stl/inc/chrono index d6558177e2f..ff9fd865f60 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -2105,8 +2105,9 @@ namespace chrono { template _NODISCARD const _Ty* _Locate_zone_impl(const vector<_Ty>& _Vec, string_view _Name) { - const auto _Result = _STD find_if(_Vec.begin(), _Vec.end(), [&](auto& _Tz) { return _Tz.name() == _Name; }); - return _Result == _Vec.end() ? nullptr : &*_Result; + const auto _Result = + _STD lower_bound(_Vec.begin(), _Vec.end(), _Name, [](auto& _Tz, auto& _Sv) { return _Tz.name() < _Sv; }); + return (_Result == _Vec.end() || _Result->name() != _Name) ? nullptr : &*_Result; } _EXPORT_STD struct tzdb { From 48c0d8859388cae3720dbd925a6b585ed8fe705b Mon Sep 17 00:00:00 2001 From: YexuanXiao Date: Tue, 27 May 2025 19:10:56 +0800 Subject: [PATCH 2/8] Remove the extra asterisk and add the benchmark to CMakeLists.txt --- benchmarks/CMakeLists.txt | 1 + benchmarks/src/locate_zone.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index 719d7deec21..06fa89b1c68 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -114,6 +114,7 @@ add_benchmark(has_single_bit src/has_single_bit.cpp) add_benchmark(iota src/iota.cpp) add_benchmark(is_sorted_until src/is_sorted_until.cpp) add_benchmark(locale_classic src/locale_classic.cpp) +add_benchmark(locate_zone src/locate_zone.cpp) add_benchmark(minmax_element src/minmax_element.cpp) add_benchmark(mismatch src/mismatch.cpp) add_benchmark(move_only_function src/move_only_function.cpp) diff --git a/benchmarks/src/locate_zone.cpp b/benchmarks/src/locate_zone.cpp index 1bc0d7f3fd0..277fdab2987 100644 --- a/benchmarks/src/locate_zone.cpp +++ b/benchmarks/src/locate_zone.cpp @@ -8,7 +8,7 @@ void locate_zone(benchmark::State& state) { auto&& tzdb = std::chrono::get_tzdb(); for (auto _ : state) { for (std::size_t i = 0; i != tzdb.zones.size(); ++i) { - auto* res = tzdb.locate_zone(tzdb.zones[i].name()); + auto res = tzdb.locate_zone(tzdb.zones[i].name()); benchmark::DoNotOptimize(res); } } From 3347fa171b5327fcde70c423c3b2ddf6a28dd25c Mon Sep 17 00:00:00 2001 From: YexuanXiao Date: Wed, 28 May 2025 06:35:59 +0800 Subject: [PATCH 3/8] Make the references point to const objects --- stl/inc/chrono | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index ff9fd865f60..3920d0f4864 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -2105,8 +2105,8 @@ namespace chrono { template _NODISCARD const _Ty* _Locate_zone_impl(const vector<_Ty>& _Vec, string_view _Name) { - const auto _Result = - _STD lower_bound(_Vec.begin(), _Vec.end(), _Name, [](auto& _Tz, auto& _Sv) { return _Tz.name() < _Sv; }); + const auto _Result = _STD lower_bound( + _Vec.begin(), _Vec.end(), _Name, [](const auto& _Tz, const auto& _Sv) { return _Tz.name() < _Sv; }); return (_Result == _Vec.end() || _Result->name() != _Name) ? nullptr : &*_Result; } From f27f49fea834a936cb2597600511d6700cc5e551 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 May 2025 09:48:01 -0700 Subject: [PATCH 4/8] Use range-for. --- benchmarks/src/locate_zone.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/benchmarks/src/locate_zone.cpp b/benchmarks/src/locate_zone.cpp index 277fdab2987..2a5c369be69 100644 --- a/benchmarks/src/locate_zone.cpp +++ b/benchmarks/src/locate_zone.cpp @@ -7,8 +7,8 @@ void locate_zone(benchmark::State& state) { auto&& tzdb = std::chrono::get_tzdb(); for (auto _ : state) { - for (std::size_t i = 0; i != tzdb.zones.size(); ++i) { - auto res = tzdb.locate_zone(tzdb.zones[i].name()); + for (const auto& z : tzdb.zones) { + auto res = tzdb.locate_zone(z.name()); benchmark::DoNotOptimize(res); } } From fccce41c09c43539ffa4d474f368c2341e2784fb Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 May 2025 09:51:51 -0700 Subject: [PATCH 5/8] Use `const auto&`, avoid quasi-shadowing `tzdb`. --- benchmarks/src/locate_zone.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/benchmarks/src/locate_zone.cpp b/benchmarks/src/locate_zone.cpp index 2a5c369be69..08eff0983ca 100644 --- a/benchmarks/src/locate_zone.cpp +++ b/benchmarks/src/locate_zone.cpp @@ -5,10 +5,10 @@ #include void locate_zone(benchmark::State& state) { - auto&& tzdb = std::chrono::get_tzdb(); + const auto& db = std::chrono::get_tzdb(); for (auto _ : state) { - for (const auto& z : tzdb.zones) { - auto res = tzdb.locate_zone(z.name()); + for (const auto& z : db.zones) { + auto res = db.locate_zone(z.name()); benchmark::DoNotOptimize(res); } } From 8cdf08e55b913a58587359e3cac1ae473ced4e02 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 May 2025 10:02:56 -0700 Subject: [PATCH 6/8] Conditional operator => if-statement. --- stl/inc/chrono | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 3920d0f4864..d8149616c86 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -2107,7 +2107,11 @@ namespace chrono { _NODISCARD const _Ty* _Locate_zone_impl(const vector<_Ty>& _Vec, string_view _Name) { const auto _Result = _STD lower_bound( _Vec.begin(), _Vec.end(), _Name, [](const auto& _Tz, const auto& _Sv) { return _Tz.name() < _Sv; }); - return (_Result == _Vec.end() || _Result->name() != _Name) ? nullptr : &*_Result; + if (_Result == _Vec.end() || _Result->name() != _Name) { + return nullptr; + } else { + return &*_Result; + } } _EXPORT_STD struct tzdb { From a0a73715a3ad491f3fae10b6e65cbe6e4453af25 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 May 2025 10:04:53 -0700 Subject: [PATCH 7/8] De Morgan isn't just a good idea - it's the law. --- stl/inc/chrono | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index d8149616c86..59ac07ac7d1 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -2107,10 +2107,10 @@ namespace chrono { _NODISCARD const _Ty* _Locate_zone_impl(const vector<_Ty>& _Vec, string_view _Name) { const auto _Result = _STD lower_bound( _Vec.begin(), _Vec.end(), _Name, [](const auto& _Tz, const auto& _Sv) { return _Tz.name() < _Sv; }); - if (_Result == _Vec.end() || _Result->name() != _Name) { - return nullptr; - } else { + if (_Result != _Vec.end() && _Result->name() == _Name) { return &*_Result; + } else { + return nullptr; } } From 5a556108c5f431b2e4946d1a5fc55fdd12c83294 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 May 2025 10:28:35 -0700 Subject: [PATCH 8/8] Comment and test the sorted guarantee. --- stl/inc/chrono | 1 + .../test.cpp | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/stl/inc/chrono b/stl/inc/chrono index 59ac07ac7d1..d85567af8ff 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -2105,6 +2105,7 @@ namespace chrono { template _NODISCARD const _Ty* _Locate_zone_impl(const vector<_Ty>& _Vec, string_view _Name) { + // N5008 [time.zone.db.tzdb]/1: "Each vector in a tzdb object is sorted to enable fast lookup." const auto _Result = _STD lower_bound( _Vec.begin(), _Vec.end(), _Name, [](const auto& _Tz, const auto& _Sv) { return _Tz.name() < _Sv; }); if (_Result != _Vec.end() && _Result->name() == _Name) { diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_time_zones/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_time_zones/test.cpp index dc509bea954..fdfa456263b 100644 --- a/tests/std/tests/P0355R7_calendars_and_time_zones_time_zones/test.cpp +++ b/tests/std/tests/P0355R7_calendars_and_time_zones_time_zones/test.cpp @@ -393,6 +393,15 @@ void timezone_precision_test() { } } +void timezone_sorted_vectors_test() { + // N5008 [time.zone.db.tzdb]/1: "Each vector in a tzdb object is sorted to enable fast lookup." + const auto& my_tzdb = get_tzdb(); + + assert(ranges::is_sorted(my_tzdb.zones)); + assert(ranges::is_sorted(my_tzdb.links)); + assert(ranges::is_sorted(my_tzdb.leap_seconds)); +} + void test() { timezone_tzdb_list_test(); timezone_version_test(); @@ -401,6 +410,7 @@ void test() { timezone_to_local_test(); timezone_local_info_test(); timezone_precision_test(); + timezone_sorted_vectors_test(); } int main() {