microsoft / wil

Windows Implementation Library
MIT License
2.58k stars 236 forks source link

Replace static_lazy<> with normal statics #471

Open jonwis opened 2 months ago

jonwis commented 2 months ago

https://github.com/microsoft/wil/blob/eb9eb355d6de431311d7f5ea3590c72d53741ccc/include/wil/Tracelogging.h#L188-L204

According to https://learn.microsoft.com/en-us/cpp/cpp/storage-classes-cpp?view=msvc-170, function-local "magic statics" are fully supported.

Instead of defining a custom static_lazy type, use the built-in support.

  1. Remove static_lazy<>
  2. On line 2283ish in the Instance() method:
static TraceLoggingClassName* Instance() WI_NOEXCEPT \
{ \
    static TraceLoggingClassName wrapper; \
    return wrapper; \
} \
friend class wil::details::static_lazy<TraceLoggingClassName>;
ChrisGuzak commented 1 month ago

magic statics have some overhead that some components, dlls that are loaded in many processes, what to avoid. using wil::init_once avoids that, at the cost of more complicated code. is that your understanding?

jonwis commented 1 week ago

magic statics have some overhead that some components, dlls that are loaded in many processes, what to avoid. using wil::init_once avoids that, at the cost of more complicated code. is that your understanding?

I'd like to understand that overhead and take feature requests to the CRT team if necessary. Magic statics are the intended language tool for this problem. Do you have examples of swapping init_once to magic statics so we can see the impact?

sylveon commented 1 week ago

I was under the impression that statics use InitOnceBeginInitialize or something equivalent, and that using static_lazy would basically just be replacing automatic overhead with manual overhead.

You're also already paying that cost anyways, because this is how it's used currently: https://github.com/microsoft/wil/blob/6f60a1b76fb812c6af5db1bc7abdec0001edd43f/include/wil/Tracelogging.h#L2287