Skip to content

Performance improvements: avoid boxing and needless string to name conversion #194

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions src/main/scala/scala/async/internal/AnfTransform.scala
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ private[async] trait AnfTransform {
stats :+ expr :+ api.typecheck(atPos(expr.pos)(Throw(Apply(Select(New(gen.mkAttributedRef(defn.IllegalStateExceptionClass)), nme.CONSTRUCTOR), Nil))))
expr match {
case Apply(fun, args) if isAwait(fun) =>
val valDef = defineVal(name.await, expr, tree.pos)
val valDef = defineVal(name.await(), expr, tree.pos)
val ref = gen.mkAttributedStableRef(valDef.symbol).setType(tree.tpe)
val ref1 = if (ref.tpe =:= definitions.UnitTpe)
// https://github.com/scala/async/issues/74
Expand Down Expand Up @@ -109,7 +109,7 @@ private[async] trait AnfTransform {
} else if (expr.tpe =:= definitions.NothingTpe) {
statsExprThrow
} else {
val varDef = defineVar(name.ifRes, expr.tpe, tree.pos)
val varDef = defineVar(name.ifRes(), expr.tpe, tree.pos)
def typedAssign(lhs: Tree) =
api.typecheck(atPos(lhs.pos)(Assign(Ident(varDef.symbol), mkAttributedCastPreservingAnnotations(lhs, tpe(varDef.symbol)))))

Expand Down Expand Up @@ -140,7 +140,7 @@ private[async] trait AnfTransform {
} else if (expr.tpe =:= definitions.NothingTpe) {
statsExprThrow
} else {
val varDef = defineVar(name.matchRes, expr.tpe, tree.pos)
val varDef = defineVar(name.matchRes(), expr.tpe, tree.pos)
def typedAssign(lhs: Tree) =
api.typecheck(atPos(lhs.pos)(Assign(Ident(varDef.symbol), mkAttributedCastPreservingAnnotations(lhs, tpe(varDef.symbol)))))
val casesWithAssign = cases map {
Expand All @@ -163,14 +163,14 @@ private[async] trait AnfTransform {
}
}

def defineVar(prefix: String, tp: Type, pos: Position): ValDef = {
val sym = api.currentOwner.newTermSymbol(name.fresh(prefix), pos, MUTABLE | SYNTHETIC).setInfo(uncheckedBounds(tp))
def defineVar(name: TermName, tp: Type, pos: Position): ValDef = {
val sym = api.currentOwner.newTermSymbol(name, pos, MUTABLE | SYNTHETIC).setInfo(uncheckedBounds(tp))
valDef(sym, mkZero(uncheckedBounds(tp))).setType(NoType).setPos(pos)
}
}

def defineVal(prefix: String, lhs: Tree, pos: Position): ValDef = {
val sym = api.currentOwner.newTermSymbol(name.fresh(prefix), pos, SYNTHETIC).setInfo(uncheckedBounds(lhs.tpe))
def defineVal(name: TermName, lhs: Tree, pos: Position): ValDef = {
val sym = api.currentOwner.newTermSymbol(name, pos, SYNTHETIC).setInfo(uncheckedBounds(lhs.tpe))
internal.valDef(sym, internal.changeOwner(lhs, api.currentOwner, sym)).setType(NoType).setPos(pos)
}

Expand Down Expand Up @@ -212,7 +212,7 @@ private[async] trait AnfTransform {
case Arg(expr, _, argName) =>
linearize.transformToList(expr) match {
case stats :+ expr1 =>
val valDef = defineVal(argName, expr1, expr1.pos)
val valDef = defineVal(name.freshen(argName), expr1, expr1.pos)
require(valDef.tpe != null, valDef)
val stats1 = stats :+ valDef
(stats1, atPos(tree.pos.makeTransparent)(gen.stabilize(gen.mkAttributedIdent(valDef.symbol))))
Expand Down Expand Up @@ -279,8 +279,9 @@ private[async] trait AnfTransform {
// TODO we can move this into ExprBuilder once we get rid of `AsyncDefinitionUseAnalyzer`.
val block = linearize.transformToBlock(body)
val (valDefs, mappings) = (pat collect {
case b@Bind(name, _) =>
val vd = defineVal(name.toTermName + AnfTransform.this.name.bindSuffix, gen.mkAttributedStableRef(b.symbol).setPos(b.pos), b.pos)
case b@Bind(bindName, _) =>
val vd = defineVal(name.freshen(bindName.toTermName), gen.mkAttributedStableRef(b.symbol).setPos(b.pos), b.pos)
vd.symbol.updateAttachment(SyntheticBindVal)
(vd, (b.symbol, vd.symbol))
}).unzip
val (from, to) = mappings.unzip
Expand Down Expand Up @@ -333,7 +334,7 @@ private[async] trait AnfTransform {
// Otherwise, create the matchres var. We'll callers of the label def below.
// Remember: we're iterating through the statement sequence in reverse, so we'll get
// to the LabelDef and mutate `matchResults` before we'll get to its callers.
val matchResult = linearize.defineVar(name.matchRes, param.tpe, ld.pos)
val matchResult = linearize.defineVar(name.matchRes(), param.tpe, ld.pos)
matchResults += matchResult
caseDefToMatchResult(ld.symbol) = matchResult.symbol
val rhs2 = ld.rhs.substituteSymbols(param.symbol :: Nil, matchResult.symbol :: Nil)
Expand Down Expand Up @@ -408,3 +409,5 @@ private[async] trait AnfTransform {
}).asInstanceOf[Block]
}
}

object SyntheticBindVal
10 changes: 10 additions & 0 deletions src/main/scala/scala/async/internal/AsyncMacro.scala
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
package scala.async.internal

import java.util

object AsyncMacro {
private val nameCache = new util.WeakHashMap[Object, AsyncNames[_]]()
def apply(c0: reflect.macros.Context, base: AsyncBase)(body0: c0.Tree): AsyncMacro { val c: c0.type } = {
import language.reflectiveCalls
val asyncNames0 = nameCache.synchronized[AsyncNames[_]] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why synchronized + computeIfAbsent?

Copy link
Member Author

Choose a reason for hiding this comment

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

I got rid of this map altogether #194 as I think this was leaky anyway.

nameCache.computeIfAbsent(c0.universe, new java.util.function.Function[Object, AsyncNames[_]] {
override def apply(t: Object): AsyncNames[_] = new AsyncNames[c0.universe.type](c0.universe)
})
}
new AsyncMacro { self =>
val c: c0.type = c0
val asyncNames: AsyncNames[c.universe.type] = asyncNames0.asInstanceOf[AsyncNames[c.universe.type]]
val body: c.Tree = body0
// This member is required by `AsyncTransform`:
val asyncBase: AsyncBase = base
Expand All @@ -23,6 +32,7 @@ private[async] trait AsyncMacro
val c: scala.reflect.macros.Context
val body: c.Tree
var containsAwait: c.Tree => Boolean
val asyncNames: AsyncNames[c.universe.type]

lazy val macroPos: c.universe.Position = c.macroApplication.pos.makeTransparent
def atMacroPos(t: c.Tree): c.Tree = c.universe.atPos(macroPos)(t)
Expand Down
109 changes: 109 additions & 0 deletions src/main/scala/scala/async/internal/AsyncNames.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package scala.async.internal

import java.util.concurrent.atomic.AtomicInteger

import scala.collection.mutable
import scala.collection.mutable.ArrayBuffer
import scala.reflect.api.Names

/**
* A per-global cache of names needed by the Async macro.
*/
final class AsyncNames[U <: Names with Singleton](val u: U) {
self =>
import u._

abstract class NameCache[N <: U#Name](base: String) {
val cached = new ArrayBuffer[N]()
protected def newName(s: String): N
def apply(i: Int): N = {
if (cached.isDefinedAt(i)) cached(i)
else {
assert(cached.length == i)
val name = newName(freshenString(base, i))
cached += name
name
}
}
}

final class TermNameCache(base: String) extends NameCache[U#TermName](base) {
override protected def newName(s: String): U#TermName = newTermName(s)
}
final class TypeNameCache(base: String) extends NameCache[U#TypeName](base) {
override protected def newName(s: String): U#TypeName = newTypeName(s)
}
private val matchRes: TermNameCache = new TermNameCache("match")
private val ifRes: TermNameCache = new TermNameCache("if")
private val await: TermNameCache = new TermNameCache("await")

private val resume = newTermName("resume")
private val completed: TermName = newTermName("completed$async")
private val apply = newTermName("apply")
private val stateMachine = newTermName("stateMachine$async")
private val stateMachineT = stateMachine.toTypeName
private val state: u.TermName = newTermName("state$async")
private val execContext = newTermName("execContext$async")
private val tr: u.TermName = newTermName("tr$async")
private val t: u.TermName = newTermName("throwable$async")

final class NameSource[N <: U#Name](cache: NameCache[N]) {
private val count = new AtomicInteger(0)
def apply(): N = cache(count.getAndIncrement())
}

class AsyncName {
final val matchRes = new NameSource[U#TermName](self.matchRes)
final val ifRes = new NameSource[U#TermName](self.matchRes)
final val await = new NameSource[U#TermName](self.await)
final val completed = self.completed
final val result = self.resume
final val apply = self.apply
final val stateMachine = self.stateMachine
final val stateMachineT = self.stateMachineT
final val state: u.TermName = self.state
final val execContext = self.execContext
final val tr: u.TermName = self.tr
final val t: u.TermName = self.t

private val seenPrefixes = mutable.AnyRefMap[Name, AtomicInteger]()
private val freshened = mutable.HashSet[Name]()

final def freshenIfNeeded(name: TermName): TermName = {
seenPrefixes.getOrNull(name) match {
case null =>
seenPrefixes.put(name, new AtomicInteger())
name
case counter =>
freshen(name, counter)
}
}
final def freshenIfNeeded(name: TypeName): TypeName = {
seenPrefixes.getOrNull(name) match {
case null =>
seenPrefixes.put(name, new AtomicInteger())
name
case counter =>
freshen(name, counter)
}
}
final def freshen(name: TermName): TermName = {
val counter = seenPrefixes.getOrElseUpdate(name, new AtomicInteger())
freshen(name, counter)
}
final def freshen(name: TypeName): TypeName = {
val counter = seenPrefixes.getOrElseUpdate(name, new AtomicInteger())
freshen(name, counter)
}
private def freshen(name: TermName, counter: AtomicInteger): TermName = {
if (freshened.contains(name)) name
else TermName(freshenString(name.toString, counter.incrementAndGet()))
}
private def freshen(name: TypeName, counter: AtomicInteger): TypeName = {
if (freshened.contains(name)) name
else TypeName(freshenString(name.toString, counter.incrementAndGet()))
}
}

private def freshenString(name: String, counter: Int): String = name.toString + "$async$" + counter
}
29 changes: 17 additions & 12 deletions src/main/scala/scala/async/internal/ExprBuilder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
*/
package scala.async.internal

import java.util.function.IntUnaryOperator

import scala.collection.mutable
import scala.collection.mutable.ListBuffer
import language.existentials
Expand All @@ -23,7 +25,7 @@ trait ExprBuilder {
trait AsyncState {
def state: Int

def nextStates: List[Int]
def nextStates: Array[Int]

def mkHandlerCaseForState[T: WeakTypeTag]: CaseDef

Expand Down Expand Up @@ -55,8 +57,8 @@ trait ExprBuilder {
final class SimpleAsyncState(var stats: List[Tree], val state: Int, nextState: Int, symLookup: SymLookup)
extends AsyncState {

def nextStates: List[Int] =
List(nextState)
val nextStates: Array[Int] =
Array(nextState)

def mkHandlerCaseForState[T: WeakTypeTag]: CaseDef = {
mkHandlerCase(state, treesThenStats(mkStateTree(nextState, symLookup) :: Nil))
Expand All @@ -69,7 +71,7 @@ trait ExprBuilder {
/** A sequence of statements with a conditional transition to the next state, which will represent
* a branch of an `if` or a `match`.
*/
final class AsyncStateWithoutAwait(var stats: List[Tree], val state: Int, val nextStates: List[Int]) extends AsyncState {
final class AsyncStateWithoutAwait(var stats: List[Tree], val state: Int, val nextStates: Array[Int]) extends AsyncState {
override def mkHandlerCaseForState[T: WeakTypeTag]: CaseDef =
mkHandlerCase(state, stats)

Expand All @@ -84,16 +86,16 @@ trait ExprBuilder {
val awaitable: Awaitable, symLookup: SymLookup)
extends AsyncState {

def nextStates: List[Int] =
List(nextState)
val nextStates: Array[Int] =
Array(nextState)

override def mkHandlerCaseForState[T: WeakTypeTag]: CaseDef = {
val fun = This(tpnme.EMPTY)
val callOnComplete = futureSystemOps.onComplete[Any, Unit](c.Expr[futureSystem.Fut[Any]](awaitable.expr),
c.Expr[futureSystem.Tryy[Any] => Unit](fun), c.Expr[futureSystem.ExecContext](Ident(name.execContext))).tree
val tryGetOrCallOnComplete: List[Tree] =
if (futureSystemOps.continueCompletedFutureOnSameThread) {
val tempName = name.fresh(name.completed)
val tempName = name.completed
val initTemp = ValDef(NoMods, tempName, TypeTree(futureSystemOps.tryType[Any]), futureSystemOps.getCompleted[Any](c.Expr[futureSystem.Fut[Any]](awaitable.expr)).tree)
val ifTree = If(Apply(Select(Literal(Constant(null)), TermName("ne")), Ident(tempName) :: Nil),
adaptToUnit(ifIsFailureTree[T](Ident(tempName)) :: Nil),
Expand Down Expand Up @@ -191,7 +193,7 @@ trait ExprBuilder {
def resultWithIf(condTree: Tree, thenState: Int, elseState: Int): AsyncState = {
def mkBranch(state: Int) = mkStateTree(state, symLookup)
this += If(condTree, mkBranch(thenState), mkBranch(elseState))
new AsyncStateWithoutAwait(stats.toList, state, List(thenState, elseState))
new AsyncStateWithoutAwait(stats.toList, state, Array(thenState, elseState))
}

/**
Expand All @@ -204,7 +206,7 @@ trait ExprBuilder {
* @param caseStates starting state of the right-hand side of the each case
* @return an `AsyncState` representing the match expression
*/
def resultWithMatch(scrutTree: Tree, cases: List[CaseDef], caseStates: List[Int], symLookup: SymLookup): AsyncState = {
def resultWithMatch(scrutTree: Tree, cases: List[CaseDef], caseStates: Array[Int], symLookup: SymLookup): AsyncState = {
// 1. build list of changed cases
val newCases = for ((cas, num) <- cases.zipWithIndex) yield cas match {
case CaseDef(pat, guard, rhs) =>
Expand All @@ -218,7 +220,7 @@ trait ExprBuilder {

def resultWithLabel(startLabelState: Int, symLookup: SymLookup): AsyncState = {
this += mkStateTree(startLabelState, symLookup)
new AsyncStateWithoutAwait(stats.toList, state, List(startLabelState))
new AsyncStateWithoutAwait(stats.toList, state, Array(startLabelState))
}

override def toString: String = {
Expand Down Expand Up @@ -299,7 +301,10 @@ trait ExprBuilder {
case Match(scrutinee, cases) if containsAwait(stat) =>
checkForUnsupportedAwait(scrutinee)

val caseStates = cases.map(_ => nextState())
val caseStates = new Array[Int](cases.length)
java.util.Arrays.setAll(caseStates, new IntUnaryOperator {
override def applyAsInt(operand: Int): Int = nextState()
})
val afterMatchState = nextState()

asyncStates +=
Expand Down Expand Up @@ -515,7 +520,7 @@ trait ExprBuilder {
}

private def isSyntheticBindVal(tree: Tree) = tree match {
case vd@ValDef(_, lname, _, Ident(rname)) => lname.toString.contains(name.bindSuffix)
case vd@ValDef(_, lname, _, Ident(rname)) => attachments(vd.symbol).contains[SyntheticBindVal.type]
case _ => false
}

Expand Down
1 change: 1 addition & 0 deletions src/main/scala/scala/async/internal/FutureSystem.scala
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ trait FutureSystem {

def mkOps(c0: Context): Ops { val c: c0.type }

@deprecated("No longer honoured by the macro, all generated names now contain $async to avoid accidental clashes with lambda lifted names", "0.9.7")
def freshenAllNames: Boolean = false
def emitTryCatch: Boolean = true
def resultFieldName: String = "result"
Expand Down
8 changes: 4 additions & 4 deletions src/main/scala/scala/async/internal/Lifter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,13 @@ trait Lifter {
val rhs1 = if (sym.asTerm.isLazy) rhs else EmptyTree
treeCopy.ValDef(vd, Modifiers(sym.flags), sym.name, TypeTree(tpe(sym)).setPos(t.pos), rhs1)
case dd@DefDef(_, _, tparams, vparamss, tpt, rhs) =>
sym.setName(this.name.fresh(sym.name.toTermName))
sym.setName(this.name.freshen(sym.name.toTermName))
sym.setFlag(PRIVATE | LOCAL)
// Was `DefDef(sym, rhs)`, but this ran afoul of `ToughTypeSpec.nestedMethodWithInconsistencyTreeAndInfoParamSymbols`
// due to the handling of type parameter skolems in `thisMethodType` in `Namers`
treeCopy.DefDef(dd, Modifiers(sym.flags), sym.name, tparams, vparamss, tpt, rhs)
case cd@ClassDef(_, _, tparams, impl) =>
sym.setName(newTypeName(name.fresh(sym.name.toString).toString))
sym.setName(name.freshen(sym.name.toTypeName))
companionship.companionOf(cd.symbol) match {
case NoSymbol =>
case moduleSymbol =>
Expand All @@ -137,13 +137,13 @@ trait Lifter {
case md@ModuleDef(_, _, impl) =>
companionship.companionOf(md.symbol) match {
case NoSymbol =>
sym.setName(name.fresh(sym.name.toTermName))
sym.setName(name.freshen(sym.name.toTermName))
sym.asModule.moduleClass.setName(sym.name.toTypeName)
case classSymbol => // will be renamed by `case ClassDef` above.
}
treeCopy.ModuleDef(md, Modifiers(sym.flags), sym.name, impl)
case td@TypeDef(_, _, tparams, rhs) =>
sym.setName(newTypeName(name.fresh(sym.name.toString).toString))
sym.setName(name.freshen(sym.name.toTypeName))
treeCopy.TypeDef(td, Modifiers(sym.flags), sym.name, tparams, rhs)
}
atPos(t.pos)(treeLifted)
Expand Down
Loading