diff options
author | Paul Phillips <paulp@improving.org> | 2011-11-22 04:34:09 +0000 |
---|---|---|
committer | Paul Phillips <paulp@improving.org> | 2011-11-22 04:34:09 +0000 |
commit | e86f07fdd4d74f19d5206ded07739b4fa17957ad (patch) | |
tree | cbd19662b877cc3e41f54d07cb293ba7c5c6bd8e /src/compiler | |
parent | eb0643210f2007775df3d116f237056ec5916874 (diff) | |
download | scala-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 % |
+----------------------------+--------------------+-----------------------+
Diffstat (limited to 'src/compiler')
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 |