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

Benchmark shall loop correctly #7781

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Sep 11, 2023

Pull Request Description

Many ListBenchmarks were wrong. They were looping to sum instead of sum_xyz. Expect regressions, but more truthful results.

Checklist

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

  • All code follows the
    Java,
  • All code has been tested:

@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Sep 11, 2023
@JaroslavTulach JaroslavTulach self-assigned this Sep 11, 2023
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.

Nice observation. I haven't noticed that at all. By the way, this is precisely the use-case, where versioning of benchmarks (mentioned in #5714) would be beneficial.

@JaroslavTulach
Copy link
Member Author

Nice observation. I haven't noticed that at all. By the way, this is precisely the use-case, where versioning of benchmarks (mentioned in #5714) would be beneficial.

At the end the slowdown doesn't seem that bad.
ListBenchmarks.pdf
We don't need to rename then as the result seem to be similar.

Maybe I skewed measurements on my local computer earlier today.


sum_multi list (acc:Text|Decimal|Integer|Any) =
case list of
Nil -> acc
Cons x xs -> @Tail_Call sum xs acc+x
Cons x xs -> @Tail_Call sum_multi xs acc+x
Copy link
Member Author

Choose a reason for hiding this comment

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

Bench runs in between 140-150ms per operation.


sum_int list (acc:Integer) =
case list of
Nil -> acc
Cons x xs -> @Tail_Call sum xs acc+x
Cons x xs -> @Tail_Call sum_int xs acc+x
Copy link
Member Author

Choose a reason for hiding this comment

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

Bench runs in between 140-150ms per operation.

@@ -104,17 +104,17 @@ sum_conv list (acc:V) =
sum_any list (acc:Any) =
case list of
Nil -> acc
Cons x xs -> @Tail_Call sum xs acc+x
Cons x xs -> @Tail_Call sum_any xs acc+x
Copy link
Member Author

Choose a reason for hiding this comment

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

Bench runs in between 140-150ms per operation.

@JaroslavTulach
Copy link
Member Author

ListBenchmarks_mapOverList also runs in between 140-150ms. Let's integrate.

@JaroslavTulach JaroslavTulach merged commit a42850f into develop Sep 11, 2023
30 checks passed
@JaroslavTulach JaroslavTulach deleted the wip/jtulach/LoopCorrectlyBenchmark branch September 11, 2023 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

3 participants