[mono] Skip exact interface matches when an interface has variance #103757
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jun 20, 2024
Closed
Open
This was referenced Jun 25, 2024
Closed
Build failure: Static graph-based restore failed with exit code .* but did not log an error.
#103526
Open
This was referenced Jun 28, 2024
Don't dereference random memory if get_virtual_method fails in interp
…kes two-pass hard Try to match the spec more closely by doing two passes per class
lambdageek
reviewed
Jul 9, 2024
| // Is any interface at this level of the type hierarchy variantly compatible with the desired interface? | ||
| // If so, select the first compatible one we find. | ||
| for (i = 0; i < vst_count; i++) { | ||
| if (!mono_class_is_variant_compatible (itf, vst [i], FALSE)) |
There was a problem hiding this comment.
Question: how is this variance search table different from the interfaces_packed table? Do we really need both? if the variance search table is more correct, can we get rid of the interfaces_packed table?
There was a problem hiding this comment.
The variance search table should be smaller. I can try to figure out whether we can get rid of interfaces_packed
There was a problem hiding this comment.
Two examples of what I was thinking in action:
System.IBinaryFloatParseAndFormatInfo`1 vst_size=1 io_count=39
System.Single vst_size=1 io_count=40
Don't build search tables for interfaces
lambdageek
approved these changes
Jul 10, 2024
This was referenced Jul 12, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #103365 appears to be caused by us preferring an 'exact match' for interfaces even when variance means an inexact match may be what we should actually use, because the exact match is on a base class and the inexact one is on a derived class.
This PR changes mono's variance interface lookup to more closely match the spec, by doing two scans per class and walking up the class hierarchy. To reduce the performance cost of this more complex scan, a new 'variance search table' is maintained (created on demand) for each type, which only contains generically variant interfaces so that we're not checking against all interfaces.
I also noticed our list of interfaces implemented by Array was incomplete, so this PR updates it.