Skip to content

perf(serialization): bound retained Arc buffer page caches - #11470

Open
ReubenBond wants to merge 3 commits into
dotnet:mainfrom
ReubenBond:rb-journal-buffer-retention
Open

ReubenBond wants to merge 3 commits into
dotnet:mainfrom
ReubenBond:rb-journal-buffer-retention

Conversation

@ReubenBond

@ReubenBond ReubenBond commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Bounds the process-wide cache of free Arc buffer pages so completed workloads release excess idle backing capacity rather than accumulating it indefinitely.

ArcBufferWriter.MaxRetainedPoolBytes configures the aggregate free-page budget, with a default of 128 MiB. Setting zero releases already-free pages and disables retention; reducing the budget immediately trims published free pages. Concurrent returns reserve byte capacity before publication and recheck the current budget afterward, keeping the settled cache within the limit. Pages up to 1 MiB are eligible for retention; released large backing arrays return to the separately managed ArrayPool<byte>.Shared. Writers and pinned readers keep ownership of active pages throughout their lifetimes. The 128 MiB default provides process-wide reuse headroom and remains configurable; it is a policy choice, not a workload-tuned benchmark result.

The change is limited to three files: the Arc writer/page-pool implementation, its generated public API, and native pool-policy tests. The property's XML API documentation describes its behavior and defaults. Tests cover the exact 128 MiB default and zero budgets, reductions and increases, oversized pages, active slice preservation, late publication after a reduction, stale trim requests after an increase, and concurrent rent/return/configuration changes. The tests declare the actual CI Suite/Provider traits. The additive public property has an officially regenerated API surface and the normal Serialization Release pack passes compatibility validation.

The PR has been narrowed to pooling only, based on main d558f4c58dc125bb83a9ca66091424d29eeeb41e. Optional journal-storage retention, the alpha storage API break and compatibility suppressions, automatic drained-tail trimming, provider adaptations, and their tests/docs are removed from this PR. Existing main journal APIs, formats, and snapshot policy are preserved. This pool bound is independent of durable messaging payload representation and requires no messaging, owned-RPC, or retained-storage prerequisite.

Microsoft Reviewers: Open in CodeFlow

Copilot AI balanced review requested due to automatic review settings October 10, 2026 16:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The low-level concurrent pooling, reference ownership, cancellation, and finalization changes warrant final human validation.

0 open findings

What changed in this PR

Bounds Arc buffer pooling and enables zero-copy retained journal storage with safe ownership across writes, reads, cancellation, and shutdown.

Changes:

  • Adds a configurable 4 MiB page-cache budget with concurrent accounting and trimming.
  • Introduces retained journal-buffer ownership and stable volatile-storage snapshots.
  • Adds extensive ownership, failure, concurrency, and regression tests plus documentation.
File Description
test/​Orleans.Serialization.UnitTests/​ArcBufferPagePoolTests.cs Tests pool budgets and concurrency.
test/​Orleans.Journaling.Tests/​VolatileJournalStorageOwnershipTests.cs Tests volatile-storage ownership.
test/​Orleans.Journaling.Tests/​StateManagerStorageOwnershipTests.cs Tests manager I/O lifetimes.
test/​Orleans.Journaling.Tests/​JournalBufferWriterOwnershipTests.cs Tests oversized-tail trimming.
src/​Orleans.Serialization/​README.md Documents page pooling.
src/​Orleans.Serialization/​Orleans.Serialization.csproj Exposes internals to journaling tests.
src/​Orleans.Serialization/​Buffers/​ArcBufferWriter.cs Implements bounded page pooling.
src/​Orleans.Journaling/​VolatileJournalStorage.cs Adds independently retained storage pages.
src/​Orleans.Journaling/​README.md Documents ownership behavior.
src/​Orleans.Journaling/​JournaledStateManager.cs Uses retained storage and drains shutdown I/O.
src/​Orleans.Journaling/​JournalBufferWriter.cs Releases oversized idle capacity.
src/​Orleans.Journaling/​IRetainedJournalStorage.cs Defines the retained-storage capability.
src/​api/​Orleans.Serialization/​Orleans.Serialization.cs Updates the generated API surface.
docs/​site/​src/​content/​docs/​grains/​journaling/​runtime-behavior.md Documents runtime ownership semantics.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage

Metric Pull request Current main Variance
Lines 83.39% (118,484 / 142,080) 83.37% (118,390 / 142,004) +0.0216 pp
Branches 72.94% (35,202 / 48,260) 72.92% (35,174 / 48,238) +0.0248 pp

Report-only conclusion: improved.

The current-main baseline is commit d558f4c58d and uses the same reviewed coverage matrix.

Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities.

The comparison remains report-only while normal line and branch variance is calibrated.

Coverage details

Copilot AI balanced review requested due to automatic review settings October 10, 2026 19:23
@ReubenBond
ReubenBond force-pushed the rb-journal-buffer-retention branch from ae3ea82 to e4647cc Compare October 10, 2026 19:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The concurrency-sensitive ownership and shutdown changes warrant final human validation despite extensive tests.

0 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 10, 2026 21:49
@ReubenBond ReubenBond changed the title perf(journaling): retain owned buffers with bounded page caches perf(journaling)!: retain storage buffers with bounded page caches Oct 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Low-level concurrent pooling, reference ownership, finalization, and breaking storage contracts warrant final human validation.

0 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 10, 2026 23:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It changes public ownership contracts and concurrent memory lifetimes across the manager and four storage providers, with updated aggregate coverage still pending.

0 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 10, 2026 23:51
@ReubenBond
ReubenBond force-pushed the rb-journal-buffer-retention branch from 0c667b0 to fcecd42 Compare October 10, 2026 23:51
@ReubenBond ReubenBond changed the title perf(journaling)!: retain storage buffers with bounded page caches perf(serialization): bound retained Arc buffer page caches Oct 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation preserves ownership and accounting invariants with comprehensive boundary and concurrency coverage.

0 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 11, 2026 00:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The retention accounting, concurrency handling, API documentation, generated surface, and behavioral tests are consistent and complete.

0 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 11, 2026 00:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The lock-free accounting and process-wide memory ownership changes warrant final human validation despite comprehensive tests.

0 open findings

🧠 Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants