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

Add another implementation of Min #1612

Merged
merged 2 commits into from
Jun 9, 2022
Merged

Conversation

rnjtranjan
Copy link
Collaborator

No description provided.

@rnjtranjan
Copy link
Collaborator Author

supposed to be faster but benchmark show slower:

minimum (window size 100): OK (0.32s)
4.98 ms ± 259 μs
minimum (window size 1000): OK (0.21s)
13.6 ms ± 818 μs
minimum2 (window size 1000): OK (0.22s)
73.5 ms ± 4.8 ms

@harendra-kumar
Copy link
Member

harendra-kumar commented May 11, 2022

This is a scan benchmark. Can we also try the fold benchmarks? Also try descending order vs ascending order.

We have descending order benchmarks under fold category, we can try the same for postscan as well:

        , benchWith sourceDescendingInt numElements
            "minimum descending (window size 1000)"
(Ring.slidingWindow 1000 Window.minimum)

Also, try for smaller window sizes e.g. 10 and 100 sized window.

@rnjtranjan
Copy link
Collaborator Author

for 10,100 size:
scan
minimum (window size 10): OK (0.15s)
4.92 ms ± 461 μs
minimum (window size 100): OK (0.16s)
4.98 ms ± 414 μs
minimum (window size 1000): OK (0.20s)
13.5 ms ± 722 μs
minimum2 (window size 10): OK (1.14s)
2.21 ms ± 103 μs
minimum2 (window size 100): OK (0.31s)
9.95 ms ± 550 μs
minimum2 (window size 1000): OK (0.22s)
72.9 ms ± 3.0 ms

@rnjtranjan
Copy link
Collaborator Author

Folds are performing better.
fold
minimum (window size 100): OK (0.23s)
3.66 ms ± 170 μs
minimum (window size 1000): OK (0.11s)
8.17 ms ± 679 μs
minimum descending (window size 1000): OK (0.22s)
7.11 ms ± 345 μs
minimum ascending (window size 1000): OK (0.18s)
11.9 ms ± 742 μs
minimum2 (window size 100): OK (0.21s)
818 μs ± 42 μs
minimum2 (window size 1000): OK (0.22s)
852 μs ± 42 μs
minimum2 descending (window size 1000): OK (0.13s)
4.04 ms ± 385 μs
minimum2 ascending (window size 1000): OK (0.13s)
4.13 ms ± 346 μs

Copy link
Collaborator Author

@rnjtranjan rnjtranjan left a comment

Choose a reason for hiding this comment

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

Looks good

@rnjtranjan
Copy link
Collaborator Author

Fixes:#1591

@harendra-kumar harendra-kumar force-pushed the minimum_maximum_fold branch from 9bfce0e to c5e0ee5 Compare June 9, 2022 11:16
@harendra-kumar harendra-kumar merged commit c5e0ee5 into master Jun 9, 2022
@harendra-kumar harendra-kumar deleted the minimum_maximum_fold branch June 9, 2022 18:39
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