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

Implement marking of type hierarchies #4

Merged

Conversation

vitek-karas
Copy link
Collaborator

Implements marking in type hierarchies

The general idea is to have a cache of type->annotation which is populated from MarkStep.MarkType. We then use the cache in Object.GetType to get the annotation. It's also used to get the set of types which are candidates to be marked during the Object.GetType processing.

  • There's no structure in the linker which would be able to quickly enumerate derived types from a base type, or implementations from interface
  • Going over all marked types would be very slow
  • The cache is global, so if there's multiple type hierarchies with annotations they are all "merged" in it, but it should still be a very small subset of all marked types (as we don't expect this feature to be used too broadly)

Due to how interfaces are marked (delayed and in effectively random order), the cache has to store all interfaces - even those without annotations. For non-interface types it only stores annotated types and their hierarchies.

  • Note that the cache will store even unmarked interfaces
  • The cache effectively stores interfaces reachable from all marked types - so generally not a tiny number, but it should not be big either.

Behavior:

  • Annotations on interfaces are applied even if the interface itself will be removed by the linker if it's not used. Not doing this would make the implement very complex (probably, maybe there's some clever idea we can come up with). Anyway this is just overmarking which should not be observable
  • Annotations are applied solely based on the attributes - the actual Object.GetType is just a trigger, it doesn't actually tell which of the base types was used and thus potentially subset the annotations applied. Again, only overmarking which is not observable

Implementation details:

  • Split it into a separate class as it's rather long
  • It's in the middle between MarkStep and ReflectionMethodBodyScanner
    • Not part of MarkStep since logically it belongs more to the DataFlow side of things
    • Not part of ReflectionMethodBodyScanner since it needs to maintain global state (the scanner is short lived)
  • It does have sort of intimate knowledge of the ReflectionMethodBodyScanner but it's not too bad

@vitek-karas vitek-karas changed the title Implement marking Implement marking of type hierarchies Mar 23, 2021
@vitek-karas
Copy link
Collaborator Author

@MichalStrehovsky can you please take a look - it should be close to what we've discussed.

Currently this is done in a private branch - once we have the whole thing working (which is basically done after this PR) we will move it over to main.

/cc @tlakollo @LakshanF

@LakshanF LakshanF merged commit 2f8b0eb into LakshanF:TrimAttrSupportFlow Mar 25, 2021
@MichalStrehovsky
Copy link

@MichalStrehovsky can you please take a look - it should be close to what we've discussed.

Sigh. Notifications for non-microsoft repos go to my personal email address and my personal email is messier than my personal life (so a whole new level of messy). Since it's already merged anyway, I'll take a look at this once this moves to the linker repo.

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.

3 participants