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.

Sign up for GitHub

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

Jump to bottom

Replacing chain() with .map() #2208

Merged
merged 3 commits into from
Nov 25, 2024
Merged

Replacing chain() with .map() #2208

merged 3 commits into from
Nov 25, 2024

Conversation

RobinTail
Copy link
Owner

@RobinTail RobinTail commented Nov 25, 2024 •
edited
Loading

went through #1498 , but it seems that no reduction was ever needed.
I need to check if there is no performance reduction caused by this change

@RobinTail RobinTail added the refactoring The better way to achieve the same result label Nov 25, 2024
@coveralls-official Coveralls Official
Copy link

coveralls-official bot commented Nov 25, 2024 •
edited
Loading

Coverage Status

coverage: 100.0%. remained the same
when pulling 925d94a on reducer-not-needed
into 5ac65a3 on master.

@RobinTail RobinTail changed the title Removing reducers where no reduction's happening. Replacing chain with .map Nov 25, 2024
@RobinTail RobinTail changed the title Replacing chain with .map Replacing chain() with .map Nov 25, 2024
@RobinTail RobinTail changed the title Replacing chain() with .map Replacing chain() with .flatMap() Nov 25, 2024
@RobinTail
Copy link
Owner Author

RobinTail commented Nov 25, 2024 •
edited
Loading

OK. chain is actually faster

   ✓ Experiment on flat mapping (2) 2056ms
     name               hz     min     max    mean     p75     p99    p995    p999     rme  samples
   · chain    2,141,268.22  0.0003  0.4719  0.0005  0.0005  0.0009  0.0011  0.0022  ±0.78%  1070635   fastest
   · flatMap    174,100.44  0.0046  0.7827  0.0057  0.0058  0.0077  0.0088  0.0264  ±0.51%    87051

@RobinTail RobinTail changed the title Replacing chain() with .flatMap() Replacing chain() with .map() Nov 25, 2024
@RobinTail RobinTail marked this pull request as ready for review November 25, 2024 19:06
RobinTail
RobinTail commented Nov 25, 2024
View reviewed changes
Copy link
Owner Author

@RobinTail RobinTail left a comment

Choose a reason for hiding this comment

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

ok

@RobinTail RobinTail merged commit 3a8fef4 into master Nov 25, 2024
11 checks passed
@RobinTail RobinTail deleted the reducer-not-needed branch November 25, 2024 19:06
@RobinTail RobinTail mentioned this pull request Nov 25, 2024
Merged
RobinTail added a commit that referenced this pull request Nov 26, 2024
@RobinTail
Replacing .flatMap() with chain() (#2209)
5b86956 
Confirmed to be faster in #2208 

with added eslint rule
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
refactoring The better way to achieve the same result
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@RobinTail