Skip to content

Perf: Eliminate audit and history entry allocations when auditor/store is disabled - #91

Merged
rian-be merged 1 commit into
mainfrom
develop
Jul 27, 2026
Merged

Perf: Eliminate audit and history entry allocations when auditor/store is disabled#91
rian-be merged 1 commit into
mainfrom
develop

Conversation

@rian-be

@rian-be rian-be commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Problem Statement

Every mutation execution allocated MutationAuditEntry and MutationHistoryEntry objects unconditionally, even when the registered IMutationAuditor and IMutationHistoryStore implementations discarded them immediately. The factory ran first, allocated, then passed the result to a consumer that ignored it pure waste on every call.

Additionally, CreateHistoryEntry called result.SideEffects.ToList() on every commit mutation, allocating redundant defensive copy of an already-read-only collection.


Solution Overview

1. Default Interface Method: IsEnabled

public interface IMutationAuditor
{
    bool IsEnabled => true;
    Task AuditAsync(MutationAuditEntry entry);
    // ...
}
  • Additive default interface property no breaking changes
  • Existing implementors use default (true), no source changes required
  • NoOp implementations override to false

2. Guard OutcomeProcessor Before Factory Calls

private async Task AuditSuccessAsync(...)
{
    if (!_auditor.IsEnabled)
        return;                    // skip factory entirely
    var entry = MutationAuditEntryFactory.CreateSuccess(...);
    await _auditor.AuditAsync(entry);
}
  • Check happens before factory is called
  • Eliminates MutationAuditEntry, MutationHistoryEntry, and all intermediate allocations when auditor/store is disabled
  • Same pattern for AuditFailureAsync and StoreInHistoryAsync

3. Remove Redundant Defensive Copy

// Before: SideEffects = result.SideEffects.ToList()
// After:  SideEffects = result.SideEffects
  • MutationHistoryEntry.SideEffects already accepts IReadOnlyList<SideEffect>, so no additional copy is required
  • Saves one List<SideEffect> allocation per commit mutation

Performance Impact

Benchmark Results
Benchmark Before After Delta
NoDiagnostics_Baseline 2.237 us 1.430 us -36.1%
AuditHistory_Enabled 4.974 us 3.660 us -26.4%
CombinedInterceptionAndDiagnostics_Enabled 5.653 us / 4.01 KB 3.966 us / 3.52 KB -29.8% / -12.2%
Optimized Path Analysis
Path Before After
NoOpAuditor + NoOpHistoryStore Audit + History entry allocated, then discarded Factory skipped entirely 0 bytes allocated for audit/history entry creation
InMemoryAuditor + InMemoryHistoryStore Entry allocated + stored Same (consumer needs data)
Any path with side effects .ToList() defensive copy Direct reference, no copy

Key Design Decisions

Why default interface method instead of separate `INoOpAuditor` interface?

Every auditor can explicitly declare whether it consumes audit entries. A separate marker interface would require runtime type checks (is INoOpAuditor) at the call site and couple the runtime to specific no op implementations. A default interface member keeps the capability part of the existing abstraction and preserves compatibility with existing implementations.

Why check in OutcomeProcessor instead of the factory?

The factory job is to create entries. Adding a bool create parameter would leak infrastructure concerns into the factory. Checking at the call site (OutcomeProcessor) keeps the factory pure and lets the caller decide the same pattern used throughout the pipeline for policy evaluation and interception.

Why not pass raw data to the auditor and let it create the entry?

That would require new method on IMutationAuditor (breaking change) and couple the audit entry schema to the interface. The current approach keeps the existing factory and audit entry schema while avoiding any breaking API changes.

Why not remove factory calls at the engine level?

The engine doesn't know which auditor/store implementation is registered. Only the OutcomeProcessor, which holds the concrete references, can make the decision. Centralizing the check there avoids leaking implementation awareness upward.


Files Changed

IMutationAuditor.cs

  • Added bool IsEnabled => true; — default interface method

IMutationHistoryStore.cs

  • Added bool IsEnabled => true; — default interface method

MutationExecutionOutcomeProcessor.cs

  • Guarded AuditSuccessAsync/AuditFailureAsync with if (!_auditor.IsEnabled) return;
  • Guarded StoreInHistoryAsync with if (!_historyStore.IsEnabled) return;

MutationAuditEntryFactory.cs

  • Replaced result.SideEffects.ToList() with result.SideEffects

NoOpAuditor.cs / NoOpHistoryStore.cs

  • Added bool IsEnabled => false; override

Migration

Note

No migration steps required. IsEnabled is additive with default true. Existing IMutationAuditor and IMutationHistoryStore implementations continue to work without modification. Implementations that do not consume entries (such as NoOpAuditor) can override the property to false to enable the allocation free fast path.

@github-actions github-actions Bot added performance Performance improvements or regressions abstractions Public abstractions and contracts runtime Runtime implementation and execution flow and removed performance Performance improvements or regressions labels Jul 27, 2026
@github-actions github-actions Bot added the performance Performance improvements or regressions label Jul 27, 2026
@rian-be rian-be changed the title [Perf]: Eliminate audit and history entry allocations when auditor/store is disabled Perf: Eliminate audit and history entry allocations when auditor/store is disabled Jul 27, 2026
@rian-be
rian-be merged commit 9b1cec8 into main Jul 27, 2026
38 of 42 checks passed
@rian-be rian-be linked an issue Jul 27, 2026 that may be closed by this pull request
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

abstractions Public abstractions and contracts performance Performance improvements or regressions runtime Runtime implementation and execution flow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Perf]: Eliminate audit and history entry allocations when auditor/store is disabled

1 participant