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.

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

{ final, regular } vals could be better optimized #4605

Closed
scabug opened this issue May 18, 2011 · 13 comments
Closed

{ final, regular } vals could be better optimized #4605

scabug opened this issue May 18, 2011 · 13 comments

Comments

@scabug
Copy link

scabug commented May 18, 2011

When I compile finalval.scala with content:

object A {
   final val c = 1234
}

an unused private attribute "private final int c;" in A$$.class is created (JD-GUI output):

A.class

import scala.reflect.ScalaSignature;

@ScalaSignature(bytes="\006\001\t:Q!\001\002\t\006\025\t\021!\021\006\002\007\0059A(Z7qift4\001\001\t\003\r\035i\021A\001\004\006\021\tA)!\003\002\002\003N\031qA\003\n\021\005-\001R\"\001\007\013\0055q\021\001\0027b]\036T\021aD\001\005U\0064\030-\003\002\022\031\t1qJ\0316fGR\004\"a\005\f\016\003QQ\021!F\001\006g\016\fG.Y\005\003/Q\0211bU2bY\006|%M[3di\")\021d\002C\0015\0051A(\0338jiz\"\022!\002\005\b9\035\021\r\021\"\002\036\003\005\031W#\001\020\020\003}i\"\001\002j\t\r\005:\001\025!\004\037\003\t\031\007\005")
public final class A
{
  public static final int c()
  {
    return A..MODULE$$.c();
  }
}

A$$.class

import scala.ScalaObject;

public final class A$$
  implements ScalaObject
{
  public static final  MODULE$$;
  private final int c;

  static
  {
    new ();
  }

  public final int c()
  {
    return 1234;
  }

  private A$$()
  {
    MODULE$$ = this;
  }
}

Scala version was 2.9.0.final
java compiler version was 1.6.0_18

@scabug
Copy link
Author

scabug commented May 18, 2011

Imported From: https://issues.scala-lang.org/browse/SI-4605?orig=1
Reporter: DaveScala (davescala)
See #6724

@scabug
Copy link
Author

scabug commented Jun 18, 2011

@paulp said (edited on Dec 16, 2011 7:52:04 AM UTC):
In the reported case, we could potentially avoid creating the field. In
addition there is the "final val vs. val" issue covered in the following
email excerpt from rex kerr.

Scala knows that it is not a mutable int because it's marked "val". And
furthermore it knows that it won't be overridden because it's in an
object or final class.

The JVM as far as I know doesn't have the level of effect tracking
needed to optimize things _annotated_ as final; there is not any
restriction on bytecode that actually prevents code from writing
multiple times to a "final" field (as far as I can tell from the spec).
In fact, Scala uses this ability when overriding vals--it doesn't
override the method, it instead overwrites the java-private-final value
of the field in the constructor.

So the Scala compiler is the only place where the knowledge exists that
this is a safe optimization.

You don't even have to give up the I-really-am-a-constant vs.
I-am-a-method-masquerading-as-a-constant thing since the JVM is
perfectly well able to elide methods that return a constant. The problem
is that Scala actually stores the value in a field, not that it uses a
method. So if you compare

  object Rng3 {
    val IA = 3877
    val IC = 29573
    var last = 42
    def next = { last = (last*IA + IC); last }
  }

  object Rng4 {
    final val IA = 3877
    final val IC = 29573
    var last = 42
    def next = { last = (last*IA + IC); last }
  }
  object Rng8 {
    def IA = 3877
    def IC = 29573
    var last = 42
    def next = { last = (last*IA + IC); last }
  }

you find that the performance is identical for 4 and 8. In the Rng4 case
the constants are pushed into the next method, but in Rng8 the code for
next is bytecode-identical to that for Rng3. The difference is

// Rng3 -- val
public int IA();
  Code:
   0:    aload_0
   1:    getfield    #20; //Field IA:I
   4:    ireturn

// Rng8 -- def
public int IA();
  Code:
   0:    sipush    3877
   3:    ireturn

and in the former case, the JVM is, as far as I know, powerless to elide
the field access.

@scabug
Copy link
Author

scabug commented Jul 1, 2011

@SethTisue said:
see also #4742

@scabug
Copy link
Author

scabug commented Jul 5, 2011

@paulp said:
Scala meeting: is there a reason we must generate the field? Not generating the field would cure this and #4742.

@scabug
Copy link
Author

scabug commented Jul 19, 2011

@VladUreche said:
Martin: We decided that Scala should do the same as Java, i.e. generate the field.

@scabug
Copy link
Author

scabug commented Jul 29, 2011

@paulp said:
OK, but there are other issues in this ticket.

@scabug
Copy link
Author

scabug commented Dec 16, 2011

@ijuma said:
By the way, there are two incorrect statements in Rex's email:

"it doesn't override the method, it instead overwrites the java-private-final value of the field in the constructor"

This is not true.

"The JVM as far as I know doesn't have the level of effect tracking
needed to optimize things annotated as final; there is not any
restriction on bytecode that actually prevents code from writing
multiple times to a "final" field (as far as I can tell from the spec)."

Not true again. David Homes, a HotSpot committer, says:

"So the compiler can optimize away re-loading of final fields, even if
reflection (or Unsafe) is mis-used. (Though compilation of deserialization
code would have to be handled specially.)"

Check the full explanation here:

http://cs.oswego.edu/pipermail/concurrency-interest/2011-January/007723.html

@scabug
Copy link
Author

scabug commented May 4, 2012

Chris Sachs (c9r) said:
This bug makes it impossible to expose a stable identifier in a class without creating a useless field. This limitation becomes particularly disheartening when dealing with small objects that implement an interface that exposes a stable identifier (e.g. a vector that exposes its vector space).

An alternative solution might be to permit @uncheckedStable to apply to defs (wouldn't that open up a nice can of worms?).

Please unburden our precious little stable identifiers from the crushing weight of gc-intensive java fields.

@scabug
Copy link
Author

scabug commented May 5, 2012

@paulp said:
I completely agree, it's a big problem. We need some form of stable def so we can put stable class data in the code segment where it belongs.

@scabug
Copy link
Author

scabug commented Jul 3, 2012

@adriaanm said:
the field is most likely for java interop -- what would javac do?

@scabug
Copy link
Author

scabug commented Jul 3, 2012

@paulp said:
What sort of interop? It's hard to see how it can involve java interop - we don't expose fields to java, we only expose getters. It's hard to draw a meaningful parallel to what java the source language does, because they don't privilege vals as stable identifiers the way that we do. In java there is little disincentive, if you wish to avoid a field, to making a final method which returns a constant value. In scala there is a huge disincentive, thus this ticket.

@scabug
Copy link
Author

scabug commented Jul 3, 2012

Chris Sachs (c9r) said:
To reinforce Paul's point about fields not serving a useful purpose for Java interop, remember that scalac already elides field definitions for abstract vals.

//  Test.java
public abstract class Test {
  public int iHaveAField;
}

Compiled from "Test.java"
public abstract class Test {
  public int iHaveAField;

  public Test();
    Code:
       0: aload_0       
       1: invokespecial #1                  // Method java/lang/Object."<init>":()V
       4: return        
}
// Test.scala
abstract class Test {
  val dudeWheresMyField: Int
}
Compiled from "Test.scala"
public abstract class Test {
  public abstract int dudeWheresMyField();

  public Test();
    Code:
       0: aload_0       
       1: invokespecial #12                 // Method java/lang/Object."<init>":()V
       4: return        
}

@SethTisue
Copy link
Member

2.12 does not generate the field

@SethTisue SethTisue modified the milestones: Backlog, 2.12.0 Mar 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants