Skip to content

Track last index in parameter marshalling#87846

Closed
jtschuster wants to merge 14 commits intodotnet:mainfrom
jtschuster:TrackLastIndex
Closed

Track last index in parameter marshalling#87846
jtschuster wants to merge 14 commits intodotnet:mainfrom
jtschuster:TrackLastIndex

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