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

EnsoMultiValue.firstDispatch to speed benchmarks up #11975

Merged
merged 31 commits into from
Jan 10, 2025

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Jan 5, 2025

Pull Request Description

Even more speed up for benchmarks introduced by #11846. Avoids cloning of array in reorderOnly situation at the expense of additional int field in each EnsoMultiValue object.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

JaroslavTulach and others added 27 commits December 19, 2024 08:39
@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Jan 5, 2025
@JaroslavTulach JaroslavTulach self-assigned this Jan 5, 2025
@JaroslavTulach JaroslavTulach requested a review from 4e6 as a code owner January 5, 2025 05:44
@JaroslavTulach JaroslavTulach marked this pull request as draft January 5, 2025 05:44
return newNode.newValue(copyTypes, 1, mv.values);
} else {
copyTypes = copyTypes.clone();
var copyValues = mv.values.clone();
Copy link
Member Author

Choose a reason for hiding this comment

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

The idea behind this PR's speed up is to avoid mv.values.clone() when we only need to select new single pivot value. Instead we give each EnsoMultiValue a firstDispatch index into the array that identifies the location of the currently selected type. Then ths reorderOnly operation can share the same mv.values array with the new EnsoMultiValue.

@JaroslavTulach JaroslavTulach added the CI: Keep up to date Automatically update this PR to the latest develop. label Jan 7, 2025
@JaroslavTulach JaroslavTulach added CI: Clean build required CI runners will be cleaned before and after this PR is built. and removed CI: Keep up to date Automatically update this PR to the latest develop. labels Jan 7, 2025
@JaroslavTulach JaroslavTulach marked this pull request as ready for review January 8, 2025 05:53
@JaroslavTulach
Copy link
Member Author

Benchmarks results. Twice as fast:

sumOverComplexAndFloat5

Twice as fast:
sumOverFloatAndComplex6

Remains the same:
sumOverComplexAndFloatRecastedToFloat3

Remains the same:
obrazek

Copy link
Member

@Akirathan Akirathan left a comment

Choose a reason for hiding this comment

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

I like the speedup and the number of asserts in EnsoMultiValue.

@JaroslavTulach JaroslavTulach merged commit 7d9d69b into develop Jan 10, 2025
49 of 50 checks passed
@JaroslavTulach JaroslavTulach deleted the wip/jtulach/FirstDispatch11846 branch January 10, 2025 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants