-
Notifications
You must be signed in to change notification settings - Fork 277
C++/WinRT ABI interop improvements #1608
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
2eab28a
a889be0
390e118
1f16ab0
0adfbeb
71cc2c6
bee7550
75bf146
ab1f299
4112ae6
6e4a578
2321741
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -837,6 +837,86 @@ WINRT_IMPL_STD_EXPORT namespace std | |
| }; | ||
| } | ||
|
|
||
| WINRT_EXPORT namespace winrt::impl | ||
| { | ||
| template <typename Derived, typename AsyncInterface> | ||
| struct ready_async_base : implements<Derived, AsyncInterface, Windows::Foundation::IAsyncInfo> | ||
| { | ||
| using AsyncStatus = Windows::Foundation::AsyncStatus; | ||
|
|
||
| void Completed(async_completed_handler_t<AsyncInterface> const& handler) | ||
| { | ||
| if (m_completed_assigned.exchange(true)) | ||
| { | ||
| throw hresult_illegal_delegate_assignment(); | ||
| } | ||
|
|
||
| if (handler) | ||
| { | ||
| winrt::impl::invoke(handler, *static_cast<Derived*>(this), AsyncStatus::Completed); | ||
| } | ||
| } | ||
|
|
||
| auto Completed() noexcept | ||
| { | ||
| return async_completed_handler_t<AsyncInterface>{ nullptr }; | ||
| } | ||
|
|
||
| std::uint32_t Id() const noexcept | ||
| { | ||
| return 1; | ||
| } | ||
|
|
||
| AsyncStatus Status() const noexcept | ||
| { | ||
| return AsyncStatus::Completed; | ||
| } | ||
|
|
||
| hresult ErrorCode() const noexcept | ||
| { | ||
| return 0; | ||
| } | ||
|
|
||
| void Cancel() const noexcept | ||
| { | ||
| } | ||
|
|
||
| void Close() const noexcept | ||
| { | ||
| } | ||
|
|
||
| private: | ||
|
|
||
| std::atomic<bool> m_completed_assigned{ false }; | ||
| }; | ||
|
|
||
| template <typename TResult> | ||
| struct ready_async_operation : | ||
| ready_async_base<ready_async_operation<TResult>, Windows::Foundation::IAsyncOperation<TResult>> | ||
| { | ||
| explicit ready_async_operation(TResult value) : m_result(std::move(value)) | ||
| { | ||
| } | ||
|
|
||
| TResult GetResults() | ||
| { | ||
| return m_result; | ||
| } | ||
|
|
||
| private: | ||
|
|
||
| TResult m_result; | ||
| }; | ||
|
|
||
| struct ready_async_action : | ||
| ready_async_base<ready_async_action, Windows::Foundation::IAsyncAction> | ||
| { | ||
| void GetResults() const noexcept | ||
| { | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| WINRT_EXPORT namespace winrt | ||
| { | ||
| template <typename... T> | ||
|
|
@@ -884,5 +964,16 @@ WINRT_EXPORT namespace winrt | |
| impl::check_status_canceled(shared->status); | ||
| co_return shared->result.GetResults(); | ||
| } | ||
|
|
||
| template <typename TResult> | ||
| Windows::Foundation::IAsyncOperation<std::decay_t<TResult>> make_ready(TResult&& value) | ||
| { | ||
| return make<impl::ready_async_operation<std::decay_t<TResult>>>(std::forward<TResult>(value)); | ||
| } | ||
|
|
||
| inline Windows::Foundation::IAsyncAction make_ready() noexcept | ||
| { | ||
| return make<impl::ready_async_action>(); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we also need WithProgress variants? Also consider the versions here, which supports both with- and without progress , as well as |
||
| } | ||
| #endif | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,35 @@ | ||
|
|
||
| WINRT_EXPORT namespace winrt::impl | ||
| { | ||
| // Detects whether an indexed (GetAt-capable) collection also exposes the bulk | ||
| // GetMany(startIndex, array) accessor. IBindableVectorView and similar have GetAt but no GetMany. | ||
| template <typename T> | ||
| class has_GetMany_indexed | ||
| { | ||
| template <typename U, typename E = decltype(std::declval<U>().GetAt(0)), | ||
| typename = decltype(std::declval<U const&>().GetMany(0u, std::declval<std::array<E, 1>&>()))> | ||
| static constexpr bool get_value(int) { return true; } | ||
| template <typename> static constexpr bool get_value(...) { return false; } | ||
|
|
||
| public: | ||
|
|
||
| static constexpr bool value = get_value<T>(0); | ||
| }; | ||
|
|
||
| // Detects whether an iterator exposes the bulk GetMany(array) accessor. IBindableIterator does not. | ||
| template <typename Iterator> | ||
| class has_GetMany_iter | ||
| { | ||
| template <typename U, typename E = decltype(std::declval<U>().Current()), | ||
| typename = decltype(std::declval<U&>().GetMany(std::declval<std::array<E, 1>&>()))> | ||
| static constexpr bool get_value(int) { return true; } | ||
| template <typename> static constexpr bool get_value(...) { return false; } | ||
|
|
||
| public: | ||
|
|
||
| static constexpr bool value = get_value<Iterator>(0); | ||
| }; | ||
|
|
||
| template <typename T> | ||
| struct fast_iterator | ||
| { | ||
|
|
@@ -73,12 +102,12 @@ WINRT_EXPORT namespace winrt::impl | |
|
|
||
| reference operator*() const | ||
| { | ||
| return m_collection->GetAt(m_index); | ||
| return fetch(m_index); | ||
| } | ||
|
|
||
| reference operator[](difference_type n) const | ||
| { | ||
| return m_collection->GetAt(m_index + static_cast<std::uint32_t>(n)); | ||
| return fetch(m_index + static_cast<std::uint32_t>(n)); | ||
| } | ||
|
|
||
| bool operator==(fast_iterator const& other) const noexcept | ||
|
|
@@ -126,8 +155,46 @@ WINRT_EXPORT namespace winrt::impl | |
|
|
||
| private: | ||
|
|
||
| // Batched forward traversal for random-access (GetAt-capable) collections that also expose | ||
| // GetMany. Instead of one GetAt ABI call per element, `fetch` fills a small block with a single | ||
| // GetMany call and serves in-window reads from it, so range-for (sequential ++ then *) crosses | ||
| // the ABI ~once per `block` elements. Random access still works: an out-of-window index | ||
| // re-anchors the block there, and an at/after-end index defers to GetAt so the component's | ||
| // bounds behavior (E_BOUNDS) is preserved. Elements are copied out -- no move-out -- so a given | ||
| // index may be read repeatedly, as the random-access contract requires. Collections without | ||
| // GetMany, or whose element type is not default-constructible, fall back to plain GetAt. | ||
| static constexpr bool can_batch = std::is_default_constructible_v<value_type> && has_GetMany_indexed<T>::value; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Has there been any consideration on how this makes
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True; if we go after
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I can imagine someone using a reverse iterator over fast_iterator wouldn't be too happy about this. |
||
|
|
||
| static constexpr std::uint32_t block = static_cast<std::uint32_t>( | ||
| (std::min<std::size_t>)(128, (std::max<std::size_t>)(1, std::size_t{ 2048 } / sizeof(value_type)))); | ||
|
|
||
| struct no_buffer {}; | ||
| using buffer_type = std::conditional_t<can_batch, std::array<value_type, block>, no_buffer>; | ||
|
|
||
| reference fetch(std::uint32_t const index) const | ||
| { | ||
| if constexpr (can_batch) | ||
| { | ||
| if (index < m_buffer_base || index >= m_buffer_base + m_buffer_size) | ||
| { | ||
| m_buffer_base = index; | ||
| m_buffer_size = m_collection->GetMany(index, m_buffer); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to reorder these assignments. Otherwise, if GetMany throws E_BOUNDS, we will propagate the exception but we've already updated the m_buffer_base, so the next query for the same out-of-bounds element will succeed and return a cached element from some earlier batch. |
||
| } | ||
|
|
||
| if (index < m_buffer_base + m_buffer_size) | ||
| { | ||
| return m_buffer[index - m_buffer_base]; | ||
| } | ||
| } | ||
|
|
||
| return m_collection->GetAt(index); | ||
| } | ||
|
|
||
| T const* m_collection = nullptr; | ||
| std::uint32_t m_index = 0; | ||
| mutable std::uint32_t m_buffer_base = 0; | ||
| mutable std::uint32_t m_buffer_size = 0; | ||
| mutable buffer_type m_buffer{}; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ``[[no_unique_address]] |
||
| }; | ||
|
|
||
| template <typename T> | ||
|
|
@@ -141,23 +208,115 @@ WINRT_EXPORT namespace winrt::impl | |
| static constexpr bool value = get_value<T>(0); | ||
| }; | ||
|
|
||
| template <typename T, std::enable_if_t<!has_GetAt<T>::value, int> = 0> | ||
| auto get_begin_iterator(T const& collection) -> decltype(collection.First()) | ||
| // Forward iterator that batches an IIterator via GetMany into a small buffer and yields from | ||
| // it, so range-for over a collection that lacks GetAt crosses the ABI once per block instead of | ||
| // once per element (Current/MoveNext). Single-pass, matching IIterator's semantics. Only used | ||
| // when the iterator exposes GetMany and its element type is default-constructible. | ||
| template <typename Iterator> | ||
| struct buffered_iterator | ||
| { | ||
| auto result = collection.First(); | ||
| using value_type = decltype(std::declval<Iterator>().Current()); | ||
| using iterator_category = std::input_iterator_tag; | ||
| using difference_type = std::ptrdiff_t; | ||
| using pointer = value_type const*; | ||
| using reference = value_type const&; | ||
|
|
||
| static constexpr bool can_batch = std::is_default_constructible_v<value_type> && has_GetMany_iter<Iterator>::value; | ||
|
|
||
| static constexpr std::uint32_t buffer_capacity = static_cast<std::uint32_t>( | ||
| (std::min<std::size_t>)(128, (std::max<std::size_t>)(1, std::size_t{ 2048 } / sizeof(value_type)))); | ||
|
|
||
| buffered_iterator() noexcept = default; | ||
|
|
||
| explicit buffered_iterator(Iterator iterator) : m_iterator(std::move(iterator)) | ||
| { | ||
| fill(); | ||
| } | ||
|
|
||
| reference operator*() const noexcept | ||
| { | ||
| return m_buffer[m_index]; | ||
| } | ||
|
|
||
| pointer operator->() const noexcept | ||
| { | ||
| return std::addressof(m_buffer[m_index]); | ||
| } | ||
|
|
||
| buffered_iterator& operator++() | ||
| { | ||
| if (++m_index == m_size) | ||
| { | ||
| fill(); | ||
| } | ||
|
|
||
| return *this; | ||
| } | ||
|
|
||
| buffered_iterator operator++(int) | ||
| { | ||
| auto previous = *this; | ||
| ++*this; | ||
| return previous; | ||
| } | ||
|
|
||
| if (!result.HasCurrent()) | ||
| bool operator==(buffered_iterator const& other) const noexcept | ||
| { | ||
| return {}; | ||
| return (m_size == 0) && (other.m_size == 0); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does lead to the weird scenario where a non-end iterator is not equal to itself! That might break some algorithms? Maybe return m_size == other.m_size;? Alternatively, have fill set m_iterator=null if the size is zero, and then this becomes m_iterator==other.m_iterator, which matches the behavior of the old iterator. |
||
| } | ||
|
|
||
| return result; | ||
| bool operator!=(buffered_iterator const& other) const noexcept | ||
| { | ||
| return !(*this == other); | ||
| } | ||
|
|
||
| private: | ||
|
|
||
| void fill() | ||
| { | ||
| m_index = 0; | ||
| m_size = m_iterator ? m_iterator.GetMany(m_buffer) : 0; | ||
| } | ||
|
|
||
| Iterator m_iterator{ nullptr }; | ||
| std::array<value_type, buffer_capacity> m_buffer; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder what algorithms we will tank now that iterator assignment is not cheap. |
||
| std::uint32_t m_size{ 0 }; | ||
| std::uint32_t m_index{ 0 }; | ||
| }; | ||
|
|
||
| template <typename T, std::enable_if_t<!has_GetAt<T>::value, int> = 0> | ||
| auto get_begin_iterator(T const& collection) | ||
| { | ||
| using iterator_type = decltype(collection.First()); | ||
| if constexpr (buffered_iterator<iterator_type>::can_batch) | ||
| { | ||
| return buffered_iterator<iterator_type>{ collection.First() }; | ||
| } | ||
| else | ||
| { | ||
| auto result = collection.First(); | ||
|
|
||
| if (!result.HasCurrent()) | ||
| { | ||
| return iterator_type{}; | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
| } | ||
|
|
||
| template <typename T, std::enable_if_t<!has_GetAt<T>::value, int> = 0> | ||
| auto get_end_iterator([[maybe_unused]] T const& collection) noexcept -> decltype(collection.First()) | ||
| auto get_end_iterator([[maybe_unused]] T const& collection) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where did noexcept go? |
||
| { | ||
| return {}; | ||
| using iterator_type = decltype(collection.First()); | ||
| if constexpr (buffered_iterator<iterator_type>::can_batch) | ||
| { | ||
| return buffered_iterator<iterator_type>{}; | ||
| } | ||
| else | ||
| { | ||
| return iterator_type{}; | ||
| } | ||
| } | ||
|
|
||
| template <typename T, std::enable_if_t<has_GetAt<T>::value, int> = 0> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this go in
wil? There has been a prior desire to move the coroutine/etc stuff to wil and keep cppwinrt focused on purely projection, is this still true?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. No particular reason to have it here.