Reusing StringBuilder with Object Pooling #965
Conversation
# Conflicts: # QuickFIXn/Message/Message.cs
There was a problem hiding this comment.
@vyourtchenko ran a perf test, and we verified that this indeed offers performance gain, so that's nice.
My opinion is split on whether to add Microsoft.Extensions.ObjectPool to this project.
- Pro: rely on the expertise of MS and get updates
- Con: It would be QF/n's first outside dependency
Are ObjectPool and DefaultObjectPool adapted/copied from existing M.E.ObjectPool classes?
|
|
||
| public void Dispose() | ||
| { | ||
| if (Builder.Length > 64 * 1024) |
There was a problem hiding this comment.
Why not clear a large StringBuilder?
There was a problem hiding this comment.
Why not clear a large StringBuilder?
Good question.
The idea here is to avoid returning unusually large StringBuilder instances to the pool. If we were to clear and return them, they would retain their large internal buffer, which could lead to excessive memory usage and potential LOH (Large Object Heap) allocations over time — especially if this happens frequently.
By skipping Return for builders over 64KB, we allow them to be garbage collected instead. This keeps the pool "lean", favoring smaller buffers that match typical usage patterns.
QuickFIXn/Message/FieldMap.cs
Outdated
| @@ -582,7 +583,8 @@ public int CalculateLength() | |||
| /// <returns></returns> | |||
| public virtual string CalculateString() | |||
| { | |||
| return CalculateString(new StringBuilder(), FieldOrder); | |||
| using PooledStringBuilder pooledSb = new PooledStringBuilder(); | |||
There was a problem hiding this comment.
CalculateString gets called very often. Is there an unnecessary perf loss by creating a PooledStringBuilder every time this is called?
Thinking further.... would it be better to have a static singleton instance of PSB, instead of creating one (and disposing it) in every function call that uses it?
There was a problem hiding this comment.
CalculateString gets called very often. Is there an unnecessary perf loss by creating a PooledStringBuilder every time this is called?
Thinking further.... would it be better to have a static singleton instance of PSB, instead of creating one (and disposing it) in every function call that uses it?
That's a great observation — CalculateString() is indeed called frequently, and performance is important here.
A key detail here is that PooledStringBuilder is a struct, not a class — so its allocation happens on the stack, not the heap.
That means the wrapper itself is extremely lightweight and incurs no GC pressure.
Internally, it rents a StringBuilder from a shared pool, which avoids unnecessary allocations.
Using a static singleton PooledStringBuilder wouldn't be safe: since StringBuilder is mutable and not thread-safe, reusing a single instance across calls would introduce race conditions and potential data corruption.
Even with locking, the contention could outweigh any perceived benefit.
The current pattern strikes a good balance: fast, thread-safe, low-GC, and isolated across invocations.
Nope, they’re not copied. |
# Conflicts: # QuickFIXn/Message/FieldMap.cs # QuickFIXn/Message/Message.cs
Reuse StringBuilder using an object pool to reduce memory allocation.
I have an additional poll, would people be more inclined to introduce the Microsoft.Extensions.ObjectPool package?