ZLayer.make fails with ClassTooLarge about zio HOT 32 CLOSED

I wouldn't say won't fix, but no giant motivation to fix. Instead of generating identifiers, we could create an untyped array and access different parts of it (e.g.). So there are ways to fix it but it is more complicated.

from zio.

Concatenation does pruning (by "default") because otherwise of surprises:

val clock: ZLayer[Any, Nothing, Clock] = ???

val logging: ZLayer[Any, Nothing, Logging] = ???

clock ++ logging : ZLayer[Any, Nothing, Clock & Logging]

Makes sense, right?

Until you see:

val clock: ZLayer[Any, Nothing, Clock] = ???

val loggingAndStompingClock: ZLayer[Any, Nothing, Logging] = ???

val logging: ZLayer[Any, Nothing, Logging] = loggingAndStompingClock

clock ++ logging : ZLayer[Any, Nothing, Clock & Logging]

The issue is that due to covariance, Scala lets you forget part or all of the output of a layer. Then when you combine them, you look at the types to gain some intuition of where your services are coming from. Which, in the absence of pruning, can be completely wrong and difficult to diagnose.

from zio.

/bounty $100 for reproducer and fix.

from zio.

💎 $100 bounty • ZIO

Steps to solve:

1. Start working: Comment /attempt #8855 with your implementation plan
2. Submit work: Create a pull request including /claim #8855 in the PR body to claim the bounty
3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to zio/zio!

Add a bounty • Share on socials

from zio.

I took a look. It seems to me, that the code generated by the Scala 3 macro hits a limit of max identifiers for a class.
If you provide me with a branch that can't build @siers, I can try to figure out more.

But my gut feeling says, that you might have hit a hard limitation of the Scala 3 compiler. Because the generated code I was looking at looked like it could not work with less identifiers. It might be possible to generate the code in a way that these identifiers are not all in the same class, but this seems complicated and maybe not worth it?

My guess is, that this only happens when the layer is build via the macro. So you could maybe wire it by hand as a workaround. Or have maybe a split of smaller argument lists for ZLayer.make in different files.

I would suggest, that you also open an issue for the Scala compiler after you have a branch with a reproducer.
At least they could improve the error message I guess so one could understand better what is going on.

from zio.

Here's a tree with the bug: https://github.com/dasch-swiss/dsp-api/tree/bug/class-too-large.

from zio.

It sounds like a macro bug rather than a compiler bug, because it probably just hit a limit as you said. What could be improved in the message? I'm not familiar enough with macro/compiler interaction to decide whether making a compiler bug ticket is warranted. (Should the output of the macro be dumped? No idea.)

Indeed, splitting it it into smaller chunks did help as a workaround.

from zio.

I don't think we can detect if the class is too big during macro expansion - first of all we don't know what what's beside our macro. The only thing I see it done is writing some heuristic that will show warning/error when we generate too much stuff (whatever that would mean).

But my gut feeling says, that you might have hit a hard limitation of the Scala 3 compiler - I believe that would be JVM limit, not Scala's. JS/Native can propably take unlimited amount. Like with number of function arguments - JVM has 255, JS does not have limit (https://stackoverflow.com/a/22747272)

The only thing Scala compiler may do is catch that exception and print that class is too large in nicer way?

from zio.

@andrzejressel might be true. But at the end, we can only optimize how many identifiers are generated by our macro. And I think that this is either optimal or close to it. What we could do though is, to have something like a identifier, that is a new instance of something, that holds a number of references to layer so that we reduce the number of identifiers on the top level, which I guess is the issue here.
But, I don't think this is worth it. The complexity is probably to high for the benefit. I doubt, that the average service runs into this issue. And since there is a workaround, I would set this to won't fix.

@jdegoes any opinion?

from zio.

I wouldn't go that far - instead I would calculate gas (I saw this name in blockchain and WASM interpreters) of macro - However I need to check what that would be (number of methods, method length - I haven't run the example code myself) - in any case after some threshhold is passed (3/4 of JVM limit) I would show warning to the user that this method has to be split to separate classes.

Zio-intellij could have an action to do it automatically - it would just split the list/graph in the middle and calculate layer inputs and outputs.

from zio.

I don't think passing everything by name is close to optimal (also generating tags and traces):

  final def ++[E1 >: E, RIn2, ROut1 >: ROut, ROut2](
    that: => ZLayer[RIn2, E1, ROut2]
  )(implicit tag: EnvironmentTag[ROut2]): ZLayer[RIn with RIn2, E1, ROut1 with ROut2] =
    self.zipWithPar(that)(_.union[ROut2](_))

  final def to[E1 >: E, ROut2](that: => ZLayer[ROut, E1, ROut2])(implicit
    trace: Trace
  ): ZLayer[RIn, E1, ROut2] =
    self >>> that

from zio.

Here is example that throws the same error: https://scastie.scala-lang.org/2Sy2v9mtQiyd0UxRR4mgAQ

from zio.

Here is example that throws the same error: https://scastie.scala-lang.org/2Sy2v9mtQiyd0UxRR4mgAQ

It compiles if I change it to:

  final def andEager[E1 >: E, RIn2, ROut1 >: ROut, ROut2](
    that: ZLayer[RIn2, E1, ROut2]
  ): ZLayer[RIn with RIn2, E1, ROut1 with ROut2] =
    self.zipWithPar(that)(_.unionAll[ROut2](_))

  final def toEager[E1 >: E, ROut2](that: ZLayer[ROut, E1, ROut2]): ZLayer[RIn, E1, ROut2] = {
    implicit val trace: Trace = Trace.empty
    self >>> that
  }

from zio.

@joroKr21 good catch I was focusing on the generated layers. We can't change the signature of the existing methods, but we can add new (package private?) methods to be used in the macro.

from zio.

Here is example that throws the same error: https://scastie.scala-lang.org/2Sy2v9mtQiyd0UxRR4mgAQ

It compiles if I change it to:

  final def andEager[E1 >: E, RIn2, ROut1 >: ROut, ROut2](

    that: ZLayer[RIn2, E1, ROut2]

  ): ZLayer[RIn with RIn2, E1, ROut1 with ROut2] =

    self.zipWithPar(that)(_.unionAll[ROut2](_))



  final def toEager[E1 >: E, ROut2](that: ZLayer[ROut, E1, ROut2]): ZLayer[RIn, E1, ROut2] = {

    implicit val trace: Trace = Trace.empty

    self >>> that

  }

The problem with this is that it used unionAll instead of union. How much does the EnvironmentTag implicit contributes to the issue?

from zio.

The problem with this is that it used unionAll instead of union.

Why is it a problem?

from zio.

Because unionAll doesn't prune the environment. So if a service exists both in the left and right layer, the one from the right layer will be preferred even if the layer isn't meant to contain that service.

And to prune the environment we need the EnvironmentTag

from zio.

Perhaps the issue is not the EnvironmentTag but the fact that we need to pass it to the function. Maybe something like this also works:

Also I'm not even sure there is much benefit in excluding the EnvironmentTag. I think the main issue might be that zipWithPar is by-name, so maybe something like this also resolves it:

final def andEager[E1 >: E, RIn2, ROut1 >: ROut, ROut2](
    that: ZLayer[RIn2, E1, ROut2]
  ): ZLayer[RIn with RIn2, E1, ROut1 with ROut2] = {
    val that0 = that.map(_.prune[ROut2])
    self.zipWithPar(that0)(_.unionAll[ROut2](_))
}

from zio.

I would expect ++ to pick the latter. Like ++ for a Map. It's kinda weird if it doesn't.

from zio.

If you insist on pruning, you could prune all input layers once and be done with it. No need to prune when combining.

from zio.

If you insist on pruning, you could prune all input layers once and be done with it. No need to prune when combining.

I don't think it's as simple as you describe it. Here's an example where this will be problematic:

