microsoft / cppwinrt

C++/WinRT
MIT License
1.64k stars 236 forks source link

Investigate a fully constexpr definition for literal hstrings #1339

Closed jonwis closed 1 year ago

jonwis commented 1 year ago
          > Making the constructor of `param::hstring` constexpr seems pretty trivial, and it would enable constexpr fast-pass strings.

I was referring to return values, which has a lot more optimizing value.

Originally posted by @kennykerr in https://github.com/microsoft/cppwinrt/issues/1338#issuecomment-1669861968

FWIW, I was noodling with something like this for constexpr/consteval "mini source location" per #1337 and got pretty far. Not super pleasant to look at, though.

kennykerr commented 1 year ago

The main challenge is ensuring that the backing memory remains pinned, so it might only be plausible in combination with WINRT_NO_MODULE_LOCK - see also #1031 - which would ideally be the default anyway.

jonwis commented 1 year ago

In the spirit of full transparency, I made an attempt at this and got basically nothing back. See https://github.com/microsoft/cppwinrt/compare/master...jonwis:cppwinrt:user/jonwis/const-param-strings?expand=1 ... the only real codegen improvement appeared to be that the "test length and abort" was compiled out, but it was very small. And the consteval part didn't work at all and actually increased binary size by a teeny amount.

So while it's doable, it's not super clear what value there is here. Maybe a real "literal" like L"foo"hs could help generate literals.

sylveon commented 1 year ago

We could use something like std::format: a type whose only constructor is consteval, that way it lets us do compile-time tricks to the string.

jonwis commented 1 year ago

winrt::param::hstring is used to take both constants (like Foo(L"bar")) as well as "kind of constants" (like const wchar_t* foo = L"foo"; Foo(foo);) ... the first one hits the right consteval one in my PR, the second does not. It generates a "use of variable outside local scope" error, presumably because the dataflow thing is observing the foo value being passed in, and not the literal that it points to.

I'll keep this issue open and noodle on it some more, but the next area of investigation for me is in reducing the cost of this:

winrt::Windows::Data::JsonArray a;
a.Append(winrt::Windows::Data::Json::JsonValue::CreateStringValue(L"foo"));
a.Append(winrt::Windows::Data::Json::JsonValue::CreateStringValue(L"bas"));
winrt::Windows::Data::JsonObject j;
j.SetNamedValue(L"muffins", a);

Question for the curious reader - how many queryinterface and release calls are in there?

sylveon commented 1 year ago

A: too many

kennykerr commented 1 year ago

Regarding queries, I added WINRT_DIAGNOSTICS when I was working on the fast ABI:

#define WINRT_DIAGNOSTICS
#include "winrt/Windows.Foundation.Collections.h"
#include "winrt/Windows.Data.Json.h"

void print_diagnostics()
{
    auto info = winrt::impl::get_diagnostics_info().detach();
    printf("\nQueries:\n");
    for (auto& pair : info.queries)
    {
        printf("%ls %d\n", pair.first.data(), pair.second);
    }
    printf("\nFactories:\n");
    for (auto& pair : info.factories)
    {
        printf("%ls %d agile=%s\n", pair.first.data(), pair.second.requests, pair.second.is_agile ? "yes" : "no");
    }
}

int main()
{
    winrt::Windows::Data::Json::JsonArray a;
    a.Append(winrt::Windows::Data::Json::JsonValue::CreateStringValue(L"foo"));
    a.Append(winrt::Windows::Data::Json::JsonValue::CreateStringValue(L"bas"));
    winrt::Windows::Data::Json::JsonObject j;
    j.SetNamedValue(L"muffins", a);

    print_diagnostics();
}
Queries:
IAgileObject 3
Windows.Data.Json.IJsonValue 1
Windows.Data.Json.JsonArray 1
Windows.Data.Json.JsonObject 1
Windows.Foundation.Collections.IVector`1<Windows.Data.Json.IJsonValue> 2

Factories:
Windows.Data.Json.JsonArray 1 agile=yes
Windows.Data.Json.JsonObject 1 agile=yes
Windows.Data.Json.JsonValue 1 agile=yes

Regarding strings, part of the trick is to place not only the literal but the entire HSTRING header into a constant. You can then make an HSTRING point to this header and pass it out as if it is reference counted, and thus returnable. This will then entirely avoid the allocation necessary when returning an HSTRING. Here's what this looks like in Rust.

github-actions[bot] commented 1 year 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.

kennykerr commented 10 months ago

For posterity, I did prototype constexpr hstring references and it didn't really move the needle. The performance is basically a rounding error compared with the overhead of virtual function calls and the likes of QueryInterface.

jonwis commented 10 months ago

Thanks, that's mostly what I came to as well. Zero real value for it. Best advice is to use your language's preferred string primitives as much as possible, and only use hstring when actually working with WinRT types. We had/have a bunch of places internally that are to_hstring(custom-enum-type) like that should have been to_wstring_view(custom-enum-type) instead where this could have helped but didn't really.

I have a long list of similar tidbits of guidance that I need to publish somewhere.