search

aspnet / BasicMiddleware

[Archived] Basic middleware components for ASP.NET Core. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
169 stars 84 forks source link

add StringBuilder pool #223

Closed david-peden-q2 closed 7 years ago

david-peden-q2 commented 7 years ago

Add pooling for StringBuilder. This PR addresses the perf concerns noted in a couple of the Segment classes. The implementation of PooledStringBuilder is borrowed from http://source.roslyn.codeplex.com/#Microsoft.CodeAnalysis.Workspaces/PooledStringBuilder.cs. The implementation of ObjectPool is inspired by https://msdn.microsoft.com/en-us/library/ff458671%28v=vs.110%29.aspx?f=255&MSPPError=-2147217396 rather than also borrowing http://source.roslyn.codeplex.com/#Microsoft.CodeAnalysis.Workspaces/ObjectPool%25601.cs. Obviously it would be trivial to switch the implementation out.

I decided to add this as a PR because I ran into needing an additional StringBuilder in a PR I'm about to add for adding header support in rewrite rules. Rather than copying and pasting the same perf code in yet another segment, I cleaned it up.

davidfowl commented 7 years ago

Can't make performance fixes without before/after performance numbers 😄

davidpeden3 commented 7 years ago

for sure, so will you guys handle the benchmark for this as you are for https://github.com/aspnet/BasicMiddleware/pull/206?

muratg commented 7 years ago

@davidpeden3 thanks for sending out this PR. It became a little stale, so I'll be closing this now. But if you have any measurements (before/after) that you'd like to open up for discussion, please file a bug and let's keep the communication open for a future PR. Thanks.