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

Reduce constant pool usage, this make Magnolia able to derive instance for sealed trait that has a lot of subtypes #486

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

nox213
Copy link

@nox213 nox213 commented Aug 23, 2023

Even after #485 has been merged, I found that there is a limit to the number of subtypes when deriving instances with the split. By splitting subtypes array initialization to multiple functions, we are able to avoid method too large error, but we aren't able to avoid class too large error because of constant pool limit. When we initialize Subtype, we pass a lot of values, so we consume constant pool a lot which has 65534 entries.
When I tested like below, the limitation of the number of subtypes is about 1550 with current implementation.

trait Print {
 ...
}

trait GenericPrint {
   ... 
}

sealed trait Event

case class AEvent(...) extends Event
... // variants

object Instances extends GenericPrint {
    implicit val aEventInstance = ...
    ... // instances of subtypes

    implicit val eventInstance = gen[Event]
}

With this PR, we can derive instance for type has about 3600 subtypes and it seems there is no compilation speed degradation.

@nox213 nox213 changed the base branch from scala3 to scala2 August 23, 2023 13:03
@adamw adamw merged commit 3ac3496 into softwaremill:scala2 Aug 25, 2023
@adamw
Copy link
Member

adamw commented Aug 25, 2023

Thanks :)

@adamw
Copy link
Member

adamw commented Aug 29, 2023

@nox213 Sorry but I have to revert this one, it causes some serialization problems (see #487). Maybe it would be possible for you or @RustedBones to fix them, then we can merge this (patched) change again :)

@nox213
Copy link
Author

nox213 commented Aug 29, 2023

I see. Thank you for letting me know. I will look into it.

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.

2 participants