Skip to content

[cxx-interop] Avoid extra copy in CxxConvertibleToCollection #63458

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

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

egorzhdan
Copy link
Contributor

This improves the performance of iterating over a C++ container that is automatically conformed to CxxConvertibleToCollection protocol by removing the extra copy of the container.

This also tightens the requirements of CxxConvertibleToCollection by making begin() and end() non-mutating.

This improves the performance of iterating over a C++ container that is automatically conformed to `CxxConvertibleToCollection` protocol by removing the extra copy of the container.

This also tightens the requirements of `CxxConvertibleToCollection` by making `begin()` and `end()` non-mutating.
@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Feb 6, 2023
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

@swift-ci please benchmark

@egorzhdan
Copy link
Contributor Author

Performance (x86_64): -O

Improvement OLD NEW DELTA RATIO
CxxSetU32.to.Array 273.429 155.083 -43.3% 1.76x
CxxSetU32.to.Set 374.2 257.2 -31.3% 1.45x (?)

Performance (x86_64): -Osize

Improvement OLD NEW DELTA RATIO
CxxSetU32.to.Array 276.714 157.0 -43.3% 1.76x
CxxSetU32.to.Set 377.2 260.714 -30.9% 1.45x

Performance (x86_64): -Onone

Improvement OLD NEW DELTA RATIO
CxxSetU32.to.Array 720.667 387.8 -46.2% 1.86x
CxxSetU32.to.Set 825.5 492.5 -40.3% 1.68x

@egorzhdan egorzhdan merged commit dc1b73f into main Feb 6, 2023
@egorzhdan egorzhdan deleted the egorzhdan/cxx-convertible-mutable branch February 6, 2023 21:21
@ravikandhadai
Copy link
Contributor

That's awesome! Do you plan on adding a compiler test as well?

@egorzhdan
Copy link
Contributor Author

Yeap, I'm going to add more compiler tests for this once the hierarchy restructure patch lands (#63469).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants