google / cctz

CCTZ is a C++ library for translating between absolute and civil times using the rules of a time zone.
Apache License 2.0
597 stars 166 forks source link

Use std::shared_mutex in time_zone_impl.cc #304

Closed hengfengli closed 2 days ago

hengfengli commented 3 days ago

We have observed high lock contention issues (consume a lot of CPUs) when calling absl::LoadTimeZone. We wonder if it is better to use std::shared_mutex to allow multiple readers to access the time zone map concurrently. The entries in the time zone map are never modified after they are initialized.

hengfengli commented 3 days ago

I just saw the thread in https://github.com/google/cctz/issues/212 so I guess the suggestion is still to use std::lock_guard and the application should build its own cache to avoid too many absl::LoadTimeZone calls, is this right?

devbww commented 3 days ago

The entries in the time zone map are never modified after they are initialized.

Right. That's why you can use the returned absl::TimeZone without further synchronization.

I just saw the thread in #212 so I guess the suggestion is still to use std::lock_guard and the application should build its own cache to avoid too many absl::LoadTimeZone calls, is this right?

Yes. But you might also consider why you're calling absl::LoadTimeZone() so much.

hengfengli commented 2 days ago

Close this. As suggested, we should avoid calling absl::LoadTimeZone() too much.