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

perf: Add dedicated collectFirst implementation. #1041

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

He-Pin
Copy link
Member

@He-Pin He-Pin commented Jan 25, 2024

Motivation:
Use dedicated collectFirst implementation

Result:
Better performance.

@He-Pin He-Pin added the t:stream Pekko Streams label Jan 25, 2024
@He-Pin He-Pin added this to the 1.1.0 milestone Jan 25, 2024
Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

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

This new class looks a lot like existing Akka based code. Is it truly new? Otherwise, use the Lightbend and Apache header from the original file.

@He-Pin
Copy link
Member Author

He-Pin commented Jan 25, 2024

Yes it's totally new, I implemented with not looking at any code of the origin Collect and this mechanism is generic and not diverged from lightbend's code, otherwise, every class that extends the GraphStage will add the Lightbend's license header? why should I? I think every collectFirst will implemented this way.

And you will see this kind of code in typelevel/fs2#3379 too and I chated at https://contributors.scala-lang.org/t/can-scala-compiler-do-the-micro-optimization-for-simple-pattern-matching/6493

@He-Pin He-Pin requested a review from pjfanning January 25, 2024 17:21
Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

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

lgtm

@He-Pin He-Pin requested a review from mdedetrich January 26, 2024 04:53
Copy link
Contributor

@mdedetrich mdedetrich left a comment

Choose a reason for hiding this comment

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

lgtm

@pjfanning I can also confirm that if you wanted to write this from scratch it would also look like this. While it may look similar to existing akka code, there isn't much you can do about that because generally all of this code has the same style so if you want to write new functionality and keep the existing style it has a very high chance of looking this way.

@He-Pin
Copy link
Member Author

He-Pin commented Jan 26, 2024

Yes, we can ask anyone to write this code, and anyone's code will result in this style.

@He-Pin He-Pin merged commit 325b463 into apache:main Jan 26, 2024
18 checks passed
@He-Pin He-Pin deleted the collectFirstOpt branch January 26, 2024 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:stream Pekko Streams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants