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

Optimize Sorting: shuffle string chars only once per workID #286

Merged

Conversation

amirylm
Copy link
Collaborator

@amirylm amirylm commented Nov 16, 2023

AUTO-7299

Description

As part of load testing, we found that our sorting doesn’t scale well as it takes too much CPU.
The motivation in this PR is to avoid shuffle the workID string multiple times, by doing it only once.

Changes

  • AddFromStagingHook is using an additional slice to hold the shuffled strings, and sort that slice instead of the original one that requires to shuffle the workID in each sort operation (up to n^2 operations)

Testing

There were no new tests added as the current cases cover this change and ensure we didn't break the function

@cl-sonarqube-production
Copy link

- use SliceStable
- using a single map for the shuffled string
@amirylm amirylm force-pushed the AUTO-7299-optimize-sorting-shuffle-string-chars-only-once branch from 4d227a1 to 0d1ff28 Compare November 28, 2023 15:29
@amirylm amirylm marked this pull request as ready for review November 28, 2023 16:26
Copy link
Collaborator

@infiloop2 infiloop2 left a comment

Choose a reason for hiding this comment

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

nice find, lgtm!
re. fergal's comment on Slice vs SliceStable, I think either should be fine since array has unique items but one might be faster

@amirylm amirylm merged commit a20bc8d into main Nov 29, 2023
5 checks passed
@amirylm amirylm deleted the AUTO-7299-optimize-sorting-shuffle-string-chars-only-once branch November 29, 2023 21:02
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.

None yet

3 participants