Refactor - #2812 sequence separate performance #2818
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hoc081098
reviewed
Sep 4, 2022
| val bsep = flatMap { gab -> gab.fold({ emptySequence() }, { sequenceOf(it) }) } | ||
| return asep to bsep | ||
| } | ||
| public fun <A, B> Sequence<Either<A, B>>.separateEither(): Pair<Sequence<A>, Sequence<B>> = |
There was a problem hiding this comment.
IMO, the return type should be Pair<List<A>, List<B>>. Because we evaluate this sequence and return sequence is not lazy
There was a problem hiding this comment.
I do agree with that, is it something that I can change in this PR or would it be done along with other breaking changes?
There was a problem hiding this comment.
@hoc081098 that is a great point! We should probably post-pone that breaking change for Arrow 2.0. Shall we create an Arrow 2.0 ticket for the breaking change, and go ahead with these changes?
@Khepu the same improvement I commented on the Iterable.kt PR can probably be applied here too.
There was a problem hiding this comment.
Just looked into how Sequence methods are implemented in the STD. It will be a bit different so as to avoid realizing the entire sequence just to separate it. Most of the implementations included an internal class that implements a custom iterator object. I should be able to have a PoC tomorrow though!
|
Coming back to this PR after some time. Sorry for the inactivity. I think @hoc081098 is correct, and it should actually be deprecated. See my message below from Slack.
This is also confirmed by looking at the documentation of
All that being said, I still think we should merge this PR. Anyone currently using this operator will benefit from the performance fix! And that way @Khepu efforts don't go to waste 👍 |
|
cc\ @hoc081098 WDYT? |
|
@nomisRev I absolutely agree with you 💯 |
nomisRev
approved these changes
Oct 7, 2022
i-walker
approved these changes
Oct 7, 2022
github-merge-queue bot
pushed a commit
to elide-dev/elide
that referenced
this pull request
Jul 23, 2023
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [io.arrow-kt:arrow-fx-coroutines](https://togithub.com/arrow-kt/arrow) | `1.1.3` -> `1.2.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [io.arrow-kt:arrow-optics-ksp-plugin](https://togithub.com/arrow-kt/arrow) | `1.1.3` -> `1.2.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [io.arrow-kt:arrow-optics](https://togithub.com/arrow-kt/arrow) | `1.1.3` -> `1.2.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [io.arrow-kt:arrow-core](https://togithub.com/arrow-kt/arrow) | `1.1.3` -> `1.2.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [io.arrow-kt:arrow-stack](https://togithub.com/arrow-kt/arrow) | `1.1.3` -> `1.2.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### ⚠ Dependency Lookup Warnings ⚠ Warnings were logged while processing this repo. Please check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>arrow-kt/arrow (io.arrow-kt:arrow-fx-coroutines)</summary> ### [`v1.2.0`](https://togithub.com/arrow-kt/arrow/releases/tag/1.2.0) [Compare Source](https://togithub.com/arrow-kt/arrow/compare/1.1.5...1.2.0) ##### What's Changed - Add CNAME file to Dokka output by [@​franciscodr](https://togithub.com/franciscodr) in [arrow-kt/arrow#3029 - Remove legacy site code by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#3030 - Update README by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3033 - Cancel previous PR action on new commit by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#3032 - Remove legacy script files by [@​franciscodr](https://togithub.com/franciscodr) in [arrow-kt/arrow#3034 - Ensure optics type with "data" modifier is a class by [@​DeoTimeTheGithubUser](https://togithub.com/DeoTimeTheGithubUser) in [arrow-kt/arrow#3036 - Update all dependencies (major) by [@​renovate](https://togithub.com/renovate) in [arrow-kt/arrow#3042 - Remove test dependency from Arrow Fx by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#3046 - Bump Kotlin, KSP and coroutines version by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#3047 - fix mapIndexed when collecting multiple times by [@​hoc081098](https://togithub.com/hoc081098) in [arrow-kt/arrow#3056 - Update versions by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#3058 - optics ksp plugin: fixed handling of variance ([#​3057](https://togithub.com/arrow-kt/arrow/issues/3057)) by [@​vladd-g](https://togithub.com/vladd-g) in [arrow-kt/arrow#3060 - Fixes recover inconsistency with raise DSL on types other than Either by [@​yoxjames](https://togithub.com/yoxjames) in [arrow-kt/arrow#3052 - Change NonEmptySet type parameter name from T to A by [@​franciscodr](https://togithub.com/franciscodr) in [arrow-kt/arrow#3062 - Add withError and (Eager)Effect.mapError by [@​kyay10](https://togithub.com/kyay10) in [arrow-kt/arrow#3059 - Update versions of several libraries by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3017 - Add merge builder for raise by [@​kyay10](https://togithub.com/kyay10) in [arrow-kt/arrow#3061 - Update all dependencies by [@​renovate](https://togithub.com/renovate) in [arrow-kt/arrow#3065 - Update dependency gradle to v8.2 by [@​renovate](https://togithub.com/renovate) in [arrow-kt/arrow#3070 - Fix warnings of single-subclass sealed classes by [@​jooohn](https://togithub.com/jooohn) in [arrow-kt/arrow#3067 - KDoc for `Raise#raise`, `Raise#ensure` and `Raise#ensureNotNull` by [@​ILIYANGERMANOV](https://togithub.com/ILIYANGERMANOV) in [arrow-kt/arrow#3038 - Fix text repetition in EffectScope's deprecation message by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3072 - Introduce `NonEmptyCollection` by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3068 - Set up Spotless by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3075 - Update all dependencies by [@​renovate](https://togithub.com/renovate) in [arrow-kt/arrow#3079 - Add option to disable `inline` when using `@optics` by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3078 - Update JS versions in `yarn.lock` by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3084 - Enable Automatic Modules for JVM by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3071 - Serialization module by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3077 - Add missing docs for `Raise` operations by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3082 - Add mapOrAccumulate extension in RaiseAccumulate by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#3086 - Additional tests for `copy` in Optics by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3089 - Apply Gradle Versioning in top project by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3092 - Add missing Versioning plug-in to `arrow-core-retrofit` by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3093 - Update `arrow-gradle-config` to 0.12-rc.4 by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3094 - MemoizedDeepRecursiveFunction by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#3091 - Add reset and barrierAction to CyclicBarrier. by [@​HSAR](https://togithub.com/HSAR) in [arrow-kt/arrow#3055 - \[HOTFIX] Fix main publish by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#3095 ##### New Contributors - [@​DeoTimeTheGithubUser](https://togithub.com/DeoTimeTheGithubUser) made their first contribution in [arrow-kt/arrow#3036 - [@​vladd-g](https://togithub.com/vladd-g) made their first contribution in [arrow-kt/arrow#3060 - [@​yoxjames](https://togithub.com/yoxjames) made their first contribution in [arrow-kt/arrow#3052 - [@​kyay10](https://togithub.com/kyay10) made their first contribution in [arrow-kt/arrow#3059 - [@​jooohn](https://togithub.com/jooohn) made their first contribution in [arrow-kt/arrow#3067 - [@​ILIYANGERMANOV](https://togithub.com/ILIYANGERMANOV) made their first contribution in [arrow-kt/arrow#3038 - [@​HSAR](https://togithub.com/HSAR) made their first contribution in [arrow-kt/arrow#3055 **Full Changelog**: arrow-kt/arrow@1.2.0-RC...1.2.0 ### [`v1.1.5`](https://togithub.com/arrow-kt/arrow/releases/tag/1.1.5) [Compare Source](https://togithub.com/arrow-kt/arrow/compare/1.1.4...1.1.5) #### What's Changed - Remove `test` modules by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#2874 - Mention Arrow 2.0 in README by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#2878 - Prisms for Either by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#2877 - Test 1.8.0 on CI by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#2864 - Implement 'align' using 'buildList' by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#2886 - Improve debugging experience of leaked shift calls by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#2884 - Fix knitCheck & re-add check by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#2887 **Full Changelog**: arrow-kt/arrow@1.1.4...1.1.5 ### [`v1.1.4`](https://togithub.com/arrow-kt/arrow/releases/tag/1.1.4) [Compare Source](https://togithub.com/arrow-kt/arrow/compare/1.1.3...1.1.4) #### What's Changed - \[2743] Migrate internal use of CircuitBreaker double to duration by [@​mjmoore](https://togithub.com/mjmoore) in [arrow-kt/arrow#2748 - Fix typo by [@​valery1707](https://togithub.com/valery1707) in [arrow-kt/arrow#2824 - Make the server disconnect test more general by [@​lukasz-kalnik-gcx](https://togithub.com/lukasz-kalnik-gcx) in [arrow-kt/arrow#2822 - Update NonEmptyList.fromList deprecation to suggest `toOption()` instead by [@​StylianosGakis](https://togithub.com/StylianosGakis) in [arrow-kt/arrow#2832 - Improve Either.getOrHandle() docs by [@​lukasz-kalnik-gcx](https://togithub.com/lukasz-kalnik-gcx) in [arrow-kt/arrow#2833 - Correct `addressStrees` -> `addressStreet` in optics documentation by [@​vikrem](https://togithub.com/vikrem) in [arrow-kt/arrow#2836 - Arrow Fx: deprecate Platform#composeError, never and unit() by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#2837 - Backport Optics KSP plugin NPE on multiplatform fix by [@​cvb941](https://togithub.com/cvb941) in [arrow-kt/arrow#2840 - Refactor - [#​2812](https://togithub.com/arrow-kt/arrow/issues/2812) sequence separate performance by [@​Khepu](https://togithub.com/Khepu) in [arrow-kt/arrow#2818 - Use `super` equals and hashCode overrides for NonEmptyList by [@​RusticFlare](https://togithub.com/RusticFlare) in [arrow-kt/arrow#2825 - Resource API deprecation, and preparation for 2.x.x. & back port improvements by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#2847 - introduce iterable.toNonEmptyListOrNone() by [@​myuwono](https://togithub.com/myuwono) in [arrow-kt/arrow#2843 - \[PROPOSAL] Either API deprecation, and preparation for 2.x.x by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#2830 - Use major versions in GitHub Actions by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#2849 - Weaken Either monoid dependencies to only require a semigroup on left by [@​mjvmroz](https://togithub.com/mjvmroz) in [arrow-kt/arrow#2845 - Do not reuse the name of the file in Optics KSP by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#2850 - Additional deprecations, and backports for JVM by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#2856 - Update various versions by [@​serras](https://togithub.com/serras) in [arrow-kt/arrow#2852 - Add CountDownLatch by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#2854 - Add CyclicBarrier by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#2857 - Temp rollback error handling deprecations until 1.2.x by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#2858 - Setup Kover as test coverage tool by [@​MarkMarkyMarkus](https://togithub.com/MarkMarkyMarkus) in [arrow-kt/arrow#2793 - Fix Monad.either binary combat by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#2867 - Backport: MPP No Trace by [@​nomisRev](https://togithub.com/nomisRev) in [arrow-kt/arrow#2869 #### New Contributors - [@​mjmoore](https://togithub.com/mjmoore) made their first contribution in [arrow-kt/arrow#2748 - [@​valery1707](https://togithub.com/valery1707) made their first contribution in [arrow-kt/arrow#2824 - [@​lukasz-kalnik-gcx](https://togithub.com/lukasz-kalnik-gcx) made their first contribution in [arrow-kt/arrow#2822 - [@​StylianosGakis](https://togithub.com/StylianosGakis) made their first contribution in [arrow-kt/arrow#2832 - [@​vikrem](https://togithub.com/vikrem) made their first contribution in [arrow-kt/arrow#2836 - [@​cvb941](https://togithub.com/cvb941) made their first contribution in [arrow-kt/arrow#2840 - [@​RusticFlare](https://togithub.com/RusticFlare) made their first contribution in [arrow-kt/arrow#2825 - [@​mjvmroz](https://togithub.com/mjvmroz) made their first contribution in [arrow-kt/arrow#2845 - [@​MarkMarkyMarkus](https://togithub.com/MarkMarkyMarkus) made their first contribution in [arrow-kt/arrow#2793 **Full Changelog**: arrow-kt/arrow@1.1.3...1.1.4-rc.3 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/elide-dev/elide). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMS4wIiwidXBkYXRlZEluVmVyIjoiMzYuMTEuMCIsInRhcmdldEJyYW5jaCI6InYzIn0=-->
](https://renovatebot.com){kind=link}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
related to #2812 similar to #2815 and #2814