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.

Sign up for GitHub

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

Jump to bottom

Scala 3 recursive structure go into infinite loop #388

Closed
liewhite opened this issue Apr 23, 2022 · 2 comments
Closed

Scala 3 recursive structure go into infinite loop #388

liewhite opened this issue Apr 23, 2022 · 2 comments
Milestone
3.0.0-M2

Comments

@liewhite
Copy link

Hello, deriving upickle's ReadWriter for recursive structures in scala 3 seems run into infinite loop...

case class Recur(recur: Option[Recur]) derives ReadWriter
println(write(Recur(None))) // hang on here

And with no compile error
Maybe we should use lazy vals somewhere?
@aaronp
Copy link

aaronp commented Nov 28, 2022

I've just come here to report this as well. I'm creating a swagger generator for parts of Li's stack (cask for server, requests for client), and the 'petstore' test case has a recursive data structure like this - exposing this issue.

I'd love to help if I can - pairing on this, whatever. Otherwise +1 for the issue

@julienrf julienrf mentioned this issue Nov 29, 2022
Closed
@lihaoyi lihaoyi mentioned this issue Feb 1, 2023
Merged
lihaoyi added a commit that referenced this issue Feb 6, 2023
@lihaoyi
Speeding up Scala 3 Writing macros (#440)
0f87fae 
Fixes #389 and
#388

This PR moves a bunch of logic from runtime to compile time, and
optimizes whatever runtime logic remains. It also consolidates the logic
with the Scala 2 logic as much as possible, e.g. re-using `writeSnippet`
to manage writes and `CaseObjectContext`/`HugeCaseObjectContext` to
manage validation and error reporting during a read

Running some ad-hoc benchmarks on my laptop, `./mill bench.jvm.run`, it
gives a significant speedup on reads and writes, and brings it close to
the Scala 2.13 numbers (higher is better):

| Benchmark (5000ms) | Scala 3 Before | Scala 3 After | Scala 2 |

|-------------------------------------|----------------|---------------|---------|
| upickleDefault Read | 637 | 1065 | 1403 |
| upickleDefault Write | 839 | 1452 | 1549 |
| upickleDefaultByteArray Read | 582 | 1172 | 1126 |
| upickleDefaultByteArray Write | 847 | 1218 | 1277 |
| upickleDefaultBinary Read | 925 | 3853 | 3844 |
| upickleDefaultBinary Write | 1252 | 3117 | 3666 |
| upickleDefaultCached Read | 620 | 1300 | 1412 |
| upickleDefaultCached Write | 829 | 1555 | 1588 |
| upickleDefaultByteArrayCached Read | 575 | 1182 | 1095 |
| upickleDefaultByteArrayCached Write | 838 | 1223 | 1297 |
| upickleDefaultBinaryCached Read | 928 | 3825 | 3885 |
| upickleDefaultBinaryCached Write | 1266 | 2907 | 3674 |

Note that the generated code, especially for reads, is still not as
optimized as the Scala 2 versions:

1. I couldn't figure out how to generate an anonymous class with typed
fields in Scala 3, so I'm putting things in an `Array[Any]`
2. I couldn't figure out how to generate `match` statements, so I
generated a `Map[K, V]` and look it up at runtime.

Fixing this issues and moving the reading logic into the macro should
also be possible, but can happen in a separate PR

All existing tests pass. Added a regression test for the recursive Scala
3 scenario that hangs on master. Also moved a bunch of `AdvancedTests`
into the shared folder now that they work
@lefou lefou added this to the 3.0.0-M2 milestone Feb 6, 2023
@lefou
Copy link
Member

lefou commented Feb 6, 2023

Closed by #440

@lefou lefou closed this as completed Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.0.0-M2
Development

No branches or pull requests
3 participants
@aaronp @lefou @liewhite