import zio.*

trait Foo
final class Foo1 extends Foo
final class Foo2 extends Foo
final class Bar

val fooBarLayer: ULayer[Foo & Bar] = ZLayer.succeed(new Foo1) ++ ZLayer.succeed(new Bar)
val barLayer: ULayer[Bar]          = fooBarLayer // Environment contains both Foo1 and Bar!
val foo2Layer: ULayer[Foo]         = ZLayer.succeed(new Foo2)

// barLayer isn't pruned, so we end up with Foo1 and Bar instead of Foo2 and Bar
val combined: ULayer[Foo & Bar] = foo2Layer ++ barLayer

Based on how we currently do things, we'll have Foo2 and Bar in the combined layer, but with what you're proposing we'll end up with Foo1 and Bar

from zio.

Based on how we currently do things, we'll have Foo2 and Bar in the combined layer, but with what you're proposing we'll end up with Foo1 and Bar

How do you know if the code generated by ZLayer.make will do foo2Layer ++ barLayer or barLayer ++ foo2Layer?
I think it would be the least surprising if you prune all inputs before you start combining them.

from zio.

Let me summarize:

• it is probably an edge case
• the current PR won't work because the pruning has to happen on combining
• we could use arrays, but this solution is very complicated (and added from me: Uses common code for Scala 2/3 which makes it harder to understand, change and keep the solution bug free)
• there is a workaround

And a question that came to my mind: Is it actually necessary to generate all of the identifiers that the macro creates into the parent class? These things can never be accessed publicly by design. Should this issue be solved on the side of the compiler?

from zio.

How do you know if the code generated by ZLayer.make will do foo2Layer ++ barLayer or barLayer ++ foo2Layer? I think it would be the least surprising if you prune all inputs before you start combining them.

We don't know that - in fact, we don't care about the order. That's the point of pruning; by keeping only the services that are statically known to be combined within the layer we remove any other services that might be there. And in the case of a user error, where more than 1 layer provide the same service, an "Ambiguous layer" compiling error is raised

from zio.

We don't know that - in fact, we don't care about the order. That's the point of pruning; by keeping only the services that are statically known to be combined within the layer we remove any other services that might be there. And in the case of a user error, where more than 1 layer provide the same service, an "Ambiguous layer" compiling error is raised

So if the order of operations is not well defined from the user's PoV, why would the right overwriting the left matter? It's an implementation detail.

from zio.

I just realised this compiles fine on Scala 2, so there must be some difference in the generated Scala code and/or bytecode.

from zio.

@joroKr21 see the test case here: #8901 (review)

from zio.

@joroKr21 I've checked it with some auto generated code: 400KB Scala 3 vs 300 KB Scala 2.13

• https://www.decompiler.com/jar/f94aa9ec06734b73956163907b13aaa9/Test%24.java
• https://www.decompiler.com/jar/123434e46e7c44758ad4b43d37b69ccc/Test%24.java

S3 seems to be generating a lot of methods while S2 is inlining them this is just how this tool is visualizing. But there is indeed huge size difference: https://contributors.scala-lang.org/t/lambdas-generate-more-bytecode-in-scala-3-than-in-scala-2-13/6671

from zio.

💡 @joroKr21 submitted a pull request that claims the bounty. You can visit your bounty board to reward.

from zio.

I just realised this compiles fine on Scala 2, so there must be some difference in the generated Scala code and/or bytecode.

We have recently migrated the dsp-api from Scala 2 to Scala 3. This is not a Scala 3 only problem I have seen the error with the dsp-api code base on Scala 2 already.

from zio.

That's interesting, I guess it's worth it to make the same changes in Scala 2.
On a general note, I think >>> by value and ++ by name strikes the right balance between a big method (which can reach method size limits) and too many lambdas (which can reach number of class entries limit).

from zio.

🎉🎈 @joroKr21 has been awarded $100! 🎈🎊

• Share on socials
• Give feedback

from zio.