Skip to content

Reduce AofAddress struct copy overhead on hot paths#1759

Merged
vazois merged 8 commits into
devfrom
vazois/mlog-perf
May 2, 2026
Merged

Reduce AofAddress struct copy overhead on hot paths#1759
vazois merged 8 commits into
devfrom
vazois/mlog-perf

Conversation

@vazois
Copy link
Copy Markdown
Contributor

@vazois vazois commented May 1, 2026

AofAddress is a 513-byte struct (1 byte + fixed long[64]) that was frequently copied by value on performance-critical paths.

Changes:

  • Add in modifier to all AofAddress instance method parameters (Equals, AnyLesser, AnyGreater, Diff, AggregateDiff, etc.)
  • Add in modifier to AofSyncDriverStore and IClusterProvider method parameters
  • Add scalar accessors (GetTailAddress(int), GetBeginAddress(int), GetPreviousAddress(int), GetReplicationOffset(int)) to avoid full struct copies when only one sublog value is needed
  • Update hot-path callers (ReplicaReplaySession, SafeTruncateAof, GarnetServerNode, ReplicaReplayDriver) to use scalar accessors

vazois and others added 6 commits April 30, 2026 17:50
…path

Add scalar GetTailAddress/GetBeginAddress methods to GarnetLog and use
GetTailAddress in ProcessPrimaryStream instead of copying full struct.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Avoids copying 513-byte struct on every call to Equals, SetValueIf,
MinExchange, AnyLesser, AnyGreater, Diff, AggregateDiff, EqualsAll,
and IsOutOfRange.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ider

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GetPreviousAddress(int) and GetStartAddress(int) avoid reconstructing
the full 513-byte AofAddress struct when only one sublog value is needed.
Updated SafeTruncateAof hot path to use the scalar accessor.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Callers that only need a single sublog offset now use the scalar
accessor instead of copying the 513-byte AofAddress struct.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 1, 2026 03:33
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 aims to reduce performance overhead from frequent by-value copying of the large AofAddress struct on AOF/replication hot paths by switching to in parameters and adding scalar sublog accessors.

Changes:

  • Updates multiple APIs to pass AofAddress by in to avoid by-value copies (e.g., cluster truncate and replication/AOF helpers).
  • Adds scalar accessors (e.g., tail/begin/replication offsets per sublog) and updates hot-path replication callers to use them.
  • Refactors some AOF sync/replication code to read/write per-sublog values without materializing full AofAddress instances.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
libs/server/Cluster/IClusterProvider.cs Updates SafeTruncateAOF signature to take in AofAddress.
libs/server/AOF/GarnetLog.cs Adds GetTailAddress(int) / GetBeginAddress(int) scalar accessors for sublogs.
libs/server/AOF/AofAddress.cs Switches many methods to in parameters; refactors comparisons/diffs; modifies max sublog constant.
libs/cluster/Server/Replication/ReplicationManager.cs Adds GetReplicationOffset(int) scalar accessor to avoid returning full AofAddress.
libs/cluster/Server/Replication/ReplicaOps/AOFReplay/ReplicaReplaySession.cs Updates divergence checks to use scalar tail address accessor.
libs/cluster/Server/Replication/ReplicaOps/AOFReplay/ReplicaReplayDriver.cs Updates offset logging and throttle logic to use scalar accessors.
libs/cluster/Server/Replication/PrimaryOps/AofOperations/AofSyncDriverStore.cs Uses scalar previous address accessor; updates APIs to accept in AofAddress.
libs/cluster/Server/Replication/PrimaryOps/AofOperations/AofSyncDriver.cs Adds scalar sublog accessors (GetPreviousAddress, GetStartAddress).
libs/cluster/Server/Gossip/GarnetServerNode.cs Uses scalar replication offset for CLUSTER NODES reporting.
libs/cluster/Server/ClusterProvider.cs Updates SafeTruncateAOF implementation to match in signature.
Comments suppressed due to low confidence (1)

libs/server/AOF/AofAddress.cs:241

  • Same as above: SetValueIf(in AofAddress value, ...) reads value[i] in a loop. If the indexer is not readonly, this can cause defensive copies / negate the benefit of using in. Prefer direct fixed-buffer reads (or a readonly accessor) for in parameters in hot loops.
        public void SetValueIf(in AofAddress value, long comparand)
        {
            for (var i = 0; i < Length; i++)
            {
                if (addresses[i] == comparand)
                    addresses[i] = value[i];
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread libs/server/AOF/AofAddress.cs
Comment thread libs/server/AOF/AofAddress.cs
- Make indexer getter readonly to prevent defensive copies on 'in' params
- Reference AofAddress.MaxSublogCount in Options.cs validation attribute
- Fix XML docs: 'allocating' -> 'copying' for scalar accessors

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@vazois vazois merged commit eaa5734 into dev May 2, 2026
42 of 45 checks passed
@vazois vazois deleted the vazois/mlog-perf branch May 2, 2026 01:18
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.

3 participants