From 94a00c31680623cd2793b0db87c2bcfac10c9563 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Mon, 27 Jun 2011 01:14:08 +0000 Subject: Profiling revealed a suspiciously heavy consume... Profiling revealed a suspiciously heavy consumer of CPU time during refchecks called checkOverloadedRestrictions. Turns out it was looking for overloads where more than one method defined default arguments, but with N^2 carved on its checking stick. Time spent in checkOverloadedRestrictions in quick.lib drops from 5800ms to 300ms. Review by rytz. --- .../scala/tools/nsc/typechecker/RefChecks.scala | 45 +++++++++++++--------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 6ff1302888..e842a667aa 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -83,32 +83,39 @@ abstract class RefChecks extends InfoTransform { var checkedCombinations = Set[List[Type]]() // only one overloaded alternative is allowed to define default arguments - private def checkOverloadedRestrictions(clazz: Symbol) { - def check(members: List[Symbol]): Unit = members match { - case x :: xs => - if (x.hasParamWhich(_.hasDefaultFlag) && !nme.isProtectedAccessorName(x.name)) { - val others = xs.filter(alt => { - alt.name == x.name && - (alt hasParamWhich (_.hasDefaultFlag)) && - (!alt.isConstructor || alt.owner == x.owner) // constructors of different classes are allowed to have defaults - }) - if (!others.isEmpty) { - val all = x :: others - val rest = if (all.exists(_.owner != clazz)) ".\nThe members with defaults are defined in "+ - all.map(_.owner).mkString("", " and ", ".") else "." - unit.error(clazz.pos, "in "+ clazz +", multiple overloaded alternatives of "+ x + - " define default arguments"+ rest) - } + private def checkOverloadedRestrictions(clazz: Symbol): Unit = { + // Using the default getters (such as methodName$default$1) as a cheap way of + // finding methods with default parameters. This way, we can limit the members to + // those with the DEFAULTPARAM flag, and infer the methods. Looking for the methods + // directly requires inspecting the parameter list of every one. That modification + // shaved 95% off the time spent in this method. + val defaultGetters = clazz.info.findMember(nme.ANYNAME, 0L, DEFAULTPARAM, false).alternatives + val defaultMethodNames = defaultGetters map (sym => nme.defaultGetterToMethod(sym.name)) + + defaultMethodNames.distinct foreach { name => + val methods = clazz.info.findMember(name, 0L, METHOD, false).alternatives + val haveDefaults = methods filter (sym => sym.hasParamWhich(_.hasDefaultFlag) && !nme.isProtectedAccessorName(sym.name)) + + if (haveDefaults.lengthCompare(1) > 0) { + val owners = haveDefaults map (_.owner) + // constructors of different classes are allowed to have defaults + if (haveDefaults.exists(x => !x.isConstructor) || owners.distinct.size < haveDefaults.size) { + unit.error(clazz.pos, + "in "+ clazz + + ", multiple overloaded alternatives of "+ haveDefaults.head + + " define default arguments" + ( + if (owners.forall(_ == clazz)) "." + else ".\nThe members with defaults are defined in "+owners.map(_.fullLocationString).mkString("", " and ", ".") + ) + ) } - check(xs) - case _ => () + } } clazz.info.decls filter (x => x.isImplicit && x.typeParams.nonEmpty) foreach { sym => val alts = clazz.info.decl(sym.name).alternatives if (alts.size > 1) alts foreach (x => unit.warning(x.pos, "parameterized overloaded implicit methods are not visible as view bounds")) } - check(clazz.info.members) } // Override checking ------------------------------------------------------------ -- cgit v1.2.3