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

Remove FindTypeDef api where appropriate. #95297

Closed
wants to merge 10 commits into from

Conversation

davidwrighton
Copy link
Member

@davidwrighton davidwrighton commented Nov 27, 2023

When loading a valuetype, which has a valuetype field referenced via a TypeRef, we use FindTypeDef on the metadata api to find a possible implementing TypeDef. This api is O(N) where N is the number of TypeDefs in the module. Coincidentally, the EEClassHashTable contains all of the data necessary to do this lookup. Use it instead, and see a substantial improvement in the performance of type loading valuetypes which have fields of valuetypes in other modules, when the valuetype is contained in a module with a large number of TypeDefs.

Additionally, there is a case where we do this as part of ExportedType handling. I've factored this change to include that fix as well, while I research whether or not that form of ExportedType is something we still need to support at all. (The logic was originally only needed for Multi-Module assemblies, and constructs which depend on this particular ExportedType behavior are not legal in ECMA 335 metadata, but that doesn't necessarily stop compilers from generating code which functions.)

Also, remove the FindTypeDef api from the metadata reader entirely. Now the only code which needs something like it is in ILDASM, so just re-implement it there.

This branch is additional work on top of #94825 which improves type load performance further in cases where we have many types in a single module.

…class

- Move namespace/name splitting to the Type.GetType code paths
- Move exported type handling into the normal PopulateAvailableClass flow
- Remove unnecessary work done to detect typedef name duplicates. We don't attempt to protect against invaild assemblies anymore
- Unify path for insertion between ExportedType and TypeDef records, also unify the path for nested vs non-nested
- Fix logic which implements inserts into the case insensitive table when dynamically adding entries to the ExportedType table (Previously it didn't work)
- Update the ECMA 335 augments to capture the requirement that nested ExportedTypes must have a higher RID than the enclosing ExportedType
  - This requirement has actually always existed since .NET 1.0, but was never recorded
…is a linear scan of the entire typedef table, and we already have the right hash to make this cheap
- Also make the code more DAC correct (probably not completely correct, but this logic does not appear to actually be used within the DAC)
- Remove all uses in the runtime in favor of using the class hash
- Port the existing code to be on top of the metadata api so that ildasm can continue to function
@ghost ghost closed this Dec 28, 2023
@ghost
Copy link

ghost commented Dec 28, 2023

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 28, 2024
This pull request was closed.
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.

1 participant