From 177fbdcd7dce0c51b8b331a156e8e74b66543ba9 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Sat, 30 Nov 2024 20:43:37 +0000 Subject: [PATCH 1/3] Add custom Type Declaration cache to speed Schema searches --- .../nawforce/apexlink/org/ModuleFind.scala | 9 ++- .../scala/com/nawforce/apexlink/org/OPM.scala | 23 +++--- .../nawforce/apexlink/org/PackageAPI.scala | 10 +-- .../apexlink/org/StreamDeployer.scala | 27 +++---- .../apexlink/org/TypeDeclarationCache.scala | 72 +++++++++++++++++++ 5 files changed, 104 insertions(+), 37 deletions(-) create mode 100644 jvm/src/main/scala/com/nawforce/apexlink/org/TypeDeclarationCache.scala diff --git a/jvm/src/main/scala/com/nawforce/apexlink/org/ModuleFind.scala b/jvm/src/main/scala/com/nawforce/apexlink/org/ModuleFind.scala index 9235f4461..3d79c0f75 100644 --- a/jvm/src/main/scala/com/nawforce/apexlink/org/ModuleFind.scala +++ b/jvm/src/main/scala/com/nawforce/apexlink/org/ModuleFind.scala @@ -91,18 +91,17 @@ trait ModuleFind { return declaration // SObject and alike, we want module specific version of these - declaration = types.get(targetType.withTail(TypeNames.Schema)) + declaration = types.getWithSchema(targetType) if (declaration.nonEmpty) return declaration if ( - targetType.params.isEmpty && (targetType.outer.isEmpty || targetType.outer.contains( - TypeNames.Schema - )) + targetType.params.isEmpty && + (targetType.outer.isEmpty || targetType.outer.contains(TypeNames.Schema)) ) { val encName = EncodedName(targetType.name).defaultNamespace(namespace) if (encName.ext.nonEmpty) { - return types.get(TypeName(encName.fullName, Nil, Some(TypeNames.Schema))) + return types.getWithSchema(TypeName(encName.fullName, Nil, None)) } } None diff --git a/jvm/src/main/scala/com/nawforce/apexlink/org/OPM.scala b/jvm/src/main/scala/com/nawforce/apexlink/org/OPM.scala index 4d0c8d74a..578689176 100644 --- a/jvm/src/main/scala/com/nawforce/apexlink/org/OPM.scala +++ b/jvm/src/main/scala/com/nawforce/apexlink/org/OPM.scala @@ -664,7 +664,7 @@ object OPM extends TriHierarchy { val isGulped: Boolean = pkg.isGulped - private[nawforce] var types = mutable.Map[TypeName, TypeDeclaration]() + private[nawforce] val types = new TypeDeclarationCache() private val schemaManager = SchemaSObjectType(this) @unused @@ -689,25 +689,25 @@ object OPM extends TriHierarchy { def schemaSObjectType: SchemaSObjectType = schemaManager - def any: AnyDeclaration = types(TypeNames.Any).asInstanceOf[AnyDeclaration] + def any: AnyDeclaration = types.getUnsafe(TypeNames.Any).asInstanceOf[AnyDeclaration] - def labels: LabelDeclaration = types(TypeNames.Label).asInstanceOf[LabelDeclaration] + def labels: LabelDeclaration = types.getUnsafe(TypeNames.Label).asInstanceOf[LabelDeclaration] def interviews: InterviewDeclaration = - types(TypeNames.Interview).asInstanceOf[InterviewDeclaration] + types.getUnsafe(TypeNames.Interview).asInstanceOf[InterviewDeclaration] - def pages: PageDeclaration = types(TypeNames.Page).asInstanceOf[PageDeclaration] + def pages: PageDeclaration = types.getUnsafe(TypeNames.Page).asInstanceOf[PageDeclaration] def components: ComponentDeclaration = - types(TypeNames.Component).asInstanceOf[ComponentDeclaration] + types.getUnsafe(TypeNames.Component).asInstanceOf[ComponentDeclaration] def nonTestClasses: Iterable[ApexClassDeclaration] = - types.values.collect { + types.values().collect { case ac: ApexClassDeclaration if !ac.inTest => ac } def testClasses: Iterable[ApexClassDeclaration] = - types.values.collect { + types.values().collect { case ac: ApexClassDeclaration if ac.inTest => ac } @@ -717,14 +717,15 @@ object OPM extends TriHierarchy { /* Iterator over available types */ def getTypes: Iterable[TypeDeclaration] = { - types.values + types.values() } /** Iterate metadata defined types, this will include referenced platform SObjects irrespective * of if they have been extended or not which is perhaps not quite accurate to the method name. */ def getMetadataDefinedTypeIdentifiers(apexOnly: Boolean): Iterable[TypeIdentifier] = { - types.values + types + .values() .collect { case x: ApexDeclaration => x case x: SObjectDeclaration if !apexOnly => x @@ -780,7 +781,7 @@ object OPM extends TriHierarchy { // Add dependencies for Apex types to a map def populateDependencies(dependencies: java.util.Map[String, Array[String]]): Unit = { val typeCache = new TypeCache() - types.values.foreach { + types.values().foreach { case td: ApexClassDeclaration => val depends = mutable.Set[TypeId]() td.gatherDependencies(depends, apexOnly = false, outerTypesOnly = true, typeCache) diff --git a/jvm/src/main/scala/com/nawforce/apexlink/org/PackageAPI.scala b/jvm/src/main/scala/com/nawforce/apexlink/org/PackageAPI.scala index 2d1293ac3..49bc7d3d8 100644 --- a/jvm/src/main/scala/com/nawforce/apexlink/org/PackageAPI.scala +++ b/jvm/src/main/scala/com/nawforce/apexlink/org/PackageAPI.scala @@ -247,10 +247,12 @@ trait PackageAPI extends Package { def flush(pc: ParsedCache): Unit = { val context = packageContext modules.foreach(module => { - module.types.values.foreach({ - case ad: ApexClassDeclaration => ad.flush(pc, context) - case _ => () - }) + module.types + .values() + .foreach({ + case ad: ApexClassDeclaration => ad.flush(pc, context) + case _ => () + }) }) } diff --git a/jvm/src/main/scala/com/nawforce/apexlink/org/StreamDeployer.scala b/jvm/src/main/scala/com/nawforce/apexlink/org/StreamDeployer.scala index aad53fda6..7afdfbfb9 100644 --- a/jvm/src/main/scala/com/nawforce/apexlink/org/StreamDeployer.scala +++ b/jvm/src/main/scala/com/nawforce/apexlink/org/StreamDeployer.scala @@ -19,10 +19,8 @@ import com.nawforce.apexlink.finding.TypeResolver.TypeCache import com.nawforce.apexlink.names.TypeNames.TypeNameUtils import com.nawforce.apexlink.opcst.OutlineParserFullDeclaration import com.nawforce.apexlink.types.apex.{FullDeclaration, SummaryApex, TriggerDeclaration} -import com.nawforce.apexlink.types.core.TypeDeclaration import com.nawforce.apexlink.types.other._ import com.nawforce.apexlink.types.platform.PlatformTypes -import com.nawforce.apexlink.types.schema.SObjectDeclaration import com.nawforce.pkgforce.diagnostics._ import com.nawforce.pkgforce.documents._ import com.nawforce.pkgforce.names._ @@ -45,7 +43,7 @@ import scala.util.hashing.MurmurHash3 class StreamDeployer( module: OPM.Module, events: Iterator[PackageEvent], - types: mutable.Map[TypeName, TypeDeclaration] + types: TypeDeclarationCache ) { load() @@ -84,25 +82,25 @@ class StreamDeployer( val labelFileEvents = labelRelatedEvents.collect { case e: LabelFileEvent => e } val labelEvents = labelRelatedEvents.collect { case e: LabelEvent => e } val labels = LabelDeclaration(module).merge(labelFileEvents, labelEvents) - upsertMetadata(labels) - upsertMetadata(labels, Some(TypeName(labels.name))) + types.put(labels) + types.put(labels, Some(TypeName(labels.name))) } private def consumeComponents(events: BufferedIterator[PackageEvent]): ComponentDeclaration = { val componentDeclaration = ComponentDeclaration(module).merge(bufferEvents[ComponentEvent](events)) - upsertMetadata(componentDeclaration) + types.put(componentDeclaration) componentDeclaration } private def consumePages(events: BufferedIterator[PackageEvent]): PageDeclaration = { val pageDeclaration = PageDeclaration(module).merge(bufferEvents[PageEvent](events)) - upsertMetadata(pageDeclaration) + types.put(pageDeclaration) pageDeclaration } private def consumeFlows(events: BufferedIterator[PackageEvent]): Unit = { - upsertMetadata(InterviewDeclaration(module).merge(bufferEvents[FlowEvent](events))) + types.put(InterviewDeclaration(module).merge(bufferEvents[FlowEvent](events))) } private def consumeSObjects(events: BufferedIterator[PackageEvent]): Unit = { @@ -110,7 +108,7 @@ class StreamDeployer( val deployer = new SObjectDeployer(module) val sobjects = deployer.createSObjects(events) sobjects.foreach(sobject => { - types.put(sobject.typeName, sobject) + types.put(sobject) module.schemaSObjectType.add(sobject.typeName.name, hasFieldSets = true) }) @@ -156,7 +154,7 @@ class StreamDeployer( FullDeclaration .create(module, doc, data, forceConstruct = false) .map(td => { - types.put(td.typeName, td) + types.put(td) td }) } @@ -288,7 +286,7 @@ class StreamDeployer( ArraySeq.from(failedDocuments.asScala.toSeq) } - /** Consume trigger events, these could be cached but they don't consume much time to for we load + /** Consume trigger events, these could be cached, but they don't consume much time to for we load * from disk and parse each time. */ private def consumeTriggers(events: BufferedIterator[PackageEvent]): Unit = { @@ -302,16 +300,11 @@ class StreamDeployer( TriggerDeclaration .create(module, doc.path, data) .map(td => { - types.put(td.typeName, td) + types.put(td) td.validate() }) } }) } } - - // Upsert some metadata to the package - def upsertMetadata(td: TypeDeclaration, altTypeName: Option[TypeName] = None): Unit = { - types.put(altTypeName.getOrElse(td.typeName), td) - } } diff --git a/jvm/src/main/scala/com/nawforce/apexlink/org/TypeDeclarationCache.scala b/jvm/src/main/scala/com/nawforce/apexlink/org/TypeDeclarationCache.scala new file mode 100644 index 000000000..6b7027bef --- /dev/null +++ b/jvm/src/main/scala/com/nawforce/apexlink/org/TypeDeclarationCache.scala @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2024 Certinia Inc. All rights reserved. + */ + +package com.nawforce.apexlink.org + +import com.nawforce.apexlink.types.core.TypeDeclaration +import com.nawforce.pkgforce.names.TypeName + +import scala.collection.mutable + +/** Cache of TypeDeclarations against their TypeName. Provided to speed up Schema namespace + * searched by avoiding the need to construct a new TypeName for each search. + */ +class TypeDeclarationCache { + private val allTypes = mutable.Map[TypeName, TypeDeclaration]() + private val schemaTypes = mutable.Map[TypeName, TypeDeclaration]() + + def size: Int = { + allTypes.size + } + + def put(td: TypeDeclaration, altTypeName: Option[TypeName] = None): Unit = { + put(altTypeName.getOrElse(td.typeName), td) + } + + /** Upsert an entry. Beware, assumes the TypeName is fully qualified. */ + def put(typeName: TypeName, td: TypeDeclaration): Unit = { + allTypes.put(typeName, td) + if (typeName.outer.contains(TypeName.Schema)) { + schemaTypes.put(typeName.withOuter(None), td) + } + } + + def get(typeName: TypeName): Option[TypeDeclaration] = { + allTypes.get(typeName) + } + + def getUnsafe(typeName: TypeName): TypeDeclaration = { + allTypes(typeName) + } + + def getWithSchema(typeName: TypeName): Option[TypeDeclaration] = { + schemaTypes.get(typeName) + } + + def contains(typeName: TypeName): Boolean = { + allTypes.contains(typeName) + } + + def values(): Iterable[TypeDeclaration] = { + allTypes.values + } + + def filter( + pred: ((TypeName, TypeDeclaration)) => Boolean + ): mutable.Map[TypeName, TypeDeclaration] = { + allTypes.filter(pred) + } + + def collect[T](pf: PartialFunction[(TypeName, TypeDeclaration), T]): mutable.Iterable[T] = { + allTypes.collect(pf) + } + + def remove(typeName: TypeName): Option[TypeDeclaration] = { + val result = allTypes.remove(typeName) + if (typeName.outer.contains(TypeName.Schema)) { + schemaTypes.remove(typeName.withOuter(None)) + } + result + } +} From f0bd1c92c62bc6bad57e134a79337f3f404e7d7c Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Sun, 1 Dec 2024 00:31:51 +0000 Subject: [PATCH 2/3] Add type declaration cache tests --- .../apexlink/org/TypeDeclarationCache.scala | 7 ++- .../org/TypeDeclarationCacheTest.scala | 62 +++++++++++++++++++ .../pkgforce/names/TypeNameFuncs.scala | 18 ++++++ 3 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 jvm/src/test/scala/com/nawforce/apexlink/org/TypeDeclarationCacheTest.scala diff --git a/jvm/src/main/scala/com/nawforce/apexlink/org/TypeDeclarationCache.scala b/jvm/src/main/scala/com/nawforce/apexlink/org/TypeDeclarationCache.scala index 6b7027bef..0fab8dc57 100644 --- a/jvm/src/main/scala/com/nawforce/apexlink/org/TypeDeclarationCache.scala +++ b/jvm/src/main/scala/com/nawforce/apexlink/org/TypeDeclarationCache.scala @@ -6,6 +6,7 @@ package com.nawforce.apexlink.org import com.nawforce.apexlink.types.core.TypeDeclaration import com.nawforce.pkgforce.names.TypeName +import com.nawforce.pkgforce.names.TypeNameFuncs.TypeNameFuncs import scala.collection.mutable @@ -27,9 +28,9 @@ class TypeDeclarationCache { /** Upsert an entry. Beware, assumes the TypeName is fully qualified. */ def put(typeName: TypeName, td: TypeDeclaration): Unit = { allTypes.put(typeName, td) - if (typeName.outer.contains(TypeName.Schema)) { - schemaTypes.put(typeName.withOuter(None), td) - } + val stripped = typeName.replaceTail(TypeName.Schema, None) + if (stripped ne typeName) + schemaTypes.put(stripped, td) } def get(typeName: TypeName): Option[TypeDeclaration] = { diff --git a/jvm/src/test/scala/com/nawforce/apexlink/org/TypeDeclarationCacheTest.scala b/jvm/src/test/scala/com/nawforce/apexlink/org/TypeDeclarationCacheTest.scala new file mode 100644 index 000000000..e77bdac69 --- /dev/null +++ b/jvm/src/test/scala/com/nawforce/apexlink/org/TypeDeclarationCacheTest.scala @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2024 Certinia Inc. All rights reserved. + */ +package com.nawforce.apexlink.org + +import com.nawforce.apexlink.TestHelper +import com.nawforce.apexlink.names.TypeNames +import com.nawforce.pkgforce.names.TypeNameFuncs.TypeNameFuncs +import com.nawforce.pkgforce.names.{Name, TypeName} +import org.scalatest.Inspectors.forAll +import org.scalatest.funsuite.AnyFunSuite + +class TypeDeclarationCacheTest extends AnyFunSuite with TestHelper { + + private val simpleTypeName = TypeName(Name("Simple")) + private val scopedTypeName = TypeName(Name("Scoped"), Nil, Some(TypeName(Name("Outer")))) + private val systemTypeName = TypeName(Name("String"), Nil, Some(TypeName(Name("System")))) + private val genericTypeName = + TypeName(Name("List"), systemTypeName :: Nil, Some(TypeName(Name("System")))) + private val schemaTypeName = + TypeName(Name("Account"), Nil, Some(TypeNames.Schema)) + private val nestedSchemaTypeName = + TypeName(Name("Fake"), Nil, Some(TypeName(Name("Account"), Nil, Some(TypeNames.Schema)))) + + test("Empty cache has no size") { + assert(new TypeDeclarationCache().size == 0) + } + + forAll( + List( + /* + simpleTypeName, + scopedTypeName, + systemTypeName, + genericTypeName, + schemaTypeName, + */ + nestedSchemaTypeName + ) + ) { testTypeName => + test(s"$testTypeName name can be added and removed") { + val cache = new TypeDeclarationCache() + cache.put(testTypeName, null) + + assert(cache.size == 1) + assert(cache.contains(testTypeName)) + assert(cache.get(testTypeName).contains(null)) + assert(cache.getUnsafe(testTypeName) == null) + assert(cache.getWithSchema(testTypeName).isEmpty) + val stripped = testTypeName.replaceTail(TypeName.Schema, None) + if (stripped != testTypeName) + assert(cache.getWithSchema(stripped).nonEmpty) + assert(cache.values().toList == List(null)) + assert(cache.filter(_._1 == testTypeName).nonEmpty) + assert(cache.collect { case (t: TypeName, null) if t == testTypeName => }.nonEmpty) + + assert(cache.remove(testTypeName).contains(null)) + assert(cache.size == 0) + } + } + +} diff --git a/shared/src/main/scala/com/nawforce/pkgforce/names/TypeNameFuncs.scala b/shared/src/main/scala/com/nawforce/pkgforce/names/TypeNameFuncs.scala index 82d4b2dae..b534c188d 100644 --- a/shared/src/main/scala/com/nawforce/pkgforce/names/TypeNameFuncs.scala +++ b/shared/src/main/scala/com/nawforce/pkgforce/names/TypeNameFuncs.scala @@ -20,5 +20,23 @@ object TypeNameFuncs { def outerName: Name = typeName.outer.map(_.outerName).getOrElse(typeName.name) + /** Replace the tail section of a TypeName with something else. + * @return an updated TypeName or this if not matched. + */ + def replaceTail(tail: TypeName, replacement: Option[TypeName]): TypeName = { + if (typeName.outer.contains(tail)) + return TypeName(typeName.name, typeName.params, replacement) + + typeName.outer + .map(outer => { + val dropped = outer.replaceTail(tail, replacement) + if (dropped ne outer) { + TypeName(typeName.name, typeName.params, Some(dropped)) + } else { + typeName + } + }) + .getOrElse(typeName) + } } } From 8ff73bf1f030e3806928e9ccfda1a3395e1daaa5 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Sun, 1 Dec 2024 09:59:43 +0000 Subject: [PATCH 3/3] Fix declaration cache remove --- .../apexlink/org/TypeDeclarationCache.scala | 7 ++++--- .../org/TypeDeclarationCacheTest.scala | 18 ++++++++++++++---- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/jvm/src/main/scala/com/nawforce/apexlink/org/TypeDeclarationCache.scala b/jvm/src/main/scala/com/nawforce/apexlink/org/TypeDeclarationCache.scala index 0fab8dc57..afaa40e37 100644 --- a/jvm/src/main/scala/com/nawforce/apexlink/org/TypeDeclarationCache.scala +++ b/jvm/src/main/scala/com/nawforce/apexlink/org/TypeDeclarationCache.scala @@ -64,9 +64,10 @@ class TypeDeclarationCache { } def remove(typeName: TypeName): Option[TypeDeclaration] = { - val result = allTypes.remove(typeName) - if (typeName.outer.contains(TypeName.Schema)) { - schemaTypes.remove(typeName.withOuter(None)) + val result = allTypes.remove(typeName) + val stripped = typeName.replaceTail(TypeName.Schema, None) + if (stripped != typeName) { + schemaTypes.remove(stripped) } result } diff --git a/jvm/src/test/scala/com/nawforce/apexlink/org/TypeDeclarationCacheTest.scala b/jvm/src/test/scala/com/nawforce/apexlink/org/TypeDeclarationCacheTest.scala index e77bdac69..5d2b89e81 100644 --- a/jvm/src/test/scala/com/nawforce/apexlink/org/TypeDeclarationCacheTest.scala +++ b/jvm/src/test/scala/com/nawforce/apexlink/org/TypeDeclarationCacheTest.scala @@ -22,19 +22,21 @@ class TypeDeclarationCacheTest extends AnyFunSuite with TestHelper { private val nestedSchemaTypeName = TypeName(Name("Fake"), Nil, Some(TypeName(Name("Account"), Nil, Some(TypeNames.Schema)))) - test("Empty cache has no size") { - assert(new TypeDeclarationCache().size == 0) + test("Empty cache behaviour") { + val cache = new TypeDeclarationCache() + assert(cache.size == 0) + assert(cache.values().toList == Nil) + assert(cache.filter(_ => true).isEmpty) + assert(cache.collect { case _ => }.isEmpty) } forAll( List( - /* simpleTypeName, scopedTypeName, systemTypeName, genericTypeName, schemaTypeName, - */ nestedSchemaTypeName ) ) { testTypeName => @@ -56,6 +58,14 @@ class TypeDeclarationCacheTest extends AnyFunSuite with TestHelper { assert(cache.remove(testTypeName).contains(null)) assert(cache.size == 0) + assert(!cache.contains(testTypeName)) + assert(cache.get(testTypeName).isEmpty) + assert(cache.getWithSchema(testTypeName).isEmpty) + if (stripped != testTypeName) + assert(cache.getWithSchema(stripped).isEmpty) + assert(cache.values().toList == Nil) + assert(cache.filter(_._1 == testTypeName).isEmpty) + assert(cache.collect { case (t: TypeName, null) if t == testTypeName => }.isEmpty) } }