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

Track last index in parameter marshalling #87846

Closed
wants to merge 14 commits into from

Conversation

jtschuster
Copy link
Member

Tracks the last index marshalled in the interop generators to ensure we don't cleanup any unmarshalled elements.

To determine if the lastElementMarshalled variable is used, we need to generate the cleanup stage, which can be expensive. We also could generate _ = __param_lastIndexMarshalled if there is no cleanup so that there is no warning for an unused variable, but we don't need to generate the cleanup stage twice.

We'll need more testing around the ComInterfaceGenerator, but I would like to get this into preview 6 for LibraryImportGenerator.

@ghost
Copy link

ghost commented Jun 20, 2023

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

Tracks the last index marshalled in the interop generators to ensure we don't cleanup any unmarshalled elements.

To determine if the lastElementMarshalled variable is used, we need to generate the cleanup stage, which can be expensive. We also could generate _ = __param_lastIndexMarshalled if there is no cleanup so that there is no warning for an unused variable, but we don't need to generate the cleanup stage twice.

We'll need more testing around the ComInterfaceGenerator, but I would like to get this into preview 6 for LibraryImportGenerator.

Author: jtschuster
Assignees: -
Labels:

area-System.Runtime.InteropServices

Milestone: -

Copy link
Member

@jkoritzinsky jkoritzinsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as we don't generate invalid code for GeneratedComInterface, I'm supportive of getting this in for P6

@jkoritzinsky
Copy link
Member

It looks like the unit tests all timed out in CI, but only for the LibraryImportGenerator unit tests. Very odd.

@jtschuster
Copy link
Member Author

Looks like this probably does hurt perf significantly enough to cause some timeouts. I made #88060 as an alternative.

@jtschuster
Copy link
Member Author

#88060 was accepted instead of this one

@jtschuster jtschuster closed this Jul 5, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants