Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8271623: Omit enclosing instance fields from inner classes that don't…
… use it

Reviewed-by: vromero, jlahoda
  • Loading branch information
cushon committed Nov 23, 2021
1 parent 0320672 commit ea85e01
Show file tree
Hide file tree
Showing 17 changed files with 429 additions and 55 deletions.
71 changes: 60 additions & 11 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
Expand Up @@ -98,6 +98,7 @@ public static Lower instance(Context context) {
private final boolean debugLower;
private final boolean disableProtectedAccessors; // experimental
private final PkgInfo pkginfoOpt;
private final boolean optimizeOuterThis;

protected Lower(Context context) {
context.put(lowerKey, this);
Expand All @@ -119,6 +120,9 @@ protected Lower(Context context) {
Options options = Options.instance(context);
debugLower = options.isSet("debuglower");
pkginfoOpt = PkgInfo.get(options);
optimizeOuterThis =
target.optimizeOuterThis() ||
options.getBoolean("optimizeOuterThis", false);
disableProtectedAccessors = options.isSet("disableProtectedAccessors");
}

Expand Down Expand Up @@ -1480,8 +1484,12 @@ Name outerThisName(Type type, Symbol owner) {

private VarSymbol makeOuterThisVarSymbol(Symbol owner, long flags) {
Type target = types.erasure(owner.enclClass().type.getEnclosingType());
// Set NOOUTERTHIS for all synthetic outer instance variables, and unset
// it when the variable is accessed. If the variable is never accessed,
// we skip creating an outer instance field and saving the constructor
// parameter to it.
VarSymbol outerThis =
new VarSymbol(flags, outerThisName(target, owner), target, owner);
new VarSymbol(flags | NOOUTERTHIS, outerThisName(target, owner), target, owner);
outerThisStack = outerThisStack.prepend(outerThis);
return outerThis;
}
Expand Down Expand Up @@ -1728,6 +1736,7 @@ JCExpression makeOuterThis(DiagnosticPosition pos, TypeSymbol c) {
}
VarSymbol ot = ots.head;
JCExpression tree = access(make.at(pos).Ident(ot));
ot.flags_field &= ~NOOUTERTHIS;
TypeSymbol otc = ot.type.tsym;
while (otc != c) {
do {
Expand All @@ -1745,6 +1754,7 @@ JCExpression makeOuterThis(DiagnosticPosition pos, TypeSymbol c) {
return makeNull();
}
tree = access(make.at(pos).Select(tree, ot));
ot.flags_field &= ~NOOUTERTHIS;
otc = ot.type.tsym;
}
return tree;
Expand Down Expand Up @@ -1784,6 +1794,7 @@ JCExpression makeOwnerThisN(DiagnosticPosition pos, Symbol sym, boolean preciseM
}
VarSymbol ot = ots.head;
JCExpression tree = access(make.at(pos).Ident(ot));
ot.flags_field &= ~NOOUTERTHIS;
TypeSymbol otc = ot.type.tsym;
while (!(preciseMatch ? sym.isMemberOf(otc, types) : otc.isSubClass(sym.owner, types))) {
do {
Expand All @@ -1796,6 +1807,7 @@ JCExpression makeOwnerThisN(DiagnosticPosition pos, Symbol sym, boolean preciseM
ot = ots.head;
} while (ot.owner != otc);
tree = access(make.at(pos).Select(tree, ot));
ot.flags_field &= ~NOOUTERTHIS;
otc = ot.type.tsym;
}
return tree;
Expand All @@ -1817,10 +1829,9 @@ JCStatement initField(int pos, Symbol rhs, Symbol lhs) {

/** Return tree simulating the assignment {@code this.this$n = this$n}.
*/
JCStatement initOuterThis(int pos) {
VarSymbol rhs = outerThisStack.head;
JCStatement initOuterThis(int pos, VarSymbol rhs) {
Assert.check(rhs.owner.kind == MTH);
VarSymbol lhs = outerThisStack.tail.head;
VarSymbol lhs = outerThisStack.head;
Assert.check(rhs.owner.owner == lhs.owner);
make.at(pos);
return
Expand Down Expand Up @@ -2224,15 +2235,25 @@ public void visitClassDef(JCClassDecl tree) {
// Convert name to flat representation, replacing '.' by '$'.
tree.name = Convert.shortName(currentClass.flatName());

// Add this$n and free variables proxy definitions to class.
// Add free variables proxy definitions to class.

for (List<JCVariableDecl> l = fvdefs; l.nonEmpty(); l = l.tail) {
tree.defs = tree.defs.prepend(l.head);
enterSynthetic(tree.pos(), l.head.sym, currentClass.members());
}
if (currentClass.hasOuterInstance()) {
// If this$n was accessed, add the field definition and
// update initial constructors to initialize it
if (currentClass.hasOuterInstance() && shouldEmitOuterThis(currentClass)) {
tree.defs = tree.defs.prepend(otdef);
enterSynthetic(tree.pos(), otdef.sym, currentClass.members());

for (JCTree def : tree.defs) {
if (TreeInfo.isInitialConstructor(def)) {
JCMethodDecl mdef = (JCMethodDecl) def;
mdef.body.stats = mdef.body.stats.prepend(
initOuterThis(mdef.body.pos, mdef.params.head.sym));
}
}
}

proxies = prevProxies;
Expand All @@ -2249,6 +2270,39 @@ public void visitClassDef(JCClassDecl tree) {
result = make_at(tree.pos()).Block(SYNTHETIC, List.nil());
}

private boolean shouldEmitOuterThis(ClassSymbol sym) {
if (!optimizeOuterThis) {
// Optimization is disabled
return true;
}
if ((outerThisStack.head.flags_field & NOOUTERTHIS) == 0) {
// Enclosing instance field is used
return true;
}
if (rs.isSerializable(sym.type) && !hasSerialVersionUID(sym)) {
// Class is serializable and does not have a stable serialVersionUID
return true;
}
return false;
}

private boolean hasSerialVersionUID(ClassSymbol sym) {
VarSymbol svuid = (VarSymbol) sym.members().findFirst(names.serialVersionUID, f -> f.kind == VAR);
if (svuid == null) {
return false;
}
if ((svuid.flags() & (STATIC | FINAL)) != (STATIC | FINAL)) {
return false;
}
if (!svuid.type.hasTag(LONG)) {
return false;
}
if (svuid.getConstValue() == null) {
return false;
}
return true;
}

List<JCTree> generateMandatedAccessors(JCClassDecl tree) {
List<JCVariableDecl> fields = TreeInfo.recordFields(tree);
return tree.sym.getRecordComponents().stream()
Expand Down Expand Up @@ -2703,11 +2757,6 @@ private void visitMethodDefInternal(JCMethodDecl tree) {
olderasure.getThrownTypes(),
syms.methodClass);
}
if (currentClass.hasOuterInstance() &&
TreeInfo.isInitialConstructor(tree))
{
added = added.prepend(initOuterThis(tree.body.pos));
}

// pop local variables from proxy stack
proxies = prevProxies;
Expand Down
Expand Up @@ -203,4 +203,11 @@ public boolean hasSealedClasses() {
public boolean obsoleteAccStrict() {
return compareTo(JDK1_17) >= 0;
}

/** Omit unused enclosing instance fields from inner classes that don't access enclosing
* instance state.
*/
public boolean optimizeOuterThis() {
return compareTo(JDK1_18) >= 0;
}
}
9 changes: 8 additions & 1 deletion test/langtools/tools/javac/6521805/T6521805d.java
Expand Up @@ -7,6 +7,8 @@
* @compile/fail/ref=T6521805d.out T6521805d.java -XDrawDiagnostics
*/

import java.util.Objects;

class T6521805 {

static class Inner extends T6521805.Outer {
Expand All @@ -22,6 +24,11 @@ public void foo() {
}
}

class Outer {}
class Outer {
{
// access enclosing instance so this$0 field is generated
Objects.requireNonNull(T6521805.this);
}
}

}
2 changes: 1 addition & 1 deletion test/langtools/tools/javac/6521805/T6521805d.out
@@ -1,2 +1,2 @@
T6521805d.java:18:18: compiler.err.cannot.generate.class: T6521805.Inner, (compiler.misc.synthetic.name.conflict: this$0, T6521805.Inner)
T6521805d.java:20:18: compiler.err.cannot.generate.class: T6521805.Inner, (compiler.misc.synthetic.name.conflict: this$0, T6521805.Inner)
1 error
9 changes: 8 additions & 1 deletion test/langtools/tools/javac/6521805/p/Outer.java
Expand Up @@ -2,6 +2,13 @@

package p;

import java.util.Objects;

class Outer {
class Super {}
class Super {
{
// access enclosing instance so this$0 field is generated
Objects.requireNonNull(Outer.this);
}
}
}
@@ -1,8 +1,6 @@

CLASSFILE MemberModifiers.c
--- SUPER
FIELD this$0
--- FINAL
METHOD <init>
---

Expand All @@ -20,17 +18,13 @@ METHOD m

CLASSFILE MemberModifiersAux.Foo.c
--- SUPER
FIELD this$1
--- FINAL
METHOD <init>
---

CLASSFILE MemberModifiersAux.Foo
--- FINAL SUPER
FIELD f
---
FIELD this$0
--- FINAL
METHOD <init>
---
METHOD m
Expand Down
Expand Up @@ -59,7 +59,7 @@ public static strictfp void main(String args[]) throws Exception {
.classes(classPath.toString())
.run()
.getOutput(Task.OutputKind.DIRECT);
if (!javapOut.contains("0: #22(): CLASS_EXTENDS, type_index=65535"))
if (!javapOut.contains("0: #20(): CLASS_EXTENDS, type_index=65535"))
throw new AssertionError("Expected output missing: " + javapOut);
}
}
}
Expand Up @@ -44,14 +44,11 @@
"<init>(AccessToPrivateInnerClassConstructorsTest)",
"<init>(AccessToPrivateInnerClassConstructorsTest, " +
"AccessToPrivateInnerClassConstructorsTest$1)"},
expectedNumberOfSyntheticFields = 1,
expectedNumberOfSyntheticMethods = 0)
@ExpectedClass(className = "AccessToPrivateInnerClassConstructorsTest$1Local",
expectedMethods = {"<init>(AccessToPrivateInnerClassConstructorsTest)"},
expectedNumberOfSyntheticFields = 1)
expectedMethods = {"<init>(AccessToPrivateInnerClassConstructorsTest)"})
@ExpectedClass(className = "AccessToPrivateInnerClassConstructorsTest$2Local",
expectedMethods = {"<init>(AccessToPrivateInnerClassConstructorsTest)"},
expectedNumberOfSyntheticFields = 1)
expectedMethods = {"<init>(AccessToPrivateInnerClassConstructorsTest)"})
public class AccessToPrivateInnerClassConstructorsTest {

public static void main(String... args) {
Expand Down
Expand Up @@ -43,15 +43,13 @@
* 3. access method for private method function().
* 4. getter/setter for private field staticVar.
* 5. access method for private method staticFunction().
* 6. field this in Inner1.
* 7. constructor for Inner*.
* 6. constructor for Inner*.
*/
@ExpectedClass(className = "AccessToPrivateInnerClassMembersTest",
expectedMethods = {"<init>()", "<clinit>()"})
@ExpectedClass(className = "AccessToPrivateInnerClassMembersTest$Inner1",
expectedMethods = {"<init>(AccessToPrivateInnerClassMembersTest)", "function()"},
expectedFields = "var",
expectedNumberOfSyntheticFields = 1)
expectedFields = "var")
@ExpectedClass(className = "AccessToPrivateInnerClassMembersTest$Inner2",
expectedMethods = {"function()", "staticFunction()", "<init>()"},
expectedFields = {"staticVar", "var"})
Expand Down
Expand Up @@ -43,14 +43,12 @@
* 3. access method for private method function().
* 4. getter/setter for private field staticVar.
* 5. access method for private method staticFunction().
* 6. field this in Inner1.
* 7. constructor for Inner*.
* 6. constructor for Inner*.
*/
@ExpectedClass(className = "AccessToPrivateSiblingsTest", expectedMethods = "<init>()")
@ExpectedClass(className = "AccessToPrivateSiblingsTest$Inner1",
expectedMethods = {"function()", "<init>(AccessToPrivateSiblingsTest)"},
expectedFields = "var",
expectedNumberOfSyntheticFields = 1)
expectedFields = "var")
@ExpectedClass(className = "AccessToPrivateSiblingsTest$Inner2",
expectedMethods = "<init>(AccessToPrivateSiblingsTest)",
expectedNumberOfSyntheticFields = 1)
Expand Down
Expand Up @@ -55,19 +55,16 @@
@ExpectedClass(className = "BridgeMethodsForLambdaTest$Inner1",
expectedMethods = {"<init>(BridgeMethodsForLambdaTest)", "function()", "run()"},
expectedFields = "lambda1",
expectedNumberOfSyntheticMethods = 1,
expectedNumberOfSyntheticFields = 1)
expectedNumberOfSyntheticMethods = 1)
@ExpectedClass(className = "BridgeMethodsForLambdaTest$Inner2",
expectedMethods = {"<init>()", "staticFunction()"},
expectedFields = "lambda1",
expectedNumberOfSyntheticMethods = 1)
@ExpectedClass(className = "BridgeMethodsForLambdaTest$Inner3",
expectedMethods = {"<init>(BridgeMethodsForLambdaTest)", "function()"},
expectedNumberOfSyntheticFields = 1)
expectedMethods = {"<init>(BridgeMethodsForLambdaTest)", "function()"})
@ExpectedClass(className = "BridgeMethodsForLambdaTest$Inner4",
expectedMethods = {"<init>(BridgeMethodsForLambdaTest)", "function()"},
expectedNumberOfSyntheticMethods = 1,
expectedNumberOfSyntheticFields = 1)
expectedNumberOfSyntheticMethods = 1)
public class BridgeMethodsForLambdaTest {

private class Inner1 implements Runnable {
Expand Down
Expand Up @@ -34,6 +34,8 @@
* @run main SyntheticTestDriver ThisFieldTest
*/

import java.util.Objects;

/**
* Synthetic members:
* 1. fields this$0 for local and anonymous classes.
Expand All @@ -49,9 +51,17 @@
public class ThisFieldTest {
{
class Local {
{
// access enclosing instance so this$0 field is generated
Objects.requireNonNull(ThisFieldTest.this);
}
}

new Local() {
{
// access enclosing instance so this$0 field is generated
Objects.requireNonNull(ThisFieldTest.this);
}
};
}
}
Expand Up @@ -24,11 +24,18 @@
// key: compiler.err.cannot.generate.class
// key: compiler.misc.synthetic.name.conflict

import java.util.Objects;

class ErrSyntheticNameConflict {

static class Outer {
ErrSyntheticNameConflict this$0 = null;
}

public class Inner extends Outer { }
public class Inner extends Outer {
{
// access enclosing instance so this$0 field is generated
Objects.requireNonNull(ErrSyntheticNameConflict.this);
}
}
}

1 comment on commit ea85e01

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.