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

=core Make use of statefulMap instead of statefulMapConcat. #311

Merged
merged 1 commit into from
Dec 24, 2023
Merged

=core Make use of statefulMap instead of statefulMapConcat. #311

merged 1 commit into from
Dec 24, 2023

Conversation

He-Pin
Copy link
Member

@He-Pin He-Pin commented Aug 30, 2023 •
edited
Loading

Motivation:
Use the statefulMap instead when concating is not needed.

[info] Benchmark                                                Mode  Cnt         Score         Error  Units
[info] MessageToFrameRendererBenchmark.benchNewStreamedFrames  thrpt   10  25333318.747 锟斤拷 1293108.754  ops/s
[info] MessageToFrameRendererBenchmark.benchOldStreamedFrames  thrpt   10  20352150.287 锟斤拷 1563364.567  ops/s

@He-Pin He-Pin requested a review from pjfanning August 30, 2023 15:11
@He-Pin He-Pin requested a review from pjfanning August 31, 2023 07:15
@pjfanning pjfanning mentioned this pull request Aug 31, 2023
Closed
pjfanning
pjfanning requested changes Aug 31, 2023
View reviewed changes
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.

could you have a look at #312 ? I'm going to block this until we know if there is a mima failure

@He-Pin He-Pin force-pushed the statefulMap branch 2 times, most recently from 686275e to 0405ca9 Compare September 1, 2023 14:23
@He-Pin
Copy link
Member Author

He-Pin commented Dec 24, 2023

@jrudolph @pjfanning @mdedetrich ping~

@pjfanning
Copy link
Contributor

@He-Pin can you rebase this PR - it's quite old

pjfanning
pjfanning approved these changes Dec 24, 2023
View reviewed changes
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 merged commit 4216e66 into apache:main Dec 24, 2023
10 checks passed
@He-Pin He-Pin deleted the statefulMap branch December 24, 2023 09:14
@pjfanning pjfanning mentioned this pull request Jan 3, 2024
Closed
@He-Pin He-Pin linked an issue Jan 30, 2024 that may be closed by this pull request
Replace statefulMapConcat with statefulMap + mapConcat in codebase #395
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pjfanning pjfanning pjfanning approved these changes

@mdedetrich mdedetrich Awaiting requested review from mdedetrich

@jrudolph jrudolph Awaiting requested review from jrudolph

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace statefulMapConcat with statefulMap + mapConcat in codebase
3 participants
@He-Pin @pjfanning @jrudolph