microsoft / cppwinrt

C++/WinRT
MIT License
1.65k stars 238 forks source link

Bug: Calling PropertyValue.Create to handle boxing is significantly slower than allocating an IReference directly #1416

Closed sylveon closed 4 months ago

sylveon commented 4 months ago

Version

2.0.240405.15

Summary

This takes 30ms to execute:

winrt::Windows::Foundation::IInspectable ref = nullptr;
for (int i = 0; i < 200000; i++)
    ref = winrt::box_value(i + 1);

This takes 8ms to execute:

winrt::Windows::Foundation::IInspectable ref = nullptr;
for (int i = 0; i < 200000; i++)
    ref = winrt::make<winrt::impl::reference<int>>(i + 1);

It seems weird for box_value to be using PropertyValue.CreateInt32 when just directly allocating a wrapper is significantly faster.

Reproducible example

int main()
{
    winrt::init_apartment();
    const auto before = std::chrono::high_resolution_clock::now();

    winrt::Windows::Foundation::IInspectable ref = nullptr;
    for (int i = 0; i < 200000; i++)
        ref = winrt::box_value(i + 1);

    const auto after = std::chrono::high_resolution_clock::now();
    const auto duration = std::chrono::duration_cast<std::chrono::milliseconds>(after - before);
    std::cout << duration.count();
}

Expected behavior

I expected boxing to be fast.

Actual behavior

It's kinda slow.

Additional comments

Spotted this while looking into https://github.com/microsoft/microsoft-ui-xaml/discussions/9154

WPF users are expecting WinUI 3 to match WPF performance - and setting 200k DPs takes 21ms with WPF, but 130ms with WinUI 3. We ought to try and match that speed where possible, and this should help by cutting middlemans, at least in C++.

JaiganeshKumaran commented 4 months ago

Reälly reminds me of those StructHelper types, which offered often basic functionality, doäble by yourself, with much greater overhead.

I approve replacing PropertyValue.Create* methods! That said, can we finally get useful interfaces like IStringable implemented on boxed objects?

kennykerr commented 4 months ago

As I recall, the PropertyValue implementations offer marshal-by-value semantics which the C++/WinRT fallback does not. This is/was required for some APIs depending on boxing. We can't change this now without the risk of breaking such callers. In practice, it's mostly only interesting to Xaml but there are other APIs that depend on this.

JaiganeshKumaran commented 4 months ago

marshal-by-value semantics

@kennykerr Can't we implement this ourselves?

sylveon commented 4 months ago

From a cursory search, CsWinRT seems to allocate its own IReference (which works fine with XAML).

github-actions[bot] commented 4 months ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.