Reduce memory allocations in Listen happy path#408
Conversation
|
Thank you for your contributions Richard, I've reviewed this PR. The guards in Two things I believe we need to address before merging:
What do you think? |
Log.Verbose("ListenWSClient.ProcessTextMessage", "ENTER");Inside the Having said that, I can remove them if you insist because I'd like to get the larger (entire JSON message) allocations removed. |
|
Good point on the inner guards, you're right that the $"{identifier}: {trace}" concat inside Log.Verbose happens before Serilog's IsEnabled check kicks in, so they genuinely save that allocation for every static-string ENTER / LEAVE call in the hot path. Please keep them as-is. The v1 commit looks great, symmetric coverage with v2. Approving. Thanks for contributing to the SDK. I hope to have a new release for your two PRs in the next day or two. |
Proposed changes
The library's internal logging facade was not designed to defer string interpolation, so strings are often interpolated by the caller even if that log level is not enabled. In a long-lived server environment, especially when Listen/WebSockets are in use, this leads to unnecessary pressure on the garbage collector.
This PR wraps the offending calls to Log in an
IsEnabledcondition so that they don't run unless necessary.I've limited the changes to the Listen "happy path" since that will involve a lot of messages back and forth, but I've left the exception blocks alone.
Types of changes
What types of changes does your code introduce to the community .NET SDK?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.