Skip to content

Conversation

@hamarb123
Copy link
Contributor

These fields being marked readonly causes 25 uses to create a defensive copy in total (since it's generic, it has no such things as readonly members, meaning every access requires a defensive copy). Removing the readonly should resolve these.

@stephentoub
Copy link
Member

Can you highlight specific examples where this is causing meaurably worse performance / worse code generation?

@steveharter steveharter added needs-author-action An issue or pull request that requires more info or actions from the author. tenet-performance Performance related issue labels Apr 17, 2024
@hamarb123
Copy link
Contributor Author

Can you highlight specific examples where this is causing meaurably worse performance / worse code generation?

Looking at the definition of EcmaMethodDecoder : IMethodDecoder, it seems like it'd be 4 pointer sizes. So the longer methods that will not be inlined will copy 32 bytes first before they can call the method (on 64-bit). I can do some proper testing at some point, but I'll probably be unable to do much for approximately the next 1.5 weeks.

@dotnet-policy-service dotnet-policy-service bot removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Apr 17, 2024
@buyaa-n buyaa-n added the needs-author-action An issue or pull request that requires more info or actions from the author. label May 8, 2024
@hamarb123
Copy link
Contributor Author

Can you highlight specific examples where this is causing meaurably worse performance / worse code generation?

This is probably not that bad really, so I'll close it, since the preference is to keep readonly where possible without meaningful performance difference (#101156 (comment)).

@dotnet-policy-service dotnet-policy-service bot removed the needs-author-action An issue or pull request that requires more info or actions from the author. label May 13, 2024
@hamarb123 hamarb123 closed this May 13, 2024
@stephentoub
Copy link
Member

Thanks

@github-actions github-actions bot locked and limited conversation to collaborators Jun 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Reflection community-contribution Indicates that the PR has been added by a community member tenet-performance Performance related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants