summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Phillips <paulp@improving.org>2011-11-22 04:34:09 +0000
committerPaul Phillips <paulp@improving.org>2011-11-22 04:34:09 +0000
commite86f07fdd4d74f19d5206ded07739b4fa17957ad (patch)
treecbd19662b877cc3e41f54d07cb293ba7c5c6bd8e
parenteb0643210f2007775df3d116f237056ec5916874 (diff)
downloadscala-e86f07fdd4d74f19d5206ded07739b4fa17957ad.tar.gz
scala-e86f07fdd4d74f19d5206ded07739b4fa17957ad.tar.bz2
scala-e86f07fdd4d74f19d5206ded07739b4fa17957ad.zip
Long-standing performance mystery solved.
I noticed a long time ago that calls to def annotations in Symbols figured way, way too high in profiling output, but my earlier efforts to modify it failed because I didn't understand the "accidental" service it was supplying. Here is the key piece of the former implementation of annotations: - val annots1 = initialize.rawannots map { - case x: LazyAnnotationInfo => x.annot() - case x: AnnotationInfo => x - } filterNot (_.atp.isError) The first thing you might notice is that because it calls initialize, any call to either annotations or (more frequently) a method like "hasAnnotation" causes a symbol to be initialized. The upshot is that taking away tens of thousands of calls to initialize means a certain amount of "free lunch" is over. The second thing is that this implementation lead to the allocation of a new list on every call to annotations. 99.999% of the time it's the same elements in the list. The fact that rawannots is typed as a list of "AnnotationInfoBase" which may as well be AnyRef means you can't even use mapConserve, but even mapConserve would be an abuse of the garbage collector given how infrequently there is any change. So here's what we have now: 1) Annotations are delivered from trees to symbols by way of an externally positioned map, not a field on the symbol. It's done once. The only overhead on a call to annotations now is a null check. 2) I added a small sprinkling of calls to initialize in sensible locations. 3) The profiler impact is hard to believe, but this is reproducible. For whatever reason the non-profiler wall clock time impact is not as impressive. My profiling target was the compilation of these 15 files: src/library/scala/collection/generic/G*.scala Before this patch, heap usage peaked at 60MB. After, 35MB. 40% drop in profiler measured time elapsed. (Again, it's not like that outside the profiler.) About a 55% drop in number of allocations. About a 40% drop in total size of allocations. +----------------------+------------------+-----------------+-----------------+ | Name | Time Diff (ms) | Old Time (ms) | New Time (ms) | +----------------------+------------------+-----------------+-----------------+ | +---<All threads> | -19,569 | 52,496 | 32,926 | +----------------------+------------------+-----------------+-----------------+ +----------------------------+--------------------+-----------------------+ | Packages and Classes | Objects (+/-) | Size (+/-) | +----------------------------+--------------------+-----------------------+ | +---<Objects by classes> | -877,387 -56 % | -26,425,512 -37 % | | | | | | | +---char[] | -43,308 -2 % | -2,756,744 -3 % | | | | | | | +---java | -67,064 -3 % | -2,027,264 -2 % | | | | | | | +---scala | -745,099 -48 % | -19,021,760 -26 % | +----------------------------+--------------------+-----------------------+
-rw-r--r--src/compiler/scala/reflect/internal/AnnotationInfos.scala4
-rw-r--r--src/compiler/scala/reflect/internal/Symbols.scala56
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/Namers.scala9
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/Typers.scala18
4 files changed, 54 insertions, 33 deletions
diff --git a/src/compiler/scala/reflect/internal/AnnotationInfos.scala b/src/compiler/scala/reflect/internal/AnnotationInfos.scala
index c096d14b76..a86853b453 100644
--- a/src/compiler/scala/reflect/internal/AnnotationInfos.scala
+++ b/src/compiler/scala/reflect/internal/AnnotationInfos.scala
@@ -13,6 +13,10 @@ import pickling.ByteCodecs
trait AnnotationInfos extends api.AnnotationInfos { self: SymbolTable =>
import definitions.{ ThrowsClass, isMetaAnnotation }
+ // Annotations which are en route from Modifiers to a Symbol.
+ // They are removed from this map when the Symbol comes to claim them.
+ val pendingSymbolAnnotations = perRunCaches.newMap[Symbol, List[AnnotationInfoBase]]()
+
// Common annotation code between Symbol and Type.
// For methods altering the annotation list, on Symbol it mutates
// the Symbol's field directly. For Type, a new AnnotatedType is
diff --git a/src/compiler/scala/reflect/internal/Symbols.scala b/src/compiler/scala/reflect/internal/Symbols.scala
index 907e8e30ff..d744cff75a 100644
--- a/src/compiler/scala/reflect/internal/Symbols.scala
+++ b/src/compiler/scala/reflect/internal/Symbols.scala
@@ -1180,41 +1180,57 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
// ----- annotations ------------------------------------------------------------
- private var rawannots: List[AnnotationInfoBase] = Nil
- def rawAnnotations = rawannots
+ // null is a marker that they still need to be obtained.
+ private var _annotations: List[AnnotationInfo] = null
+ // Namer has stored the annotations waiting for us to come calling.
+ private def obtainAnnotations() {
+ // .initialize: the type completer of the symbol parses the annotations,
+ // see "def typeSig" in Namers.
+ initialize
+ _annotations = pendingSymbolAnnotations remove this match {
+ case Some(rawAnnots) =>
+ rawAnnots map {
+ case x: LazyAnnotationInfo => x.annot()
+ case x: AnnotationInfo => x
+ } filterNot (_.atp.isError)
+ case _ =>
+ Nil
+ }
+ }
+ // Gets _annotations without forcing initialization/obtainment.
+ def rawAnnotations = if (_annotations eq null) Nil else _annotations
+ // Used in namer to check whether annotations were already assigned or not.
+ def hasAssignedAnnotations = (_annotations ne null) && _annotations.nonEmpty
- /* Used in namer to check whether annotations were already assigned or not */
- def hasAssignedAnnotations = rawannots.nonEmpty
+ @deprecated("This method will be removed", "2.10.0")
+ def setRawAnnotations(annots: List[AnnotationInfoBase]): this.type = {
+ // Just in case this is still in use somewhere.
+ pendingSymbolAnnotations(this) = annots
+ _annotations = null
+ this
+ }
/** After the typer phase (before, look at the definition's Modifiers), contains
* the annotations attached to member a definition (class, method, type, field).
*/
def annotations: List[AnnotationInfo] = {
- // .initialize: the type completer of the symbol parses the annotations,
- // see "def typeSig" in Namers
- val annots1 = initialize.rawannots map {
- case x: LazyAnnotationInfo => x.annot()
- case x: AnnotationInfo => x
- } filterNot (_.atp.isError)
- rawannots = annots1
- annots1
+ if (_annotations eq null)
+ obtainAnnotations()
+ _annotations
}
-
- def setRawAnnotations(annots: List[AnnotationInfoBase]): this.type = {
- this.rawannots = annots
+ def setAnnotations(annots: List[AnnotationInfo]): this.type = {
+ _annotations = annots
this
}
- def setAnnotations(annots: List[AnnotationInfo]): this.type =
- setRawAnnotations(annots)
def withAnnotations(annots: List[AnnotationInfo]): this.type =
- setRawAnnotations(annots ::: rawannots)
+ setAnnotations(annots ::: rawAnnotations)
def withoutAnnotations: this.type =
- setRawAnnotations(Nil)
+ setAnnotations(Nil)
def addAnnotation(annot: AnnotationInfo): this.type =
- setRawAnnotations(annot :: rawannots)
+ setAnnotations(annot :: rawAnnotations)
// Convenience for the overwhelmingly common case
def addAnnotation(sym: Symbol, args: Tree*): this.type =
diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index fe474e5d3d..960919905d 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -1193,10 +1193,11 @@ trait Namers extends MethodSynthesis {
// need to be lazy, #1782
LazyAnnotationInfo(() => typer.typedAnnotation(ann))
}
- if (!ainfos.isEmpty)
- annotated.setRawAnnotations(ainfos)
- if (annotated.isTypeSkolem)
- annotated.deSkolemize.setRawAnnotations(ainfos)
+ if (ainfos.nonEmpty) {
+ pendingSymbolAnnotations(annotated) = ainfos
+ if (annotated.isTypeSkolem)
+ pendingSymbolAnnotations(annotated.deSkolemize) = ainfos
+ }
case _ =>
}
}
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index 41e0819fe2..b09309f056 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -1492,15 +1492,15 @@ trait Typers extends Modes with Adaptations with PatMatVirtualiser {
*/
def typedValDef(vdef: ValDef): ValDef = {
// attributes(vdef)
- val sym = vdef.symbol
+ val sym = vdef.symbol.initialize
val typer1 = constrTyperIf(sym.isParameter && sym.owner.isConstructor)
val typedMods = removeAnnotations(vdef.mods)
// complete lazy annotations
val annots = sym.annotations
-
var tpt1 = checkNoEscaping.privates(sym, typer1.typedType(vdef.tpt))
checkNonCyclic(vdef, tpt1)
+
if (sym.hasAnnotation(definitions.VolatileAttr)) {
if (!sym.isMutable)
error(vdef.pos, "values cannot be volatile")
@@ -1676,7 +1676,7 @@ trait Typers extends Modes with Adaptations with PatMatVirtualiser {
* @return ...
*/
def typedDefDef(ddef: DefDef): DefDef = {
- val meth = ddef.symbol
+ val meth = ddef.symbol.initialize
reenterTypeParams(ddef.tparams)
reenterValueParams(ddef.vparamss)
@@ -1753,7 +1753,7 @@ trait Typers extends Modes with Adaptations with PatMatVirtualiser {
private def typedTypeDef0(tdef: TypeDef): TypeDef = {
tdef.symbol.initialize
reenterTypeParams(tdef.tparams)
- val tparams1 = tdef.tparams mapConserve {typedTypeDef(_)}
+ val tparams1 = tdef.tparams mapConserve typedTypeDef
val typedMods = removeAnnotations(tdef.mods)
// complete lazy annotations
val annots = tdef.symbol.annotations
@@ -2693,11 +2693,11 @@ trait Typers extends Modes with Adaptations with PatMatVirtualiser {
error(arg.pos, "classfile annotation arguments have to be supplied as named arguments")
(nme.ERROR, None)
}
-
- for (name <- names) {
- if (!name.annotations.contains(AnnotationInfo(AnnotationDefaultAttr.tpe, List(), List())) &&
- !name.hasDefaultFlag)
- error(ann.pos, "annotation " + annType.typeSymbol.fullName + " is missing argument " + name.name)
+ for (sym <- names) {
+ // make sure the flags are up to date before erroring (jvm/t3415 fails otherwise)
+ sym.initialize
+ if (!sym.hasAnnotation(AnnotationDefaultAttr) && !sym.hasDefaultFlag)
+ error(ann.pos, "annotation " + annType.typeSymbol.fullName + " is missing argument " + sym.name)
}
if (hasError) annotationError