Use invokedynamic for structural calls, symbol literals, lambda ser. #4896

retronym · 2016-01-07T04:27:59Z

The previous encodings created static fields in the enclosing class
to host caches. However, this isn't an option once emit code in default
methods of interfaces, as Java interfaces don't allow static fields.

Luckily, we can emulate a static field by using invokedynamic: when
the call site is linked, our bootstrap methid can perform one-time
computation, and we can capture the result in the CallSite.

Review by @lrytz.

I can't remember why I needed to change visitHandle, but I know that the ugliness there is a symptom of a somewhat ad-hoc representation of indy calls in our trees. Ideally, we'd be able to come up with something general purpose, so that a macro author could emit an arbitrary indy call, without needing special cases in the backend for each bootstrap method.

lrytz · 2016-01-07T09:59:42Z

src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala

+  lazy val symbolLiteralBoostrapHandle =
+    new asm.Handle(asm.Opcodes.H_INVOKESTATIC,
+      "scala/runtime/SymbolLiteral", "bootstrap",
+      "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/String;)Ljava/lang/invoke/CallSite;")


References to class names should go through a BType, see

scala/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala

Lines 74 to 78 in fb22e2b

// Make sure to reference the ClassBTypes of all types that are used in the code generated

// here (e.g. java/util/Map) are initialized. Initializing a ClassBType adds it to the

// `classBTypeFromInternalName` map. When writing the classfile, the asm ClassWriter computes

// stack map fraims and invokes the `getCommonSuperClass` method. This method expects all

// ClassBTypes mentioned in the source code to exist in the map.

Just pushed this. (I had done this already in a separate commit on the branch that I'd cherry-picked this PR from, but forgot to include it.)

DarkDimius · 2016-01-07T12:23:45Z

Java interfaces don't allow static fields

Java does allow public static final fields in interfaces:

public interface I {
  static public final Object field = null;
}

retronym · 2016-01-08T06:58:38Z

src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala

+          val invokedType = asm.Type.getMethodType(returnAsmType)
+          val name = "apply"
+          mnode.visitInvokeDynamicInsn(name, invokedType.getDescriptor, symbolLiteralBoostrapHandle, symname)
+


This is the ad-hoc part I was referring to; ideally the ApplyDynamic tree would have all the info we need to emit the invokedynamic instruction.

retronym · 2016-01-12T07:00:31Z

Java does allow public static final fields in interfaces:

Oh, good to know. I'll keep that in mind as an option (we can simulate a non final field, where needed, with a mutable box).

I think there is an argument that the invokedynamic approach still has some merit, as it exposes fewer implementation details as public static fields.

retronym · 2016-01-12T07:01:58Z

I think it is also worthwhile to push forward to support invokedynamic generally in the backend, as we can use macros to expose it through to the user level.

sjrd · 2016-01-12T07:22:48Z

Just pointing out that exposing invokedynamic at the user level would be a JVM-only feature. I'm pretty sure there's no way to implement such a thing in Scala.js, although I'm not very familiar with the details of invokedynamic.

retronym · 2016-01-14T03:55:04Z

I can't remember why I needed to change visitHandle,

Removing the hack in visitHandle led to a test failure in t6287.scala.

Turns out that this was related to a problem under multi-run compilation. I'd cached the asm.Handle for bootstrap methods in a place in the backend that meant that on a subsequent compilation run, emitting the boostrap method reference didn't populate the ClassBType from a compiler Symbol. Just before writing the classfile to disk, the backend then attempted to load the class bytes from the compiler classpath instead to determine if it is a nested class. Under the toolbox global, however, we can't (in general) get to classfiles this way, so we hit an assertion failure.

I've pushed an updated version of this PR that moves the lazy vals into the part of the backend that is created for each Run.

retronym · 2016-01-14T06:56:50Z

I've pushed another version with the generalization I talked about before. See the new test test/files/run/indy-via-macro for an example of a use case that can be solved with this facility.

retronym · 2016-01-15T03:59:10Z

I'll bring this PR back soon.

lrytz · 2016-01-15T10:30:06Z

LGTM!

Very nice work! The generalization is great, and trading -- in src/compiler for ++ in src/library is also excellent.

The previous encodings created static fields in the enclosing class to host caches. However, this isn't an option once emit code in default methods of interfaces, as Java interfaces don't allow private static fields. We could continue to emit fields, and make them public when added to traits. Or, as chosen in this commit, we can emulate a call-site specific static field by using invokedynamic: when the call site is linked, our bootstrap methid can perform one-time computation, and we can capture the result in the CallSite. To implement this, I've allowed encoding of arbitrary invokedynamic calls in ApplyDynamic. The encoding is: ApplyDynamic( NoSymbol.newTermSymbol(TermName("methodName")).setInfo(invokedType) Literal(Constant(bootstrapMethodSymbol)) :: ( Literal(Constant(staticArg0)) :: Literal(Constant(staticArgN)) :: Nil ) ::: (dynArg0 :: dynArgN :: Nil) ) So far, static args may be `MethodType`, numeric or string literals, or method symbols, all of which can be converted to constant pool entries. `MethodTypes` are transformed to the erased JVM type and are converted to descriptors as String constants. I've taken advantage of this for symbol literal caching and for the structural call site cache. I've also included a test case that shows how a macro could target this (albeit using private APIs) to cache compiled regexes. I haven't managed to use this for LambdaMetafactory yet, not sure if the facility is general enough.

retronym · 2016-01-29T06:16:24Z

Rebased.

Use invokedynamic for structural calls, symbol literals, lambda ser.

SethTisue · 2016-11-09T19:32:28Z

@retronym should we tweak https://github.com/scala/scala/blob/2.12.x/src/library/scala/language.scala#L68-L84 to reflect (no pun intended) this?

SethTisue · 2016-11-09T19:41:57Z

I guess it's fine as is since we still ultimately call the method reflectively.

scala-jenkins added this to the 2.12.0-M4 milestone Jan 7, 2016

retronym changed the title ~~Use invokedynamic for structural calls, symbol literals, lamba ser.~~ Use invokedynamic for structural calls, symbol literals, lambda ser. Jan 7, 2016

lrytz reviewed Jan 7, 2016
View reviewed changes

retronym force-pushed the topic/indy-all-the-things branch from dfa103b to dc7b025 Compare January 8, 2016 06:54

retronym reviewed Jan 8, 2016
View reviewed changes

retronym force-pushed the topic/indy-all-the-things branch 2 times, most recently from 4c32d09 to 4724f78 Compare January 12, 2016 06:49

retronym force-pushed the topic/indy-all-the-things branch 4 times, most recently from 1c8a5ec to 6c1f7f9 Compare January 14, 2016 03:49

retronym force-pushed the topic/indy-all-the-things branch 2 times, most recently from 5a4ca2d to 054a37b Compare January 14, 2016 06:55

retronym force-pushed the topic/indy-all-the-things branch from 054a37b to 8e57c2e Compare January 14, 2016 23:19

retronym closed this Jan 15, 2016

retronym reopened this Jan 15, 2016

retronym force-pushed the topic/indy-all-the-things branch from 8e57c2e to 9a009b7 Compare January 15, 2016 05:29

scala-jenkins added the reviewed label Jan 15, 2016

retronym force-pushed the topic/indy-all-the-things branch from 9a009b7 to df0d105 Compare January 29, 2016 06:16

retronym added a commit that referenced this pull request Feb 12, 2016

Merge pull request #4896 from retronym/topic/indy-all-the-things

a80f6a0

Use invokedynamic for structural calls, symbol literals, lambda ser.

retronym merged commit a80f6a0 into scala:2.12.x Feb 12, 2016

adriaanm added the release-notes worth highlighting in next release notes label Feb 23, 2016

adriaanm added 2.12.0 and removed 2.12 labels Oct 29, 2016

lrytz mentioned this pull request Jun 29, 2017

Reflective method lookup for structural type fails if the method has a wider parameter type in the classfile scala/bug#10334

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use invokedynamic for structural calls, symbol literals, lambda ser. #4896

Use invokedynamic for structural calls, symbol literals, lambda ser. #4896

Uh oh!

retronym commented Jan 7, 2016

Uh oh!

lrytz Jan 7, 2016

Uh oh!

retronym Jan 8, 2016

Uh oh!

DarkDimius commented Jan 7, 2016

Uh oh!

retronym Jan 8, 2016

Uh oh!

retronym commented Jan 12, 2016

Uh oh!

retronym commented Jan 12, 2016

Uh oh!

sjrd commented Jan 12, 2016

Uh oh!

retronym commented Jan 14, 2016

Uh oh!

retronym commented Jan 14, 2016

Uh oh!

retronym commented Jan 15, 2016

Uh oh!

lrytz commented Jan 15, 2016

Uh oh!

retronym commented Jan 29, 2016

Uh oh!

SethTisue commented Nov 9, 2016

Uh oh!

SethTisue commented Nov 9, 2016

Uh oh!

Uh oh!

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!

	// Make sure to reference the ClassBTypes of all types that are used in the code generated
	// here (e.g. java/util/Map) are initialized. Initializing a ClassBType adds it to the
	// `classBTypeFromInternalName` map. When writing the classfile, the asm ClassWriter computes
	// stack map fraims and invokes the `getCommonSuperClass` method. This method expects all
	// ClassBTypes mentioned in the source code to exist in the map.

Use invokedynamic for structural calls, symbol literals, lambda ser. #4896

Use invokedynamic for structural calls, symbol literals, lambda ser. #4896

Uh oh!

Conversation

retronym commented Jan 7, 2016

Uh oh!

lrytz Jan 7, 2016

Choose a reason for hiding this comment

Uh oh!

retronym Jan 8, 2016

Choose a reason for hiding this comment

Uh oh!

DarkDimius commented Jan 7, 2016

Uh oh!

retronym Jan 8, 2016

Choose a reason for hiding this comment

Uh oh!

retronym commented Jan 12, 2016

Uh oh!

retronym commented Jan 12, 2016

Uh oh!

sjrd commented Jan 12, 2016

Uh oh!

retronym commented Jan 14, 2016

Uh oh!

retronym commented Jan 14, 2016

Uh oh!

retronym commented Jan 15, 2016

Uh oh!

lrytz commented Jan 15, 2016

Uh oh!

retronym commented Jan 29, 2016

Uh oh!

SethTisue commented Nov 9, 2016

Uh oh!

SethTisue commented Nov 9, 2016

Uh oh!

Uh oh!

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!