Cache the local time zone calculation on Windows #1791
Conversation
There was a problem hiding this comment.
A couple things.
Changes to this code must be send to https://github.com/google/cctz. We automatically import this code from there.
However, I'm not sure the changes would be accepted. I am a maintainer of CCTZ but I usually defer to @devbww if he has an opinion on CCTZ code. In general though I think gRPC should be caching absl::LocalTimeZone() and passing that to absl::FormatTime(). As I think you understand from the text of this pull request, the local time zone can change while the program is running and caching would defeat that and may do something unexpected.
Also note that Abseil's logging library caches the local time: https://github.com/abseil/abseil-cpp/blob/c7cf999bda8390d2dd294ef903716a80135e6f4c/absl/log/initialize.cc
| } | ||
|
|
||
| static std::string& cached_win32_local_time_zone() { | ||
| static std::string cached_time_zone = get_once_win32_local_time_zone(); |
There was a problem hiding this comment.
Note that this is not safe for global destruction. See https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables.
There was a problem hiding this comment.
Good point! I wonder what else can be done here - maybe leaking the object? (Not ideal)
There was a problem hiding this comment.
Yes that is what I would do, but I was just pointing that out. I think the approach here is wrong anyway. See above.
|
Per @derekmauro 's comment - closing this as it needs to be really cached in |
I agree with Derek, and just made a comment to that effect on grpc/grpc#37766, for what that's worth. |
On Windows 10+, combase.dll gets loaded, unloaded, then subsequently various COM-related functions called to initialize/deinitialize the system leading to performance degradation #1760
This "fix" here caches the value once. Obviously this won't work for long running apps, so ideally some kind of way knowing when to update the value are needed (like subscribing to time zone changes, or simply refreshing the value every once in a while).
In the mean time, this affects grpc for Windows where if grpc returns anything else than grpc::Status::OK it'll have to call this function. For processes running into grpc issues this means extra time added. It's also not a thing that normally would be caught, and I could not find benchmark for this case.
grpc/grpc#37766