Minor stress improvement #57715
Minor stress improvement #57715
Conversation
|
Tagging subscribers to this area: @dotnet/ncl Issue Details
None of this is critical and can be scratched. I'm just pushing changes that I've done for my own testing.
|
|
/azp run runtime-libraries stress-http |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-libraries stress-ssl |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| string s = sb.ToString(); | ||
| sb.Clear(); | ||
| Interlocked.Exchange(ref _cachedStringBuilder, sb); | ||
| await _messagesChannel.Writer.WriteAsync(s, _stopProcessing.Token); | ||
| await _messagesChannel.Writer.WriteAsync(sb.ToString(), _stopProcessing.Token); | ||
| _stringBuilderPool.Return(sb); |
There was a problem hiding this comment.
Does StringBuilderPooledObjectPolicy clear the StringBuilder now?
There was a problem hiding this comment.
Yep, taken from this blog post: https://docs.microsoft.com/en-us/aspnet/core/performance/objectpool?view=aspnetcore-5.0
And confirmed by this:
https://source.dot.net/#Microsoft.Extensions.ObjectPool/StringBuilderPooledObjectPolicy.cs,30f83640ea3a7033,references
| private int _lastLogNumber = 0; | ||
| private FileStream _log; | ||
| private Channel<string> _messagesChannel = Channel.CreateUnbounded<string>(); | ||
| private Task _processMessages; | ||
| private CancellationTokenSource _stopProcessing = new CancellationTokenSource(); | ||
| private const string LogDirectory = "./clientlog"; | ||
| private ObjectPool<StringBuilder> _stringBuilderPool = new DefaultObjectPool<StringBuilder>(new StringBuilderPooledObjectPolicy()); |
There was a problem hiding this comment.
Nit: since we don't really need the abstraction, we can use the specific type & devirtualize the method calls:
| private ObjectPool<StringBuilder> _stringBuilderPool = new DefaultObjectPool<StringBuilder>(new StringBuilderPooledObjectPolicy()); | |
| private DefaultObjectPool<StringBuilder> _stringBuilderPool = new DefaultObjectPool<StringBuilder>(new StringBuilderPooledObjectPolicy()); |
|
Unrelated to the PR, but since we are in Will public override void Dispose()
{
base.Dispose();
_log.Flush();
_stopProcessing.Cancel();
// Try to wait for the task to finish before disposing
_processMessages.Wait(TimeSpan.FromSeconds(30));
_log.Dispose();
} |
Ha ha the |
|
/azp run runtime-libraries stress-http |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Fudge, win docker copy is borked 😢 |
|
/azp run runtime-libraries stress-http |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ManickaP
force-pushed
the
mapichov/stress_improvements
branch
from
84697ff
to
f4dfff7
Compare
ManickaP
force-pushed
the
mapichov/stress_improvements
branch
from
f4dfff7
to
4801fd1
Compare
|
/azp run runtime-libraries stress-http |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-libraries stress-ssl |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ghost
locked as resolved and limited conversation to collaborators
None of this is critical and can be scratched. I'm just pushing changes that I've done for my own testing.