User/dmachaj/slim source location

[dmachaj]

Fixes: #1337

Why is this change being made?

This change is being made because a lot of projects are having to disable std::source_location usage because the binary size impact is too large or because they are overly concerned about information leakage with fully-annotated function names being in the binary. We have see binaries spike in size by 50% (see https://devblogs.microsoft.com/performance-diagnostics/deep-dive-analysis-why-did-this-dll-increase-in-file-size-by-50/ for an example).

The biggest contributor here is std::source_location::function_name. Those have the largest number of unique strings and they are often the largest strings too. This is especially bad when the function is templated because each expansion gets a unique name in the binary and they cannot fold. Sadly it is not possible to selectively disable this aspect without disabling all of it. Function name is also less critical than file name and line number. With file/line information the function name can be figured out from the source code.

This set of changes are being made to try and get the best bang for our binary-size buck.  

Briefly summarize what changed

A new winrt::details::slim_source_location struct has been created to be highly similar to std::source_location while skipping function_name. This is powered using __builtin_LINE and __builtin_FILE so it should be the same underlying implementation provided by the compiler. It has the same fields so it should be a drop-in replacement.

Instead of a wholesale replacement the behavior here now depends on whether _DEBUG is defined. The theory is that debug builds are less sensitive to binary size so including function names is helpful and the downside doesn't matter. Non-debug (aka release) builds use the new slim_source_location struct instead.

I encountered an order-of-concatenation problem between base_types.h and base_macros.h so I had to include this struct in base_macros. The type must be fully defined before it can be used by the macros. The types header comes after the macros so the order of operations was wrong. This is not ideal but it compiles.

I also tweaked the ODR violation check to have a different value between these source location types. Linking translation units with different understandings of source location will trigger a build break.

How was this change tested?

build_test_all.cmd locally.

I also swapped out the cppwinrt.exe used by our product code and rebuilt the project as both debug and release. (The latest stable release was 6 months ago and has had breaking changes so the switchover was annoyingly nontrivial.). When debugging I can see that the originate method has std::source_location in debug builds and winrt::impl::slim_source_location in release builds. The function_name pointer is null in release builds. The same is true of the WIL logging callback that is hit by originate.

I used SizeBench to compare release builds before and after this change. The difference is tiny (2KB). Most files already had strings in the binary thanks to WIL logging. The increase is from cppwinrt generated headers having origination information, which is expected. The small code size increase is worth it for the improved debuggability, in my opinion.

[jonwis]

There was a test that validated source location - does that need updating at all?

[dmachaj]

There was a test that validated source location - does that need updating at all?

D'oh I needed to scroll up in my local test run to see the failure. I didn't think it would fail because this is a drop-in replacement (aside from function name).

[sylveon]

Should there be a detect_mismatch between this slim source location and std::source_location?

[dmachaj]

Should there be a detect_mismatch between this slim source location and std::source_location?

There already is. There is a preexisting ODR check for whether source_location is on or off ("true" vs. "false"). The slim source location ODR has a new "slim" value for that ODR check that will mismatch against the two preexisting values.

See:

#ifdef _MSC_VER
#pragma detect_mismatch("WINRT_SOURCE_LOCATION", "slim")
#endif // _MSC_VER