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

Multiple Vector.slice over each other slow things down #11945

Merged
merged 6 commits into from
Jan 4, 2025

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Dec 27, 2024

Pull Request Description

Fixes #11859 by unwrapping results of previous Vector.slice before doing another slice.

Checklist

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

  • All code follows the
    Scala,
    Java,
  • Benchmarks measured and improved

@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Dec 27, 2024
@JaroslavTulach JaroslavTulach self-assigned this Dec 27, 2024
@JaroslavTulach
Copy link
Member Author

The current benchmarks results are here:

Benchmark                                    Mode  Cnt    Score    Error  Units
VectorBenchmarks.averageAbstractList         avgt    5    0.053 ±  0.007  ms/op
VectorBenchmarks.averageOverArray            avgt    5    0.001 ±  0.001  ms/op
VectorBenchmarks.averageOverArrayProxy       avgt    5    0.001 ±  0.001  ms/op
VectorBenchmarks.averageOverArrayProxyNew    avgt    5    0.001 ±  0.001  ms/op
VectorBenchmarks.averageOverPolyglotArray    avgt    5    0.001 ±  0.001  ms/op
VectorBenchmarks.averageOverPolyglotVector   avgt    5    0.001 ±  0.001  ms/op
VectorBenchmarks.averageOverSlice            avgt    5    0.001 ±  0.001  ms/op
VectorBenchmarks.averageOverSliceWrapped10   avgt    5    0.073 ±  0.002  ms/op
VectorBenchmarks.averageOverSliceWrapped100  avgt    5  254.710 ± 46.822  ms/op
VectorBenchmarks.averageOverVector           avgt    5    0.001 ±  0.001  ms/op

as can be seen averageOverSlice is as fast as direct access. averageOverSliceWrapped10 is 70x slower and averageOverSliceWrapped100 completely bailed out on the compilation. Time to make things faster.

@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Dec 28, 2024

This benchmark run shows that

  • VectorBenchmarks_averageOverSlice takes 0.0014021641678018011 s
  • VectorBenchmarks_averageOverSliceWrapped10 takes 0.10331784013080388 s

e.g. there is something to speed up. ba0593a helps a lot, but uncovers a problem of "Insert Node" which converts Vector.Long into Vector.Generic. Accessing generic references is indeed slower than accessing long[] behind Vector.Long. New benchmark run has been scheduled.

@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Dec 28, 2024

@4e6, with the here-in proposed change ba0593a your project finishes in a few seconds:

sbt:enso> runEngineDistribution --jvm --run Text_Find_Bench
...
lines remain: 7
lines remain: 3
960
[success] Total time: 11 s, completed Dec 28, 2024, 6:46:13 AM

in this case 11s.

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.

OK. But I don't understand what findStorage should actually do, with this weird infinite loop?

@JaroslavTulach JaroslavTulach added CI: Ready to merge This PR is eligible for automatic merge CI: Keep up to date Automatically update this PR to the latest develop. labels Jan 3, 2025
@mergify mergify bot merged commit b759d75 into develop Jan 4, 2025
47 of 48 checks passed
@mergify mergify bot deleted the wip/jtulach/TextFindSlowsDown11859 branch January 4, 2025 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Keep up to date Automatically update this PR to the latest develop. CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Text.find slows down with the number of invocations
3 participants