diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2014-05-21 10:40:13 +0200 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2014-05-26 17:31:26 +0200 |
commit | 4aa10a1f6fa803159638355ef9c2aa968eb16556 (patch) | |
tree | 9b75da5c4a6194f3324313099fe31e94045c2ead | |
parent | a6714f8fee2f8df9bc9d4760ed4c0c799f4f2c11 (diff) | |
download | scala-4aa10a1f6fa803159638355ef9c2aa968eb16556.tar.gz scala-4aa10a1f6fa803159638355ef9c2aa968eb16556.tar.bz2 scala-4aa10a1f6fa803159638355ef9c2aa968eb16556.zip |
Optimize nested scope creation
We can copy the hash table from the parent scope, rather
than constructing it from scratch.
This takes us to:
% rm -rf /tmp/pkg; (for i in {1..50}; do for j in {1..100}; do echo "package pkg { class A_${i}_${j}___(val a: Int, val b: Int) }"; done; done) > sandbox/A1.scala && time qbin/scalac -Ybackend:GenASM -J-Xmx1G -J-XX:MaxPermSize=400M -d /tmp sandbox/A1.scala;
real 0m19.639s // head~1 was 0m35.662s
user 0m41.683s // head~1 was 0m58.275s
sys 0m1.886s
In more detail, this commit:
- Removes the unused `fingerprint` constructor parameter from
scopes. This is a remnant from a previous optimization attempt
- Leave only one constructor on Scope which creates an empty
scope
- Update the factory method, `newNestedScope`, to copy the hash
table from the parent if it exists. We can rely on the invariant
that `outer.hashTable != null || outer.size < MIN_HASH)`, so we
don't need `if (size >= MIN_HASH) createHash()` anymore. This
used to be needed in `Scope#<init>`, which accepted an aribitrary
`initElems: ScopeEntry`.
- Update subclasses and factories of `Scope` in runtime reflection
to accomodate the change. Pleasingly, we could actually remove
the override for `newNestedScope`.
- Unit tests the functionality I'm touching
-rw-r--r-- | bincompat-backward.whitelist.conf | 4 | ||||
-rw-r--r-- | bincompat-forward.whitelist.conf | 4 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/Scopes.scala | 24 | ||||
-rw-r--r-- | src/reflect/scala/reflect/runtime/SymbolLoaders.scala | 2 | ||||
-rw-r--r-- | src/reflect/scala/reflect/runtime/SynchronizedOps.scala | 3 | ||||
-rw-r--r-- | test/junit/scala/reflect/internal/ScopeTest.scala | 54 |
6 files changed, 76 insertions, 15 deletions
diff --git a/bincompat-backward.whitelist.conf b/bincompat-backward.whitelist.conf index 703c5add42..ffacbf0442 100644 --- a/bincompat-backward.whitelist.conf +++ b/bincompat-backward.whitelist.conf @@ -181,6 +181,10 @@ filter { { matchName="scala.reflect.api.Internals#ReificationSupportApi.SyntacticExistentialType" problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.runtime.SynchronizedOps.newNestedScope" + problemName=MissingMethodProblem } ] } diff --git a/bincompat-forward.whitelist.conf b/bincompat-forward.whitelist.conf index ca8f7468eb..3cd985aeae 100644 --- a/bincompat-forward.whitelist.conf +++ b/bincompat-forward.whitelist.conf @@ -230,6 +230,10 @@ filter { { matchName="scala.reflect.runtime.JavaMirrors#JavaMirror.scala$reflect$runtime$JavaMirrors$JavaMirror$$followStatic" problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.runtime.SynchronizedOps.newNestedScope" + problemName=MissingMethodProblem } ] } diff --git a/src/reflect/scala/reflect/internal/Scopes.scala b/src/reflect/scala/reflect/internal/Scopes.scala index cf3f356daa..103f885ad4 100644 --- a/src/reflect/scala/reflect/internal/Scopes.scala +++ b/src/reflect/scala/reflect/internal/Scopes.scala @@ -48,22 +48,17 @@ trait Scopes extends api.Scopes { self: SymbolTable => * This is necessary because when run from reflection every scope needs to have a * SynchronizedScope as mixin. */ - class Scope protected[Scopes] (initElems: ScopeEntry = null, initFingerPrints: Long = 0L) extends ScopeApi with MemberScopeApi { + class Scope protected[Scopes]() extends ScopeApi with MemberScopeApi { - protected[Scopes] def this(base: Scope) = { - this(base.elems) - nestinglevel = base.nestinglevel + 1 - } - - private[scala] var elems: ScopeEntry = initElems + private[scala] var elems: ScopeEntry = _ /** The number of times this scope is nested in another */ - private var nestinglevel = 0 + private[Scopes] var nestinglevel = 0 /** the hash table */ - private var hashtable: Array[ScopeEntry] = null + private[Scopes] var hashtable: Array[ScopeEntry] = null /** a cache for all elements, to be used by symbol iterator. */ @@ -84,8 +79,6 @@ trait Scopes extends api.Scopes { self: SymbolTable => */ private val MIN_HASH = 8 - if (size >= MIN_HASH) createHash() - /** Returns a new scope with the same content as this one. */ def cloneScope: Scope = newScopeWith(this.toList: _*) @@ -435,7 +428,14 @@ trait Scopes extends api.Scopes { self: SymbolTable => } /** Create a new scope nested in another one with which it shares its elements */ - def newNestedScope(outer: Scope): Scope = new Scope(outer) + final def newNestedScope(outer: Scope): Scope = { + val nested = newScope // not `new Scope`, we must allow the runtime reflection universe to mixin SynchronizedScopes! + nested.elems = outer.elems + nested.nestinglevel = outer.nestinglevel + 1 + if (outer.hashtable ne null) + nested.hashtable = java.util.Arrays.copyOf(outer.hashtable, outer.hashtable.length) + nested + } /** Create a new scope with given initial elements */ def newScopeWith(elems: Symbol*): Scope = { diff --git a/src/reflect/scala/reflect/runtime/SymbolLoaders.scala b/src/reflect/scala/reflect/runtime/SymbolLoaders.scala index c56bc28d90..7ba68b8733 100644 --- a/src/reflect/scala/reflect/runtime/SymbolLoaders.scala +++ b/src/reflect/scala/reflect/runtime/SymbolLoaders.scala @@ -91,7 +91,7 @@ private[reflect] trait SymbolLoaders { self: SymbolTable => // // Short of significantly changing SymbolLoaders I see no other way than just // to slap a global lock on materialization in runtime reflection. - class PackageScope(pkgClass: Symbol) extends Scope(initFingerPrints = -1L) // disable fingerprinting as we do not know entries beforehand + class PackageScope(pkgClass: Symbol) extends Scope with SynchronizedScope { assert(pkgClass.isType) diff --git a/src/reflect/scala/reflect/runtime/SynchronizedOps.scala b/src/reflect/scala/reflect/runtime/SynchronizedOps.scala index c90901410a..4a8585d616 100644 --- a/src/reflect/scala/reflect/runtime/SynchronizedOps.scala +++ b/src/reflect/scala/reflect/runtime/SynchronizedOps.scala @@ -37,8 +37,7 @@ private[reflect] trait SynchronizedOps extends internal.SymbolTable // Scopes - override def newScope = new Scope() with SynchronizedScope - override def newNestedScope(outer: Scope): Scope = new Scope(outer) with SynchronizedScope + override def newScope = new Scope with SynchronizedScope trait SynchronizedScope extends Scope { // we can keep this lock fine-grained, because methods of Scope don't do anything extraordinary, which makes deadlocks impossible diff --git a/test/junit/scala/reflect/internal/ScopeTest.scala b/test/junit/scala/reflect/internal/ScopeTest.scala new file mode 100644 index 0000000000..5166620189 --- /dev/null +++ b/test/junit/scala/reflect/internal/ScopeTest.scala @@ -0,0 +1,54 @@ +package symtab + +import scala.tools.nsc.symtab + +import org.junit.Assert._ +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +import scala.tools.testing.AssertUtil.assertThrows +import scala.tools.nsc.symtab.SymbolTableForUnitTesting + +@RunWith(classOf[JUnit4]) +class ScopeTest { + object symbolTable extends SymbolTableForUnitTesting + + import symbolTable._ + + @Test + def testNestedScopeSmall(): Unit = testNestedScope(0) + @Test + def testNestedScopeLarge(): Unit = testNestedScope(64) // exceeding MIN_HASH + + private def testNestedScope(initSize: Int) { + def sym(termName: String): Symbol = NoSymbol.newValue(TermName(termName)) + val foo = sym("foo") + val bar = sym("bar") + + val outerElems = List.tabulate(initSize)(i => sym(i.toString)) + val outer = newScopeWith(outerElems ++ List(foo, bar) : _*) + assertTrue(outer.containsName(foo.name)) + assertTrue(outer.containsName(bar.name)) + + val baz = sym("baz") + val nested = newNestedScope(outer) + + // Entries from the outer scope are entered in the nested. + assertTrue(outer.containsName(foo.name)) + assertTrue(outer.containsName(bar.name)) + + // Nested scopes structurally share ScopeEntry-s with the outer. + assertSame(outer.lookupEntry(foo.name), nested.lookupEntry(foo.name)) + nested.enter(baz) + + // Symbols entered in the nested scope aren't visible in the outer. + assertTrue(nested.containsName(baz.name)) + assertTrue(!outer.containsName(baz.name)) + + // Unlinking a symbol in the inner scope doesn't modify the outer + nested.unlink(bar) + assert(!nested.containsName(bar.name)) + assert(outer.containsName(bar.name)) + } +} |