Skip to content

Make lambda body public rather than using static accessor#4454

Merged
retronym merged 1 commit intoscala:2.11.xfrom
retronym:topic/indylambda-no-accessor
Apr 20, 2015
Merged

Make lambda body public rather than using static accessor#4454
retronym merged 1 commit intoscala:2.11.xfrom
retronym:topic/indylambda-no-accessor

Conversation

@retronym
Copy link
Member

Under -Ydelambdafy:method (the basis of the upcoming "indylambda"
translation for -target:jvm-1.8), an anonymous function is currently
encoded as:

  1. a private method containing the lambda's code
  2. a public, static accessor method that allows access to 1 from
    other classes, namely:
  3. an anonymous class capturing free variables and implementing
    the suitable FunctionN interface.

In our prototypes of indylambda, we do away with 3, instead deferring
creating of this class to JDK8's LambdaMetafactory by way of an
invokedynamic instruction at the point of lambda capture. This
facility can use a private method as the lambda target; access is
mediated by the runtime-provided instance of
java.lang.invoke.MethodHandles.Lookup that confers the privelages
of the lambda capture call site to the generated implementation class.
Indeed, The java compiler uses this to emit private lambda body
methods.

However, there are two Scala specific factors that make this
a little troublesome.

First, Scala's optimizer may want to inline the lambda capture
call site into a new enclosing class. In general, this isn't a
safe optimization, as invokedynamic instructions have call-site
specific semantics. But we will rely the ability to inline like
this in order to eliminate closures altogether where possible.

Second, to support lambda deserialization, the Java compiler creates
a synthetic method $dersializeLamda$ in each class that captures
them, just to be able to get the suitable access to spin up an
anoymous class with access to the private method. It only needs
to do this for functional interfaces that extends Serializable,
which is the exception rather than the rule. But in Scala, every
function must support serialization, so blindly copying the Java
approach will lead to some code bloat.

I have prototyped a hybrid approach to these challenges: use the
private method directly where it is allowed, but fall back to
using the accessor method in a generic lambda deserializer or
in call sites that have been inlined into a new enclosing class.

However, the most straight forward approach is to simply emit the
lambda bodies as public (with an mangled name and with the SYHTNETIC
flag) and use them in all cases. That is what is done in this commit.

This does moves us one step backwards from the goals of SI-7085,
but it doesn't seem sensible to incur the inconvenience from locking
down one small part of our house when we don't have a plan or the
budget to complete that job.

