Skip to content

[IMPROVED] Optimise needAck with new SubjectForSeq store function #6990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 18, 2025

Conversation

neilalexander
Copy link
Member

Adds a new SubjectForSeq function to the filestore that copies less and performs fewer allocations compared to LoadMsg. This is then used inside needAck when we don't know what the subject is for the sequence, i.e. when called from checkStateForInterestStream.

There are more improvements that need to be made here but for now this will reduce the amount of garbage created by this process, and by extension, reduce GC pressure and CPU usage on large interest-based streams.

Signed-off-by: Neil Twigg [email protected]

@neilalexander neilalexander requested a review from a team as a code owner June 18, 2025 16:43
Copy link
Member

@MauriceVanVeen MauriceVanVeen left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

@neilalexander neilalexander merged commit f4864cb into main Jun 18, 2025
90 of 92 checks passed
@neilalexander neilalexander deleted the neil/needack branch June 18, 2025 20:08
@wallyqs wallyqs changed the title Optimise needAck with new SubjectForSeq store function [IMPROVED] Optimise needAck with new SubjectForSeq store function Jun 24, 2025
neilalexander added a commit that referenced this pull request Jun 24, 2025
Includes the following:

- #6990
- #6967
- #6995
- #6999
- #7001

Signed-off-by: Neil Twigg <[email protected]>
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