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

Zip and combine performance #2308

Merged
merged 15 commits into from
Oct 20, 2020
Merged

Zip and combine performance #2308

merged 15 commits into from
Oct 20, 2020

Conversation

qwwdfsad
Copy link
Collaborator

No description provided.

@qwwdfsad
[WIP] Improve performance of comine(Iterable<Flow>) by reusing lambda…
f63052e 
…s for onReceive

Addresses #2296
@qwwdfsad
[WIP] optimize performance of Zip by 40%
ec9d084
@qwwdfsad
Optimize Flow.combine
bb167b3 
    * Get rid of two code paths
    * Get rid of accidental O(N^2) where N is the number of flows
    * Get rid of select that hits performance hard

Fixes #2296
@qwwdfsad qwwdfsad requested a review from elizarov October 15, 2020 17:35
@qwwdfsad
~combine rework
fc58f95 
    * Make it completely fair, tho backwards incompatible
    * Optimize it here and there
    * Add more tests
@qwwdfsad
~optimize shouldSuspend away
37d17fe
@qwwdfsad
~indent
ee57def
@qwwdfsad
~update jmh
c2d0707
@qwwdfsad
~more small optimizations
5f01641 
    * Do not allocate array on each transform call in combine, do it only for combineTransform
    * Simplify zip operator even further
    * Get rid of crossinline in combine to fix weird Android crashes

Fixes #1743
Fixes #1683
@qwwdfsad qwwdfsad requested a review from elizarov October 19, 2020 15:09
@qwwdfsad
Fix stack walking in Flow
0d1658a 
    * withContextUndispatched properly wraps completion into CoroutineStackFrame
    * SafeCollector that mimics Continuation properly implements and updates CoroutineStackFrame
elizarov
elizarov approved these changes Oct 19, 2020
View reviewed changes
Copy link
Contributor

@elizarov elizarov left a comment

Choose a reason for hiding this comment

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

🚀 Great!
(however, LocalAtomicRef can be removed)

kotlinx-coroutines-core/common/src/internal/LocalAtomics.common.kt Outdated Show resolved Hide resolved
@qwwdfsad qwwdfsad merged commit 9587590 into develop Oct 20, 2020
@qwwdfsad qwwdfsad deleted the combine-perf branch October 20, 2020 09:40
@alexvanyo alexvanyo mentioned this pull request Nov 30, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@denis-bezrukov denis-bezrukov denis-bezrukov left review comments

@LouisCAD LouisCAD LouisCAD left review comments

@elizarov elizarov elizarov approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@qwwdfsad @elizarov @LouisCAD @denis-bezrukov