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

using virtual fuction instead of reflection #11513

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

SimaTian
Copy link
Member

@SimaTian SimaTian commented Feb 27, 2025

###Part of #11160
Building on top of Eric's IPC pr - his work allowed for other bottlenecks to reveal themselves.

Context

read_from_stream_reflection
read_from_stream_virtual
Currently, the ReadFromStream function uses

ArgsReaderDelegate readerMethod = (ArgsReaderDelegate)CreateDelegateRobust(typeof(ArgsReaderDelegate), _buildEvent, methodInfo);

for a large portion of it's packet deserialization. This is large enough to be seen in the performance profiler as shown above more than 3% of CPU an it's on a critical path.
When I checked, the function was already virtual so the change is minimal. Unfortunately I had to make an allowance to the task host since it wasn't compatible.

Changes Made

Exposed a convenience public endpoint for the CreateFromStream function, that calls the virtual method Create from stream.
Used this endpoint instead of delegate creation.

Testing

As long as nothing breaks, I consider that a win.
I did local profiling.

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.

1 participant