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

Large case class crashes compiler with stackoverflow exception in Scala 3 #386

Closed
Quafadas opened this issue Apr 19, 2022 · 7 comments
Closed

Comments

@Quafadas
Copy link
Contributor

Quafadas commented Apr 19, 2022

I have a large case class (let's say 65ish fields). If commen out random subsets of these fields - i.e. reduce the number of parameters, compilation works fine.

All fields are basically all doubles, strings, or Option[Double] Option[String]

Naive compilations gets me this error;

[error] 141 |  implicit val rw_auth: ReadWriter[Coa] = macroRW
[error]     |                                                   ^^^^^^^
[error]     |                   Maximal number of successive inlines (32) exceeded,
[error]     |                   Maybe this is caused by a recursive inline method?
[error]     |                   You can use -Xmax-inlines to change the limit.

[error]     | This location contains code that was inlined from corre.scala:141
[error]     | This location contains code that was inlined from macros.scala:40

If I follow the error message (which is great) and set this;

 def scalacOptions = Seq[String](
    "-Xmax-inlines:64",
  )

I can compile a "smaller" version of the case class. If I boost that "max-inlines" to 128, then I get a stackoverflow error.

Even setting this in ".mill-jvm-opts"

-Xss10m
-Xmx12G

Which I feel is rather generous, getd me this in compile.log;

�[0m[�[0m�[31merror�[0m] �[0m�[0m## Exception when compiling 47 sources to C:\temp\Scala-ILS-Lib\out\tools\compile.dest\classes�[0m
�[0m[�[0m�[31merror�[0m] �[0m�[0mjava.lang.StackOverflowError�[0m
�[0m[�[0m�[31merror�[0m] �[0m�[0mdotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1527)�[0m
�[0m[�[0m�[31merror�[0m] �[0m�[0mdotty.tools.dotc.ast.Trees$Instance$TreeTraverser.traverseChildren(Trees.scala:1648)�[0m
�[0m[�[0m�[31merror�[0m] �[0m�[0mdotty.tools.dotc.typer.Inliner$$anon$8.traverse(Inliner.scala:1743)�[0m
�[0m[�[0m�[31merror�[0m] �[0m�[0mdotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1647)�[0m
�[0m[�[0m�[31merror�[0m] �[0m�[0mdotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1647)�[0m
�[0m[�[0m�[31merror�[0m] �[0m�[0mdotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1597)�[0m
�[0m[�[0m�[31merror�[0m] �[0m�[0mdotty.tools.dotc.ast.Trees$Instance$TreeTraverser.traverseChildren(Trees.scala:1648)�[0m
�[0m[�[0m�[31merror�[0m] �[0m�[0mdotty.tools.dotc.typer.Inliner$$anon$8.traverse(Inliner.scala:1743)�[0m

I'm aware that historically, one couldn't go beyond 21 fields or so. My questions:

Would this be expected to work?
Any hints?
I feel like this is a bug somewhere, as it causes a compiler stackoverflow, with what I feel is not an unreasonable amount of data. I do not claim certainty, that it is a bug, or that the bug is in upickle.

@Quafadas
Copy link
Contributor Author

Quafadas commented Apr 19, 2022 via email

@Quafadas
Copy link
Contributor Author

Quafadas commented Apr 19, 2022

Perhaps noteworthy, I have these in scope...

object Coa {
  import utils.implicits.OptionReader
  implicit val rw_auth: ReadWriter[Coa] = macroRW
}
  implicit def OptionReader[T: Reader]: Reader[Option[T]] = {
    new Reader.Delegate[Any, Option[T]](implicitly[Reader[T]].map(Some(_))) {
      override def visitNull(index: Int) = None
    }
  }

I'll look into seeing if I can produce a minimal example if I can get some time.

It's all otherwise the "default" upickle behaviour

@jodersky
Copy link
Member

jodersky commented Apr 19, 2022

I'll look into seeing if I can produce a minimal example if I can get some time.

That would be very helpful, thank you!

Without rushing to conclusions, here are some potentially relevant points:

  • The macroRW derivation method uses Scala 3 inlines with quotes and splices. The depth of quotes within splices within quotes etc, is limited. I think that this should be independent of the number of fields, but maybe there is a bug in the derivation code.

  • If this is indeed a limitation, it may be possible to create ReadWriters through direct manipulation of the AST instead of using quotes and splices. Updating the derivation macro to use ASTs is a larger project however, and I would prefer to first exhaust all other options before attempting it.

@Quafadas
Copy link
Contributor Author

I've wanted to experiment with mill tasks for a while... which I'm not totally sure if it was successful, but this repo might help with a reproduction.
https://github.com/Quafadas/upickle386

@Quafadas
Copy link
Contributor Author

Quafadas commented Apr 27, 2022

For the record, circe experiences a similar outcome on the scala 3 compiler.

https://scastie.scala-lang.org/Quafadas/Pay4wq3zSMCger0ScI9wdQ/38

Which is suspicious.

The above comment was wrong, and not really relevant to this issue.

@lihaoyi
Copy link
Member

lihaoyi commented Feb 27, 2023

This is still an issue. We have tests that pass on Scala 2 that currently do not pass on Scala 3

https://github.com/com-lihaoyi/upickle/blob/85bd87b23a9b38eb7481173af8157c8aaa3341a9/upickle/test/src-2/upickle/TooBigTests.scala

Might need help from Scala 3 compiler folks to help figure this out

@lihaoyi lihaoyi changed the title Large case class crashes compiler with stackoverflow exception Large case class crashes compiler with stackoverflow exception in Scala 3 Feb 27, 2023
@lihaoyi lihaoyi changed the title Large case class crashes compiler with stackoverflow exception in Scala 3 Large case class crashes compiler with stackoverflow exception in Scala 3 (500USD Bounty) Feb 16, 2024
@lihaoyi lihaoyi changed the title Large case class crashes compiler with stackoverflow exception in Scala 3 (500USD Bounty) Large case class crashes compiler with stackoverflow exception in Scala 3 Feb 16, 2024
@lihaoyi
Copy link
Member

lihaoyi commented Feb 16, 2024

From what I can tell, this works in 3.3.1 with -Xmax-inlines, and my tests seem to work correctly for case classes up to 150 fields. The problem also goes away entirely in Scala 3.4.0. See #557 for details

@lihaoyi lihaoyi closed this as completed Feb 16, 2024
lihaoyi added a commit that referenced this issue Feb 16, 2024
Turn on tests to verify
#386

Scala 3 tests were accidentally disabled in
#553, this re-enables them

The big-case-class tests do work, with the caveat you need to set
`-Xmax-inlines` to be greater than the width of your case class, and the
compilation is notably slower on 3.3.1 than in Scala 213.11. However,
the performance seems to be better in 3.4.0, and `-Xmax-inlines` is no
longer necessary.

For now, I just add `-Xmax-inlines` and accept the compilation slowness,
so as to not break anyone using Scala 3.3.1. Eventually, as people
upgrade to 3.4.0, the problem can be expected to go away
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

No branches or pull requests

3 participants