Reduce the code size of generated consume methods by skipping casts when the type is already a match (#1448)

Why is this change being made?
I have been doing some local builds of an internal component against the latest cppwinrt.exe, which includes the fixes in #1442. I have noticed some modest (1%) binary size grown when comparing the April 2024 release and the latest. Using SizeBench to analyze binary size differences I determined that the inlined code gen seems to explain the difference. This change eliminates that increase, at least for the binary I'm testing against, and even shrinks it a bit further.

Briefly summarize what changed
The various winrt::impl::consume_THING methods get heavily inlined in release builds. The current code gen has some casts for when the types don't match (and is an AddRef/Release when they do match). The small amount of new cast result checking ends up at many call sites and slowly adds up a bit. I think we can do better in the cases where the types already match.

This seems to have proven true in practice. Using if constexpr to determine if the type is a match allows us to skip any casts when this is true. In fact that is a net improvement over the original baseline because we don't need to AddRef/Release either. We can directly call the appropriate method. When a cast is necessary the code gen is identical to the previous baseline. The QueryInterface call is unavoidable and so is checking the result.

The code writer format string is definitely starting to creak under the weight of many arguments. I don't want to refactor that as part of this PR but it would be good to simplify in the future.

Also included is late feedback from the previous PR from @oldnewthing. The check_cast_result method can take void* instead of a template argument because it only null checks it. In my local measurements the optimizer already folded them so it makes no binary size difference but it is still a nice change to take.

How was this change tested?
Ran the full suite of cppwinrt tests locally. I also debugged into Windows.Foundation.IStringable.ToString for cases where it both is and is not the correct type. They run the expected code paths. I also checked the binary size of a large example to confirm the expected reduction. The disassembly for a Release build similarly matches expectations.
This commit is contained in:
David Machaj 2024-11-11 16:09:14 -08:00 коммит произвёл GitHub
Родитель 849498c617
Коммит d296af6a43
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
2 изменённых файлов: 41 добавлений и 14 удалений

Просмотреть файл

@ -1135,10 +1135,18 @@ namespace cppwinrt
// immediately while preserving the error code and local variables.
format = R"( template <typename D%> auto consume_%<D%>::%(%) const noexcept
{%
auto const& castedResult = static_cast<% const&>(static_cast<D const&>(*this));
auto const abiType = *(abi_t<%>**)&castedResult;
check_cast_result(abiType);
abiType->%(%);%
if constexpr (!std::is_same_v<D, %>)
{
auto const& castedResult = static_cast<% const&>(static_cast<D const&>(*this));
auto const abiType = *(abi_t<%>**)&castedResult;
check_cast_result(abiType);
abiType->%(%);
}
else
{
auto const abiType = *(abi_t<%>**)this;
abiType->%(%);
}%
}
)";
}
@ -1146,10 +1154,18 @@ namespace cppwinrt
{
format = R"( template <typename D%> auto consume_%<D%>::%(%) const noexcept
{%
auto const& castedResult = static_cast<% const&>(static_cast<D const&>(*this));
auto const abiType = *(abi_t<%>**)&castedResult;
check_cast_result(abiType);
WINRT_VERIFY_(0, abiType->%(%));%
if constexpr (!std::is_same_v<D, %>)
{
auto const& castedResult = static_cast<% const&>(static_cast<D const&>(*this));
auto const abiType = *(abi_t<%>**)&castedResult;
check_cast_result(abiType);
WINRT_VERIFY_(0, abiType->%(%));
}
else
{
auto const abiType = *(abi_t<%>**)this;
WINRT_VERIFY_(0, abiType->%(%));
}%
}
)";
}
@ -1158,10 +1174,18 @@ namespace cppwinrt
{
format = R"( template <typename D%> auto consume_%<D%>::%(%) const
{%
auto const& castedResult = static_cast<% const&>(static_cast<D const&>(*this));
auto const abiType = *(abi_t<%>**)&castedResult;
check_cast_result(abiType);
check_hresult(abiType->%(%));%
if constexpr (!std::is_same_v<D, %>)
{
auto const& castedResult = static_cast<% const&>(static_cast<D const&>(*this));
auto const abiType = *(abi_t<%>**)&castedResult;
check_cast_result(abiType);
check_hresult(abiType->%(%));
}
else
{
auto const abiType = *(abi_t<%>**)this;
check_hresult(abiType->%(%));
}%
}
)";
}
@ -1175,6 +1199,10 @@ namespace cppwinrt
bind<write_consume_return_type>(signature, false),
type,
type,
type,
get_abi_name(method),
bind<write_abi_args>(signature),
type,
get_abi_name(method),
bind<write_abi_args>(signature),
bind<write_consume_return_statement>(signature));

Просмотреть файл

@ -539,8 +539,7 @@ namespace winrt::impl
return result;
}
template <typename T>
WINRT_IMPL_NOINLINE void check_cast_result(T* from, winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current())
inline WINRT_IMPL_NOINLINE void check_cast_result(void* from, winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current())
{
if (!from)
{