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

Avoid redundant field in TermName, reducing size 40->32 bytes #8177

Merged
merged 1 commit into from Jun 26, 2019

Conversation

retronym
Copy link
Member

Before:

➜  scala git:(topic/name-waste) ✗ java -Djdk.attach.allowAttachSelf=true -cp $(coursier fetch -q -p 'org.openjdk.jol:jol-cli:0.9') org.openjdk.jol.Main internals -cp $(scala-classpath $(scala-ref-version 2.13.x)) 'scala.reflect.internal.Names$TermName'

Failed to find matching constructor, falling back to class-only introspection.

scala.reflect.internal.Names$TermName object internals:
 OFFSET  SIZE                                    TYPE DESCRIPTION                               VALUE
      0    12                                         (object header)                           N/A
     12     4                 scala.reflect.api.Names NameApi.$outer                            N/A
     16     4                                     int Name.index                                N/A
     20     4                                     int Name.len                                  N/A
     24     4                        java.lang.String Name.cachedString                         N/A
     28     4   scala.reflect.internal.Names.TermName TermName.next                             N/A
     32     4                        java.lang.String TermName.cachedString                     N/A
     36     4                                         (loss due to the next object alignment)
Instance size: 40 bytes
Space losses: 0 bytes internal + 4 bytes external = 4 bytes total

After:

➜  scala git:(topic/name-waste) ✗ java -Djdk.attach.allowAttachSelf=true -cp $(coursier fetch -q -p 'org.openjdk.jol:jol-cli:0.9') org.openjdk.jol.Main internals -cp build/quick/classes/reflect 'scala.reflect.internal.Names$TermName'

Failed to find matching constructor, falling back to class-only introspection.

scala.reflect.internal.Names$TermName object internals:
 OFFSET  SIZE                                    TYPE DESCRIPTION                               VALUE
      0    12                                         (object header)                           N/A
     12     4                 scala.reflect.api.Names NameApi.$outer                            N/A
     16     4                                     int Name.index                                N/A
     20     4                                     int Name.len                                  N/A
     24     4                        java.lang.String Name.cachedString                         N/A
     28     4   scala.reflect.internal.Names.TermName TermName.next                             N/A
Instance size: 32 bytes
Space losses: 0 bytes internal + 0 bytes external = 0 bytes total

Exposing Name.cachedString as a protected val makes it eligible for
the parameter aliasing layout optimization.

@scala-jenkins scala-jenkins added this to the 2.13.1 milestone Jun 26, 2019
Before:

```
➜  scala git:(topic/name-waste) ✗ java -Djdk.attach.allowAttachSelf=true -cp $(coursier fetch -q -p 'org.openjdk.jol:jol-cli:0.9') org.openjdk.jol.Main internals -cp $(scala-classpath $(scala-ref-version 2.13.x)) 'scala.reflect.internal.Names$TermName'

Failed to find matching constructor, falling back to class-only introspection.

scala.reflect.internal.Names$TermName object internals:
 OFFSET  SIZE                                    TYPE DESCRIPTION                               VALUE
      0    12                                         (object header)                           N/A
     12     4                 scala.reflect.api.Names NameApi.$outer                            N/A
     16     4                                     int Name.index                                N/A
     20     4                                     int Name.len                                  N/A
     24     4                        java.lang.String Name.cachedString                         N/A
     28     4   scala.reflect.internal.Names.TermName TermName.next                             N/A
     32     4                        java.lang.String TermName.cachedString                     N/A
     36     4                                         (loss due to the next object alignment)
Instance size: 40 bytes
Space losses: 0 bytes internal + 4 bytes external = 4 bytes total
```

After:

```
➜  scala git:(topic/name-waste) ✗ java -Djdk.attach.allowAttachSelf=true -cp $(coursier fetch -q -p 'org.openjdk.jol:jol-cli:0.9') org.openjdk.jol.Main internals -cp build/quick/classes/reflect 'scala.reflect.internal.Names$TermName'

Failed to find matching constructor, falling back to class-only introspection.

scala.reflect.internal.Names$TermName object internals:
 OFFSET  SIZE                                    TYPE DESCRIPTION                               VALUE
      0    12                                         (object header)                           N/A
     12     4                 scala.reflect.api.Names NameApi.$outer                            N/A
     16     4                                     int Name.index                                N/A
     20     4                                     int Name.len                                  N/A
     24     4                        java.lang.String Name.cachedString                         N/A
     28     4   scala.reflect.internal.Names.TermName TermName.next                             N/A
Instance size: 32 bytes
Space losses: 0 bytes internal + 0 bytes external = 0 bytes total
```

Exposing `Name.cachedString` as a protected val makes it eligible for
the parameter aliasing layout optimization.
@retronym retronym changed the base branch from 2.13.x to 2.12.x June 26, 2019 05:39
@retronym retronym modified the milestones: 2.13.1, 2.12.9 Jun 26, 2019
@retronym
Copy link
Member Author

Trivial change, merging without review.

@retronym retronym merged commit 58c13dd into scala:2.12.x Jun 26, 2019
@dwijnand
Copy link
Member

Where can I read up more about this? "parameter aliasing layout optimization" doesn't google me anything strictly relevant.

@retronym
Copy link
Member Author

retronym commented Jun 26, 2019

The compiler detects if a constructor parameter field can be elided because there is a field in a superclass that is known to store the same value.

The implementation is spread out between:

// For each class, we collect a mapping from constructor param accessors that are aliases of their superclass
// param accessors. At the end of the typer phase, when this information is available all the way up the superclass
// chain, this is used to determine which are true aliases, ones where the field can be elided from this class.
// And yes, if you were asking, this is yet another binary fragility, as we bake knowledge of the super class into
// this class.
private val superConstructorCalls: mutable.AnyRefMap[Symbol, collection.Map[Symbol, Symbol]] = perRunCaches.newAnyRefMap()

// associate superclass paramaccessors with their aliases
val (superConstr, superArgs) = decompose(rhs)
if (superConstr.symbol.isPrimaryConstructor) {
val superClazz = superConstr.symbol.owner
if (!superClazz.isJavaDefined) {
val superParamAccessors = superClazz.constrParamAccessors
if (sameLength(superParamAccessors, superArgs)) {
val accToSuperAcc = mutable.AnyRefMap[Symbol, Symbol]()
for ((superAcc, superArg@Ident(name)) <- superParamAccessors zip superArgs) {
if (mexists(vparamss)(_.symbol == superArg.symbol)) {
val ownAcc = clazz.info decl name suchThat (_.isParamAccessor) match {
case acc if !acc.isDeferred && acc.hasAccessorFlag => acc.accessed
case acc => acc
}
ownAcc match {
case acc: TermSymbol if !acc.isVariable && !isByNameParamType(acc.info) =>
accToSuperAcc(acc) = superAcc
case _ =>
}
}
}
if (!accToSuperAcc.isEmpty) {
superConstructorCalls(clazz) = accToSuperAcc
}
}
}
}

/** Finish computation of param aliases after typechecking is completed */
final def finishComputeParamAlias(): Unit = {
val classes = superConstructorCalls.keys.toArray
// superclasses before subclasses to avoid a data race between `superAcc.alias` and `acc.setAlias` below.
scala.util.Sorting.quickSort(classes)(Ordering.fromLessThan((a, b) => b.isLess(a)))
for (sym <- classes) {
for ((ownAcc, superAcc) <- superConstructorCalls.getOrElse(sym, Nil)) {
// We have a corresponding parameter in the super class.
val superClazz = sym.superClass
val alias = (
superAcc.initialize.alias // Is the param accessor is an alias for a field further up the class hierarchy?
orElse (superAcc getterIn superAcc.owner) // otherwise, lookup the accessor for the super
filter (alias => superClazz.info.nonPrivateMember(alias.name) == alias) // the accessor must be public
)
if (alias.exists && !alias.accessed.isVariable && !isRepeatedParamType(alias.accessed.info)) {
ownAcc match {
case acc: TermSymbol if !acc.isVariable && !isByNameParamType(acc.info) =>
debuglog(s"$acc has alias ${alias.fullLocationString}")
acc setAlias alias
case _ =>
}
}
}
}
superConstructorCalls.clear()
}
}

* (2) Converts references to parameter fields that have the same name as a corresponding
* public parameter field in a superclass to a reference to the superclass
* field (corresponding = super class field is initialized with subclass field).
* This info is pre-computed by the `alias` field in Typer. `dotc` follows a different
* route; it computes everything in SuperAccessors and changes the subclass field
* to a forwarder instead of manipulating references. This is more modular.

// Direct calls to aliases of param accessors to the superclass in order to avoid
// duplicating fields.
// ... but, only if accessible (scala/bug#6793)
if (sym.isParamAccessor && sym.alias != NoSymbol && isAccessibleFromSuper(sym.alias)) {
val result = (localTyper.typedPos(tree.pos) {
Select(Super(qual, tpnme.EMPTY) setPos qual.pos, sym.alias)
}).asInstanceOf[Select]
debuglog(s"alias replacement: $sym --> ${sym.alias} / $tree ==> $result"); //debug
localTyper.typed(gen.maybeMkAsInstanceOf(transformSuperSelect(result), sym.tpe, sym.alias.tpe, beforeRefChecks = true))

@dwijnand
Copy link
Member

dwijnand commented Jun 26, 2019

Ah right. I thought this was a JIT thing (silly, in retrospect).

Thanks!

@SethTisue SethTisue added the performance the need for speed. usually compiler performance, sometimes runtime performance. label Jul 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance the need for speed. usually compiler performance, sometimes runtime performance.
Projects
None yet
4 participants