8000 Do not cache overall class · scala-js/scala-js@42efb2a · GitHub
[go: up one dir, main page]
Skip to content

Commit 42efb2a

Browse files
gzm0gzm0
committed
Do not cache overall class
This reduces some memory overhead for negligible performance cost. Residual (post link memory) benchmarks for the test suite: Baseline: 1.13 GB, new 1.01 GB
1 parent 5c56042 commit 42efb2a

File tree

2 files changed

+70
-60
lines changed

2 files changed

+70
-60
lines changed

‎linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/Emitter.scala

Lines changed: 63 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -619,37 +619,41 @@ final class Emitter[E >: Null <: js.Tree](
619619
}
620620
}
621621

622-
val fullClass = extractChanged {
623-
val fullClassCache = classCache.getFullClassCache()
624-
625-
fullClassCache.getOrElseUpdate(linkedClass.version, ctorWithGlobals,
626-
memberMethodsWithGlobals, exportedMembersWithGlobals, {
627-
for {
628-
ctor <- ctorWithGlobals
629-
memberMethods <- WithGlobals.flatten(memberMethodsWithGlobals)
630-
exportedMembers <- WithGlobals.flatten(exportedMembersWithGlobals)
631-
allMembers = ctor ::: memberMethods ::: exportedMembers
632-
clazz <- classEmitter.buildClass(
633-
className, // invalidated by overall class cache (part of ancestors)
634-
isJSClass, // invalidated by class version
635-
linkedClass.jsClassCaptures, // invalidated by class version
636-
hasClassInitializer, // invalidated by class version (optimizer cannot remove it)
637-
linkedClass.superClass, // invalidated by class version
638-
storeJSSuperClass, // invalidated by class version
639-
useESClass, // invalidated by class version (depends on kind, config and ancestry only)
640-
allMembers // invalidated directly
641-
)(moduleContext, fullClassCache, linkedClass.pos) // pos invalidated by class version
642-
} yield {
643-
// Avoid a nested post transform if we just got the original members back.
644-
if (clazz eq allMembers) {
645-
statsNestedPostTransformsAvoided += 1
646-
allMembers
647-
} else {
648-
statsNestedPostTransforms += 1
649-
postTransform(clazz, 0)
650-
}
622+
val fullClass = {
623+
val fullClassChangeTracker = classCache.getFullClassChangeTracker()
624+
625+
// Put changed state into a val to avoid short circuiting behavior of ||.
626+
val classChanged = fullClassChangeTracker.trackChanged(
627+
linkedClass.version, ctorWithGlobals,
628+
memberMethodsWithGlobals, exportedMembersWithGlobals)
629+
630+
changed ||= classChanged
631+
632+
for {
633+
ctor <- ctorWithGlobals
634+
memberMethods <- WithGlobals.flatten(memberMethodsWithGlobals)
635+
exportedMembers <- WithGlobals.flatten(exportedMembersWithGlobals)
636+
allMembers = ctor ::: memberMethods ::: exportedMembers
637+
clazz <- classEmitter.buildClass(
638+
className, // invalidated by overall class cache (part of ancestors)
639+
isJSClass, // invalidated by class version
640+
linkedClass.jsClassCaptures, // invalidated by class version
641+
hasClassInitializer, // invalidated by class version (optimizer cannot remove it)
642+
linkedClass.superClass, // invalidated by class version
643+
storeJSSuperClass, // invalidated by class version
644+
useESClass, // invalidated by class version (depends on kind, config and ancestry only)
645+
allMembers // invalidated directly
646+
)(moduleContext, fullClassChangeTracker, linkedClass.pos) // pos invalidated by class version
647+
} yield {
648+
// Avoid a nested post transform if we just got the original members back.
649+
if (clazz eq allMembers) {
650+
statsNestedPostTransformsAvoided += 1
651+
allMembers
652+
} else {
653+
statsNestedPostTransforms += 1
654+
postTransform(clazz, 0)
651655
}
652-
})
656+
}
653657
}
654658

655659
main ++= extractWithGlobals(fullClass)
@@ -862,7 +866,7 @@ final class Emitter[E >: Null <: js.Tree](
862866
private[this] val _exportedMembersCache =
863867
mutable.Map.empty[Int, MethodCache[List[E]]]
864868

865-
private[this] var _fullClassCache: Option[FullClassCache] = None
869+
private[this] var _fullClassChangeTracker: Option[FullClassChangeTracker] = None
866870

867871
override def invalidate(): Unit = {
868872
/* Do not invalidate contained methods, as they have their own
@@ -878,7 +882,7 @@ final class Emitter[E >: Null <: js.Tree](
878882
_methodCaches.foreach(_.valuesIterator.foreach(_.startRun()))
879883
_memberMethodCache.valuesIterator.foreach(_.startRun())
880884
_constructorCache.foreach(_.startRun())
881-
_fullClassCache.foreach(_.startRun())
885+
_fullClassChangeTracker.foreach(_.startRun())
882886
}
883887

884888
def getCache(version: Version): (DesugaredClassCache[List[E]], Boolean) = {
@@ -917,10 +921,10 @@ final class Emitter[E >: Null <: js.Tree](
917921
def getExportedMemberCache(idx: Int): MethodCache[List[E]] =
918922
_exportedMembersCache.getOrElseUpdate(idx, new MethodCache)
919923

920-
def getFullClassCache(): FullClassCache = {
921-
_fullClassCache.getOrElse {
922-
val cache = new FullClassCache
923-
_fullClassCache = Some(cache)
924+
def getFullClassChangeTracker(): FullClassChangeTracker = {
925+
_fullClassChangeTracker.getOrElse {
926+
val cache = new FullClassChangeTracker
927+
_fullClassChangeTracker = Some(cache)
924928
cache
925929
}
926930
}
@@ -934,8 +938,8 @@ final class Emitter[E >: Null <: js.Tree](
934938

935939
_exportedMembersCache.filterInPlace((_, c) => c.cleanAfterRun())
936940

937-
if (_fullClassCache.exists(!_.cleanAfterRun()))
938-
_fullClassCache = None
941+
if (_fullClassChangeTracker.exists(!_.cleanAfterRun()))
942+
_fullClassChangeTracker = None
939943

940944
if (!_cacheUsed)
941945
invalidate()
@@ -980,28 +984,26 @@ final class Emitter[E >: Null <: js.Tree](
980984
}
981985
}
982986

983-
private class FullClassCache extends knowledgeGuardian.KnowledgeAccessor {
984-
private[this] var _tree: WithGlobals[List[E]] = null
987+
private class FullClassChangeTracker extends knowledgeGuardian.KnowledgeAccessor {
985988
private[this] var _lastVersion: Version = Version.Unversioned
986989
private[this] var _lastCtor: WithGlobals[List[E]] = null
987990
private[this] var _lastMemberMethods: List[WithGlobals[List[E]]] = null
988991
private[this] var _lastExportedMembers: List[WithGlobals[List[E]]] = null
989-
private[this] var _cacheUsed = false
992+
private[this] var _trackerUsed = false
990993

991994
override def invalidate(): Unit = {
992995
super.invalidate()
993-
_tree = null
994996
_lastVersion = Version.Unversioned
995997
_lastCtor = null
996998
_lastMemberMethods = null
997999
_lastExportedMembers = null
9981000
}
9991001

1000-
def startRun(): Unit = _cacheUsed = false
1002+
def startRun(): Unit = _trackerUsed = false
10011003

1002-
def getOrElseUpdate(version: Version, ctor: WithGlobals[List[E]],
1003-
memberMethods: List[WithGlobals[List[E]]], exportedMembers: List[WithGlobals[List[E]]],
1004-
compute: => WithGlobals[List[E]]): (WithGlobals[List[E]], Boolean) = {
1004+
def trackChanged(version: Version, ctor: WithGlobals[List[E]],
1005+
memberMethods: List[WithGlobals[List[E]]],
1006+
exportedMembers: List[WithGlobals[List[E]]]): Boolean = {
10051007

10061008
@tailrec
10071009
def allSame[A <: AnyRef](xs: List[A], ys: List[A]): Boolean = {
@@ -1011,28 +1013,33 @@ final class Emitter[E >: Null <: js.Tree](
10111013
}
10121014
}
10131015

1014-
_cacheUsed = true
1016+
_trackerUsed = true
10151017

1016-
if (_tree == null || !version.sameVersion(_lastVersion) || (_lastCtor ne ctor) ||
1017-
!allSame(_lastMemberMethods, memberMethods) ||
1018-
!allSame(_lastExportedMembers, exportedMembers)) {
1018+
val changed = {
1019+
!version.sameVersion(_lastVersion) ||
1020+
(_lastCtor ne ctor) ||
1021+
!allSame(_lastMemberMethods, memberMethods) ||
1022+
!allSame(_lastExportedMembers, exportedMembers)
1023+
}
1024+
1025+
if (changed) {
1026+
// Input has changed or we were invalidated.
1027+
// Clean knowledge tracking and re-track dependencies.
10191028
invalidate()
1020-
_tree = compute
10211029
_lastVersion = version
10221030
_lastCtor = ctor
10231031
_lastMemberMethods = memberMethods
10241032
_lastExportedMembers = exportedMembers
1025-
(_tree, true)
1026-
} else {
1027-
(_tree, false)
10281033
}
1034+
1035+
changed
10291036
}
10301037

10311038
def cleanAfterRun(): Boolean = {
1032-
if (!_cacheUsed)
1039+
if (!_trackerUsed)
10331040
invalidate()
10341041

1035-
_cacheUsed
1042+
_trackerUsed
10361043
}
10371044
}
10381045

‎linker/shared/src/test/scala/org/scalajs/linker/EmitterTest.scala

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,18 +208,21 @@ class EmitterTest {
208208

209209
// Post transforms
210210

211-
val Seq(postTransforms1, _, _) =
211+
val Seq(postTransforms1, nestedPostTransforms1, _) =
212212
lines1.assertContainsMatch(EmitterPostTransformStatsMessage).map(_.toInt)
213213

214-
val Seq(postTransforms2, _, _) =
214+
val Seq(postTransforms2, nestedPostTransforms2, _) =
215215
lines2.assertContainsMatch(EmitterPostTransformStatsMessage).map(_.toInt)
216216

217-
// At the time of writing this test, postTransformsTotal1 reports 216
217+
// At the time of writing this test, postTransforms1 reports 216
218218
assertTrue(
219219
s"Not enough post transforms (got $postTransforms1); extraction must have gone wrong",
220220
postTransforms1 > 200)
221221

222-
assertEquals("Second run must not have any post transforms", 0, postTransforms2)
222+
assertEquals("Second run must only have nested post transforms",
223+
nestedPostTransforms2, postTransforms2)
224+
assertEquals("Both runs must have the same number of nested post transforms",
225+
nestedPostTransforms1, nestedPostTransforms2)
223226
}
224227
}
225228
}

0 commit comments

Comments
 (0)
0