Skip to content

perf: cache Result.Success() singleton#114

Merged
Chris-Wolfgang merged 3 commits into
mainfrom
perf/cached-success-singleton
May 11, 2026
Merged

perf: cache Result.Success() singleton#114
Chris-Wolfgang merged 3 commits into
mainfrom
perf/cached-success-singleton

Conversation

@Chris-Wolfgang
Copy link
Copy Markdown
Owner

Summary

  • Result is immutable, so Success() can return a single shared instance instead of allocating on every call.
  • Eliminates per-call allocations on hot paths (Flatten, Try.Run, consumer code).

Test plan

  • dotnet build -c Release — 0 warnings, 0 errors
  • dotnet test -c Release --framework net8.0 — 97 passed
  • CI green across all TFMs

🤖 Generated with Claude Code

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 optimizes the core Result API by caching a single successful Result instance and returning it from Result.Success() to eliminate repeated allocations on hot paths.

Changes:

  • Added a cached singleton instance for successful Result values.
  • Updated Result.Success() to return the cached instance and documented the caching behavior.

Comment thread src/Wolfgang.TryPattern/Result.cs Outdated
Comment thread src/Wolfgang.TryPattern/Result.cs Outdated
Chris-Wolfgang added a commit that referenced this pull request May 11, 2026
- Rename `SuccessInstance` to `_successInstance` to match the
  project's private-field naming convention (e.g., `_value`).
- Expand XML <remarks> to call out that successful results returned
  from `Success()` are reference-equal, so callers must not use
  identity comparison to distinguish them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 11, 2026 21:24
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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread src/Wolfgang.TryPattern/Result.cs
Chris-Wolfgang and others added 3 commits May 11, 2026 18:02
Result is immutable, so Success() can return a single shared instance
instead of allocating a new Result on every call. Eliminates allocations
on hot paths where Success() is called frequently (Flatten, Try.Run,
and consumer code).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Rename `SuccessInstance` to `_successInstance` to match the
  project's private-field naming convention (e.g., `_value`).
- Expand XML <remarks> to call out that successful results returned
  from `Success()` are reference-equal, so callers must not use
  identity comparison to distinguish them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a test that asserts two consecutive `Result.Success()` calls
return the same reference. This locks in the cached-singleton
optimization introduced in this PR so any future change back to
per-call allocation has to deliberately update the test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 11, 2026 22:02
@Chris-Wolfgang Chris-Wolfgang force-pushed the perf/cached-success-singleton branch from bdc6637 to 7ae8337 Compare May 11, 2026 22:02
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

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

@Chris-Wolfgang Chris-Wolfgang merged commit e050691 into main May 11, 2026
12 checks passed
Chris-Wolfgang added a commit that referenced this pull request May 11, 2026
- Rename `SuccessInstance` to `_successInstance` to match the
  project's private-field naming convention (e.g., `_value`).
- Expand XML <remarks> to call out that successful results returned
  from `Success()` are reference-equal, so callers must not use
  identity comparison to distinguish them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Chris-Wolfgang Chris-Wolfgang deleted the perf/cached-success-singleton branch May 11, 2026 22:23
@Chris-Wolfgang Chris-Wolfgang mentioned this pull request May 18, 2026
4 tasks
Chris-Wolfgang added a commit that referenced this pull request May 19, 2026
Changes since v0.3.1:

- fix: null-element validation in Result.Flatten / AllSucceeded /
  AnyFailed (#113) — previously calling `.Failed` on a null element
  threw NullReferenceException; now throws ArgumentException with the
  index of the offending element. Adds [NotNull] post-condition
  attribute to all three methods plus all Try.Run/RunAsync overloads.
  Polyfills NotNullAttribute for net462 / netstandard2.0.
- perf: Result.Success() now returns a cached singleton instead of
  allocating per call (#114). Behavior is locked in by a test using
  Assert.Same; XML docs warn callers not to rely on reference identity.
- perf: replace LINQ in Flatten / AllSucceeded / AnyFailed with index-
  based for-loops (#116). Flatten is now single-pass with a lazy
  StringBuilder so the common single-failure case has zero extra
  allocation. AllSucceeded / AnyFailed short-circuit on first
  failure, so a trailing null past the decisive element no longer
  throws (documented in <exception> XML and locked in by tests).
- refactor: simplify Result ctor validation (#115) — switch-when
  replaced with two guard-clause ifs. No behavior change; exception
  messages and ParamName preserved.
- chore: test cleanup (#117) — remove redundant inheritance-
  verification tests, fix tabs vs spaces, drop unused pragma.
- chore(deps): Bump Meziantou.Analyzer 3.0.77 → 3.0.85 (#118).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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