Skip to content
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

Optimisations for LoadNextMsgMulti #6448

Merged
merged 2 commits into from
Feb 4, 2025
Merged

Conversation

neilalexander
Copy link
Member

@neilalexander neilalexander commented Feb 4, 2025

This PR optimises LoadNextMsgMulti by doing two things:

  • Avoid large linear scans in firstMatchingMulti by using subject tree intersection when it looks like the FSS size is smaller than the scan range, this should be less expensive (although the threshold for choosing strategy may need further attention);
  • Check the dmap before calling mb.cacheLookup() in either the FSS search or the linear scan, such that we can avoid calling time.Now() a lot when we are tight-looped scanning over a large number of interior deletes.

This PR provides a 2x improvement on the 1000-filter LoadNextMsgMulti unit test and provides a 10x improvement when tested against a store that has extremely sparse messages and 8 consumer filters.

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

@neilalexander neilalexander requested a review from a team as a code owner February 4, 2025 13:23
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.

In general LGTM, but when we hit a dmap entry, before we would update load ts. So maybe we should set a boolean when we match dmap and on exit update that just once?

@neilalexander neilalexander force-pushed the neil/firstmatchingmulti branch 2 times, most recently from 46d73c5 to 2a93e6c Compare February 4, 2025 14:01
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

This avoids a potentially expensive linear walk if it is obvious that the
block FSS contains less subjects than sequences that we would otherwise
have to scan.

Signed-off-by: Neil Twigg <[email protected]>
@neilalexander neilalexander force-pushed the neil/firstmatchingmulti branch from 2a93e6c to 5499017 Compare February 4, 2025 14:03
@derekcollison derekcollison merged commit be97bc9 into main Feb 4, 2025
5 checks passed
@derekcollison derekcollison deleted the neil/firstmatchingmulti branch February 4, 2025 14:34
derekcollison added a commit that referenced this pull request Feb 4, 2025
This updates `generatePerSubjectInfo`, `NumPending` and
`NumPendingMulti` to avoid updating the last load timestamp in a
tight-loop while skipping over a potentially large number of interior
deletes, as this becomes noticeable in the CPU profile.

Similar to one of the changes in #6448.

Signed-off-by: Neil Twigg <[email protected]>
neilalexander added a commit that referenced this pull request Feb 6, 2025
Includes the following:

- #6406
- #6412
- #6408
- #6416
- #6425
- #6424
- #6438
- #6439
- #6446
- #6447
- #6448
- #6449
- #6450
- #6451
- #6452
- #6453
- #6456
- #6458
- #6457
- #6459
- #6460
- #6461

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.

2 participants