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

Optimise _IdentityFn #7862

Merged
merged 1 commit into from
Feb 25, 2023
Merged

Conversation

guizmaii
Copy link
Member

@guizmaii guizmaii commented Feb 24, 2023

Here's how I see this (and I can be wrong):
The probability that someone using ZIO doesn't use this _IdentityFn lazy val is IMO close to 0. Each time you flatten a ZIO, you probably use it.
AFAIR, using a lazy val over a val adds several instructions in the bytecode to make the lazy mechanism work.

Because this probably is low, the memory impact of this val not being lazy is extremelly low, and the additional bytecode instructions needed for the lazy mechanism, I think we can just remove the lazy part

WDYT?

Here's how I see this (and I can be wrong):
The probability that someone using ZIO doesn't use this `_IdentityFn` lazy val is IMO close to 0.
Each time you flatten a ZIO, you probably use it.
AFAIR, using a `lazy val` over a `val` adds several instructions in the bytecode to make the `lazy` mechanism work.

Because, this probably is low, the memory impact of this `val` not being lazy is extremelly low and the additional bytecode instructions needed for the `lazy` mechanism, I think we can just remove the `lazy` part

WDYT?
@yoohaemin
Copy link
Contributor

yoohaemin commented Feb 24, 2023

I'm curious to hear why this was lazy in the first place. I certainly don't usually do lazy when following this pattern of saving allocations.

@jdegoes
Copy link
Member

jdegoes commented Feb 25, 2023

It was probably lazy to avoid having to think about circular references and order of declaration, which Scala is very sensitive to. If it works with lazy, however, let's get it merged! :shipit:

@adamgfraser adamgfraser merged commit 2d65382 into zio:series/2.x Feb 25, 2023
@guizmaii guizmaii deleted the optimize_IdentityFn branch February 26, 2023 04:46
guizmaii added a commit to conduktor/zio-kafka that referenced this pull request Feb 28, 2023
guizmaii added a commit to conduktor/zio-kafka that referenced this pull request Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants