-
Notifications
You must be signed in to change notification settings - Fork 1.4k
using virtual fuction instead of reflection #11513
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please measure impact in perfstar?
I've set up a perf star run. It seems that this has a measurable impact for evaluation performance: |
###Part of #11160
Building on top of Eric's IPC pr - his work allowed for other bottlenecks to reveal themselves.
Context
Currently, the ReadFromStream function uses
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.