Use type system entities instead of strings for node identity#35918
Merged
nattress merged 4 commits intodotnet:masterfrom May 7, 2020
Merged
Use type system entities instead of strings for node identity#35918nattress merged 4 commits intodotnet:masterfrom
nattress merged 4 commits intodotnet:masterfrom
Conversation
The calling method for interface dispatches is used to provide imports specific to each call site (actually currently a shared one per calling method). Replace the use of mangled calling method name with its `MethodDesc` object as the key for the interface dispatch cell. This saves ~4% working set measured on a large application. Also, removed mangled name as the key for method-specific readonly data blobs the JIT allocates. This was much more modest since most methods don't need a readonly blob.
Contributor
Author
|
cc @dotnet/crossgen-contrib |
trylek
approved these changes
May 6, 2020
Member
trylek
left a comment
There was a problem hiding this comment.
LGTM, thanks for cleaning this up!
| private ObjectData _data; | ||
|
|
||
| public SettableReadOnlyDataBlob(Utf8String name, ObjectNodeSection section) | ||
| public SettableReadOnlyDataBlob(MethodDesc owningMethod, ObjectNodeSection section) |
Member
There was a problem hiding this comment.
Since this is making the node not general purpose anymore, maybe we can go all the way and remove the section parameter from the constructor and name the node in a way that conveys its purpose (MethodReadOnlyDataNode?).
Member
There was a problem hiding this comment.
Could you rename the file as well? :)
Contributor
Author
There was a problem hiding this comment.
Ugh, VS crashed during refactor. Will do.
Member
|
Since there can only be one of these nodes per compiled method, instead of using a NodeCache, could you simply allocate it directly? |
Contributor
Author
|
We absolutely can 😄 |
davidwrighton
approved these changes
May 6, 2020
MichalStrehovsky
approved these changes
May 7, 2020
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
The calling method for interface dispatches is used to provide imports specific to each call site (actually currently a shared one per calling method). Replace the use of mangled calling method name with its
MethodDescobject as the key for the interface dispatch cell.This saves ~4% compiler working set measured on a large application.
Also, removed mangled name as the key for method-specific readonly data blobs the JIT allocates. This was much more modest since most methods don't need a readonly blob.
With these changes, we shouldn't hit the name mangler at all during compilation unless we opt to generate a map file.