Add support for lambda serialization #5837
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dottybot
reviewed
Feb 3, 2019
There was a problem hiding this comment.
Hello, and thank you for opening this PR! 🎉
All contributors have signed the CLA, thank you! ❤️
Commit Messages
We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).
Please stick to these guidelines for commit messages:
- Separate subject from body with a blank line
- When fixing an issue, start your commit message with
Fix #<ISSUE-NBR>:- Limit the subject line to 72 characters
- Capitalize the subject line
- Do not end the subject line with a period
- Use the imperative mood in the subject line ("Add" instead of "Added")
- Wrap the body at 80 characters
- Use the body to explain what and why vs. how
adapted from https://chris.beams.io/posts/git-commit
Have an awesome day! ☀️
smarter
force-pushed
the
serialize-lambdas
branch
3 times, most recently
from
e9971c4 to
ff4c5cf
Compare
smarter
force-pushed
the
serialize-lambdas
branch
from
ff4c5cf to
baa05cd
Compare
smarter
force-pushed
the
serialize-lambdas
branch
4 times, most recently
from
e50de50 to
beaed8c
Compare
OlivierBlanvillain
approved these changes
Feb 6, 2019
smarter
force-pushed
the
serialize-lambdas
branch
from
beaed8c to
97ffc24
Compare
To avoid deadlocks when combining objects, lambdas and multi-threading, lambdas in objects are compiled to instance methods of the module class instead of static methods (see tests/run/deadlock.scala and scala/scala-dev#195 for details). This has worked well for us so far but this is problematic for serialization: serializing a lambda requires serializing all the values it captures, if this lambda is in an object, this means serializing the enclosing object, which fails if the object does not extend Serializable. Because serializing objects is basically free since scala#5775, it seems like the simplest solution is to simply make all objects Serializable, this certainly seems preferable to deadlocks. This commit causes a cyclic reference to happen in some cases, we add a workaround to avoid this in Trees.scala and fix it properly in the commit.
The previous commit added _root_.scala.Serializable as a parent to every object, this lead to a cyclic reference manifesting itselfs as "not found: type Type" in Trees.scala that we worked around by replace `Type` with `Types.Type`. This commit fixes this properly by adding a shortcut in typedIdent when typing `_root_` which avoids forcing imports.
These interfaces are only used when creating closures with no explicit SAM types, and those closures should always be serializable.
…for FunctionXXL This only makes a difference after erasure, and I believe this is less surprising than the previous behavior. This necessitated a slight refactoring in GenericSignature to avoid an infinite loop now that `isXXLFunctionClass` returns true for scala.FunctionXXL and not just for the classes that erase to scala.FunctionXXL.
In Scala, lambdas whose SAM extend Serializable as well as lambdas whose SAM is simply scala.Function* should be serializable but this was not the case in Dotty so far. On the JVM, lambdas instantiated using invokedynamic calls require some special handling to be serializable: 1. We need to use the invokedynamic bootstrap method `LambdaMetaFactory#altMetafactory` instead of `LambdaMetaFactory#metafactory`, this allows us to pass the FLAG_SERIALIZABLE flag. This is implemented in the backend submodule commit included in this commit (see lampepfl/scala#39). 2. In the enclosing class where the lambda is defined, a $deserializeLambda$ method needs to be generated, this is implemented in this commit. Most of the logic for $deserializeLambda$ is implemented in the Scala 2.12 standard libraries class scala.runtime.LambdaDeserialize and scala.runtime.LambdaDeserializer which can be used here as-is, the only logic we actually need to implement here is: 1. In `collectSerializableLambdas`, we collect all serializable lambdas. Unlike scalac, our backend does not do any inlining currently so our implementation is more straightfoward than theirs. 2. In `addLambdaDeserialize`, we implement the actual $deserializeLambda$ method, the implementation here is directly copied from scalac, it's complex because it needs to work around a limitation of bootstrap methods (they cannot take more than 251 arguments). Since some of this code comes from scalac, this is: Co-Authored-By: Jason Zaugg <[email protected]> Co-Authored-By: Lukas Rytz <[email protected]>
smarter
force-pushed
the
serialize-lambdas
branch
from
97ffc24 to
5fc2119
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See also the backend PR: lampepfl/scala#39