close
Skip to content

Pool ResponseBufferingStream in HttpLoggingMiddleware#65147

Merged
adityamandaleeka merged 4 commits into
dotnet:mainfrom
adityamandaleeka:perf/idea-001
Feb 3, 2026
Merged

Pool ResponseBufferingStream in HttpLoggingMiddleware#65147
adityamandaleeka merged 4 commits into
dotnet:mainfrom
adityamandaleeka:perf/idea-001

Conversation

@adityamandaleeka
Copy link
Copy Markdown
Member

The ResponseBufferingStream was being instantiated on every request where response body logging is enabled or interceptors are present. This change uses object pooling to reduce those allocations.

- Add parameterless constructor to ResponseBufferingStream for pooling
- Add Initialize() method to set per-request state
- Add ResetForPool() method to clear state when returning to pool
- Create ResponseBufferingStreamPooledObjectPolicy for the pool
- Update HttpLoggingMiddleware to inject and use the pool
- Register ObjectPool<ResponseBufferingStream> in DI

This reduces object allocations per request when response body logging
is enabled or interceptors are present, reducing GC pressure on
high-traffic servers.
Updated HttpLoggingMiddlewareTests to use the new constructor signature:
- Added ObjectPool<ResponseBufferingStream> parameter to CreateMiddleware helper
- Updated Ctor_ThrowsExceptionsWhenNullArgs test to include pool in all cases
- Added new test case for null responseBufferingStreamPool argument
ObjectPool.Create<T>() requires a public parameterless constructor.
ResponseBufferingStream has an internal constructor for pooling, so we
need to use DefaultObjectPoolProvider.Create() with our policy instead.

This fixes the build errors in both the source and test projects.
Copilot AI review requested due to automatic review settings January 20, 2026 22:00
@github-actions github-actions Bot added the area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares label Jan 20, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces object pooling for ResponseBufferingStream to reduce heap allocations when response body logging or interceptors are enabled.

Changes:

  • Added ResponseBufferingStreamPooledObjectPolicy to manage pooled stream lifecycle
  • Modified ResponseBufferingStream to support pooling with parameterless constructor, Initialize(), and ResetForPool() methods
  • Updated HttpLoggingMiddleware to use object pool for getting/returning streams instead of creating/disposing them
  • Registered the stream pool in the DI container
  • Updated all test constructors to include the new pool parameter

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
ResponseBufferingStreamPooledObjectPolicy.cs New pooling policy that creates streams and resets them when returned to pool
ResponseBufferingStream.cs Added pooling support with initialization and reset methods to enable reuse
HttpLoggingMiddleware.cs Replaced stream instantiation/disposal with pool Get/Return pattern
HttpLoggingServicesExtensions.cs Registered ResponseBufferingStream pool as singleton in DI container
HttpLoggingMiddlewareTests.cs Updated test middleware constructors to include new pool parameter

Copy link
Copy Markdown
Member

@DeagleGross DeagleGross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this, Aditya!
I think it would be beneficial to have benchmarks added to see how much do we save; but for such a change probably not needed.

Comment thread src/Middleware/HttpLogging/src/HttpLoggingMiddleware.cs Outdated
Comment thread src/Middleware/HttpLogging/src/ResponseBufferingStream.cs Outdated
@dotnet-policy-service dotnet-policy-service Bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 3, 2026
Copy link
Copy Markdown
Member

@BrennanConroy BrennanConroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: don't need to reset the logger

@adityamandaleeka adityamandaleeka enabled auto-merge (squash) February 3, 2026 21:25
@adityamandaleeka adityamandaleeka merged commit aaa133a into dotnet:main Feb 3, 2026
25 checks passed
@dotnet-policy-service dotnet-policy-service Bot added this to the 11.0-preview1 milestone Feb 3, 2026
@wtgodbe wtgodbe modified the milestones: 11.0-preview1, 11.0-preview2 Feb 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants