Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: Backlog
    • Component/s: Optimizer
    • Environment:

      trunk

      Description

      Compiling

      trait Foo {
        @inline final def foo(x: Int): Unit = {
          println(x)
        }
      }
      class Bar {
        def bar(y: Foo): Unit = {
          y.foo(7)
        }
      }
      

      yields (with -Ydebug and -Ylog:inline)

      [log inliner] Treating CALL_METHOD test2.Foo.foo (dynamic)
      	receiver: trait Foo
      	icodes.available: true
      	concreteMethod.isEffectivelyFinal: true
      [log inliner] inline failed for test2.Foo.foo:
        pair.sameSymbols: false
        inc.numInlined < 2: true
        inc.hasCode: false
        isSafeToInline: false
        shouldInline: false
              
      test2.scala:12: warning: Could not inline required method foo because bytecode was unavailable.
          y.foo(7)
               ^
      [log inliner]  test2.Bar.bar blocks before inlining: 1 (4) after: 1 (4)
      [log inliner] Analyzing test2.Foo$class
      [log inliner] Not inlining into foo because it is marked @inline.
      [log inliner]  test2.Foo$class.foo blocks before inlining: 1 (7) after: 1 (7)
      

      Expected behavior would be inlining foo into bar. It all works if you change Foo to be a class instead of a trait.

      I did some digging around and apparently ICode reading does not resolve methods bodies in traits:

      scala> global.icodes.icode(global.definitions.getClass("scala.collection.immutable.MapLike"),true).methods.map(_.code)
      res0: List[global.icodes.global.icodes.Code] = List(null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, ICode 'companion', null, null, null, null, null, null, null, null, null, null, ...
      

      Furthermore, it looks like class file reading will never load a trait's impl class (which actually contains the method bodies):

      scala> global.definitions.getClass("scala.collection.immutable.MapLike")
      res2: global.Symbol = trait MapLike
      
      scala> res2.implClass
      res3: global.Symbol = <none>
      

      There's really no way to get at it:

      scala> global.definitions.getClass("scala.collection.immutable.MapLike$class")
      scala.reflect.internal.MissingRequirementError: class scala.collection.immutable.MapLike$class not found.
      	at scala.reflect.internal.Definitions$definitions$.getModuleOrClass(Definitions.scala:662)
      	at scala.reflect.internal.Definitions$definitions$.getClass(Definitions.scala:615)
      

      The reason is that ClassPath.scala explicitly excludes these class files (ending in $class.class):

      /** A useful name filter. */
      def isTraitImplementation(name: String) = name endsWith "$class.class"
      ...
      object DefaultJavaContext extends JavaContext {
        override def isValidName(name: String) = !isTraitImplementation(name)
      }
      

      I believe this is a bug on its own - commenting out isValidName fixes the implClass behavior but not the inlining issue altogether.

        Issue Links

          Activity

          Hide
          Miguel Garcia added a comment -
          Show
          Miguel Garcia added a comment - Discussion on fixing ICodeReader at https://groups.google.com/d/topic/scala-internals/w_ySCPJRmvg/discussion
          Hide
          Miguel Garcia added a comment -

          To get a handle on this problem, I've implemented a fix in the experimental optimizer http://magarciaepfl.github.com/scala/

          As a result:

          • all callsites to final methods of traits are emitted as invokestatic
          • of the above, those methods additionally marked @inline are inlined too

          In the example

          trait T {
            @inline final def m(x: Int) {
              println(x)
            }
          }
          class C {
            def host(y: T) {
              y.m(7)
            }
          }
          
          

          The resulting bytecode for host() is:

          public void host(T);
            Code:
             Stack=2, Locals=3, Args_size=2
             0:	bipush	7
             2:	istore_2
             3:	getstatic	#15; //Field scala/Predef$.MODULE$:Lscala/Predef$;
             6:	iload_2
             7:	invokestatic	#21; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
             10:	invokevirtual	#25; //Method scala/Predef$.println:(Ljava/lang/Object;)V
             13:	return
          

          Without the @inline annotation, the bytecode for host() looks like:

          public void host(T);
            Code:
             Stack=2, Locals=2, Args_size=2
             0:	aload_1
             1:	bipush	7
             3:	invokestatic	#15; //Method T$class.m:(LT;I)V
             6:	return
          

          With the current optimizer as we know the callsite to T.m(Int) is emitted as:

             3:	invokeinterface	#15,  2; //InterfaceMethod T.m:(I)V
          

          Here's how the new optimizer supports inlining of trait methods, documentation included:

          https://github.com/magarciaEPFL/scala/commit/dbf85bfef19e285daa191875338b70f0b64dc20e

          Show
          Miguel Garcia added a comment - To get a handle on this problem, I've implemented a fix in the experimental optimizer http://magarciaepfl.github.com/scala/ As a result: all callsites to final methods of traits are emitted as invokestatic of the above, those methods additionally marked @inline are inlined too In the example trait T { @inline final def m(x: Int) { println(x) } } class C { def host(y: T) { y.m(7) } } The resulting bytecode for host() is: public void host(T); Code: Stack=2, Locals=3, Args_size=2 0: bipush 7 2: istore_2 3: getstatic #15; //Field scala/Predef$.MODULE$:Lscala/Predef$; 6: iload_2 7: invokestatic #21; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer; 10: invokevirtual #25; //Method scala/Predef$.println:(Ljava/lang/Object;)V 13: return Without the @inline annotation, the bytecode for host() looks like: public void host(T); Code: Stack=2, Locals=2, Args_size=2 0: aload_1 1: bipush 7 3: invokestatic #15; //Method T$class.m:(LT;I)V 6: return With the current optimizer as we know the callsite to T.m(Int) is emitted as: 3: invokeinterface #15, 2; //InterfaceMethod T.m:(I)V Here's how the new optimizer supports inlining of trait methods, documentation included: https://github.com/magarciaEPFL/scala/commit/dbf85bfef19e285daa191875338b70f0b64dc20e
          Hide
          Adriaan Moors added a comment -

          It would be great if you could take the knowledge gained from this quest and push the current optimizer a little further down the right path.

          Show
          Adriaan Moors added a comment - It would be great if you could take the knowledge gained from this quest and push the current optimizer a little further down the right path.
          Hide
          Adriaan Moors added a comment -

          Un-assigning to foster work stealing, as announced in https://groups.google.com/forum/?fromgroups=#!topic/scala-internals/o8WG4plpNkw

          Show
          Adriaan Moors added a comment - Un-assigning to foster work stealing, as announced in https://groups.google.com/forum/?fromgroups=#!topic/scala-internals/o8WG4plpNkw
          Hide
          Adriaan Moors added a comment -

          Unassigning and rescheduling to M6 as previous deadline was missed.

          Show
          Adriaan Moors added a comment - Unassigning and rescheduling to M6 as previous deadline was missed.

            People

            • Assignee:
              Unassigned
              Reporter:
              Tiark Rompf
            • Votes:
              7 Vote for this issue
              Watchers:
              18 Start watching this issue

              Dates

              • Created:
                Updated:

                Development