Skip to content
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

[Perf] Add IID Lookup Optimization #849

Merged
merged 32 commits into from
Jul 19, 2021
Merged

[Perf] Add IID Lookup Optimization #849

merged 32 commits into from
Jul 19, 2021

Conversation

j0shuams
Copy link
Contributor

@j0shuams j0shuams commented May 20, 2021

This PR adds guid optimization to all assemblies that use cswinrt. Any time an interop assembly would call for a guid (GetIID, say), we replace that call with a look up in a static map from types to guids.

Using .targets, we run the patcher on a target assembly and pass along the references that assembly has, and copy out the patched assembly mid-build so that the one copied to the bin dir must be the patched dll.

The optimizer comes with an "opt out" in case we need to turn it off for any new issues that might arise in it. The opt out is "CsWinRTIIDOptimizerOptOut" and just needs to be set to "true" in the target assembly's project file.

For testing we know that a set of our unittests cover the codepaths used by the optimizer. The async tests and the storage file/folder tests cover the nontrivial code paths. We have also tested that the symbols files written by the optimizer for the patched dll are usable.

There is an addition to the TestComponentCSharp that arose from a bug in a tool used by the optimizer. The optimizer uses Mono.Cecil nuget package, and on version 0.11.3 of Cecil there was a bug writing long class names to pdb files. I added a new runtime class with a long, nonsensical name, and a unit test that would call for an event on this class. This reproduces the error we originally saw with 0.11.3, and verifies that the Cecil fix in 0.11.4 works for us.

Also, there was a version mis-match in the TestWinRT repo that caused local build issues. This was fixed by a PR on TestWinRT, and we reflect that by updating the commit used to checkout TestWinRT in our local .cmd.

Closes #686

@j0shuams j0shuams requested a review from manodasanW May 20, 2021 23:23
@j0shuams j0shuams force-pushed the jlarkin/GuidPatcher branch from a0f0a5d to 52f5c99 Compare May 21, 2021 02:16
@j0shuams j0shuams force-pushed the jlarkin/GuidPatcher branch from 6ce7b4e to d4baf0b Compare May 24, 2021 21:03
@j0shuams j0shuams requested a review from manodasanW May 24, 2021 21:07
@j0shuams j0shuams changed the title [Perf] Add Guid Patcher [Perf] Add IID Lookup Optimization May 24, 2021
@j0shuams j0shuams force-pushed the jlarkin/GuidPatcher branch from b83c384 to 2f3128c Compare May 25, 2021 23:59
@manodasanW
Copy link
Member

Should we and can we run the IID optimizer on WinRT.Runtime?

@j0shuams j0shuams force-pushed the jlarkin/GuidPatcher branch from c8e26cb to ea4431f Compare July 14, 2021 23:01
nuget/NOTICE.txt Outdated Show resolved Hide resolved
src/cswinrt.sln Outdated Show resolved Hide resolved
@j0shuams j0shuams force-pushed the jlarkin/GuidPatcher branch from c1de3fa to db582bf Compare July 15, 2021 15:33
@j0shuams j0shuams merged commit 2f2e3fc into master Jul 19, 2021
@j0shuams j0shuams deleted the jlarkin/GuidPatcher branch July 19, 2021 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance: Improve performance of GUID lookup for types
4 participants