The REPL has some fancy logic to decompile the bodies of lambdas
(:javap -fun C#m) which needed tweaking to accomodate this change.
I haven't tried to make this backwards compatible with the old
encoding as -Ydelambdafy:method is still experimental.

@scala-jenkins scala-jenkins added this to the 2.11.7 milestone Apr 17, 2015
@retronym
Copy link
Member Author

Review by @lrytz

@retronym retronym force-pushed the topic/indylambda-no-accessor branch from a593bcd to 261601b Compare April 17, 2015 05:21
@retronym
Copy link
Member Author

/cc @som-snytt for the :javap -fun fun.

@som-snytt
Copy link
Contributor

Thx, @lrytz had forewarned me. I've been waiting for the knock at the door.

I hope that wasn't too painful. I've blotted out the memory, but it seems to me that last time it was painful.

Under -Ydelambdafy:method (the basis of the upcoming "indylambda"
translation for -target:jvm-1.8), an anonymous function is currently
encoded as:

 1. a private method containing the lambda's code
 2. a public, static accessor method that allows access to 1 from
    other classes, namely:
 3. an anonymous class capturing free variables and implementing
    the suitable FunctionN interface.

In our prototypes of indylambda, we do away with 3, instead deferring
creating of this class to JDK8's LambdaMetafactory by way of an
invokedynamic instruction at the point of lambda capture. This
facility can use a private method as the lambda target; access is
mediated by the runtime-provided instance of
`java.lang.invoke.MethodHandles.Lookup` that confers the privelages
of the lambda capture call site to the generated implementation class.
Indeed, The java compiler uses this to emit private lambda body
methods.

However, there are two Scala specific factors that make this
a little troublesome.

First, Scala's optimizer may want to inline the lambda capture
call site into a new enclosing class. In general, this isn't a
safe optimization, as `invokedynamic` instructions have call-site
specific semantics. But we will rely the ability to inline like
this in order to eliminate closures altogether where possible.

Second, to support lambda deserialization, the Java compiler creates
a synthetic method `$dersializeLamda$` in each class that captures
them, just to be able to get the suitable access to spin up an
anoymous class with access to the private method. It only needs
to do this for functional interfaces that extends Serializable,
which is the exception rather than the rule. But in Scala, *every*
function must support serialization, so blindly copying the Java
approach will lead to some code bloat.

I have prototyped a hybrid approach to these challenges: use the
private method directly where it is allowed, but fall back to
using the accessor method in a generic lambda deserializer or
in call sites that have been inlined into a new enclosing class.

However, the most straight forward approach is to simply emit the
lambda bodies as public (with an mangled name and with the SYHTNETIC
flag) and use them in all cases. That is what is done in this commit.

This does moves us one step backwards from the goals of SI-7085,
but it doesn't seem sensible to incur the inconvenience from locking
down one small part of our house when we don't have a plan or the
budget to complete that job.

The REPL has some fancy logic to decompile the bodys of lambdas
(`:javap -fun C#m`) which needed tweaking to accomodate this change.
I haven't tried to make this backwards compatible with the old
encoding as `-Ydelambdafy:method` is still experimental.
@retronym retronym force-pushed the topic/indylambda-no-accessor branch from 261601b to 1aa97cb Compare April 17, 2015 05:40
@retronym
Copy link
Member Author

Here's how the rest of the indylambda work looks rebased on this commit:

1aa97cb...retronym:topic/indylambda-emit-indy

@retronym
Copy link
Member Author

I was tempted to unsupport the feature, but I managed to bend it into shape. This PR removes one level of indirection, which lessens the pain ever so slightly.

But it is good to reflect (preflect?) on potential for maintenance costs like these when weighing up whether or not to add features that have a small audience.

Conversely, let's try to seek out cases where a bit of complexity will really pay its way: can we rip out the ad-hoc typechecking in the REPL and get real tab completion going based on the typechecker proper? Ammonite seems to have done this in a week or so of @lihaoyi's time (admittedly, this represents about a six months in regular-human-time...)

@retronym
Copy link
Member Author

BTW, I plan to revisit the naming scheme of lambda bodies when as part of the work on lambda serialization. The goal will be to have some measure of stability in the face of what the programmer perceives to be unrelated changes in a source file. We should be able to largely mimic Java's design in the regard.

@retronym
Copy link
Member Author

Another design possibility is to make all lambda body methods static (by substituting this for a parameter $this). (Currently we detect lambda bodies that don't capture this and emit them as static.)

This would let us avoid the ugly expanded com$acme$Widget$anonFun$1 names that makeNonPrivate creates (to avoid potential name clashes in sub/super-classes).

I don't think we need to decide on that as part of this PR though, it can come in a later step.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for having this abstract class, given there's only one implementation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lrytz
Copy link
Member

lrytz commented Apr 17, 2015

Looks great! Should we run a community build with delambdafy:method before merging?

@lrytz
Copy link
Member

lrytz commented Apr 17, 2015

LGTM - given that the community build might be broken for another day or so, we can also merge this now. We do have the nightly delambdafy-genbcode-optimize build anyway.

retronym added a commit that referenced this pull request Apr 20, 2015
Make lambda body public rather than using static accessor
@retronym retronym merged commit 555f8f0 into scala:2.11.x Apr 20, 2015
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.

4 participants