From fab2d3cd9f5a8d3ce58cc760a4c0865064065110 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 6 Dec 2024 23:51:10 +0000 Subject: [PATCH] Improve type finding performance --- .../nawforce/apexlink/org/ModuleFind.scala | 39 ++++++------ .../nawforce/pkgforce/names/EncodedName.scala | 60 ++++++++++++++++--- .../nawforce/pkgforce/pkgs/TriHierarchy.scala | 32 +++++----- .../pkgforce/names/EncodedNameTest.scala | 26 ++++++++ 4 files changed, 115 insertions(+), 42 deletions(-) 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 035997eb6..87ceefbc8 100644 --- a/jvm/src/main/scala/com/nawforce/apexlink/org/ModuleFind.scala +++ b/jvm/src/main/scala/com/nawforce/apexlink/org/ModuleFind.scala @@ -19,7 +19,7 @@ import com.nawforce.apexlink.finding.TypeResolver.TypeResponse import com.nawforce.apexlink.names.TypeNames import com.nawforce.apexlink.types.core.TypeDeclaration import com.nawforce.pkgforce.modifiers.GLOBAL_MODIFIER -import com.nawforce.pkgforce.names.{EncodedName, TypeName} +import com.nawforce.pkgforce.names.{EncodedName, Name, TypeName} trait ModuleFind { self: OPM.Module => @@ -58,6 +58,17 @@ trait ModuleFind { typeName: TypeName, from: Option[TypeDeclaration], inPackage: Boolean = true + ): Option[TypeDeclaration] = { + // Use aliased type name here so we don't mishandle an ambiguous typename when searching + val targetType = TypeNames.aliasOrReturn(typeName) + + findPackageTypeInternal(targetType, from, inPackage) + } + + private def findPackageTypeInternal( + typeName: TypeName, + from: Option[TypeDeclaration], + inPackage: Boolean ): Option[TypeDeclaration] = { // Might be an outer in this module var declaration = findModuleType(typeName) @@ -70,7 +81,7 @@ trait ModuleFind { // Or maybe an inner if (typeName.outer.nonEmpty) { - declaration = findPackageType(typeName.outer.get, from, inPackage = inPackage) + declaration = findPackageTypeInternal(typeName.outer.get, from, inPackage = inPackage) .flatMap(_.findNestedType(typeName.name).filter(td => td.isExternallyVisible || inPackage)) if (declaration.nonEmpty) return declaration @@ -82,28 +93,22 @@ trait ModuleFind { /** Find a type just in this module. */ def findModuleType(typeName: TypeName): Option[TypeDeclaration] = { - // Use aliased type name here so we don't mishandle an ambiguous typename when searching - val targetType = TypeNames.aliasOrReturn(typeName) - // Direct hit - var declaration = types.get(targetType) + val declaration = types.get(typeName) if (declaration.nonEmpty) return declaration // SObject and alike, we want module specific version of these - declaration = types.getWithSchema(targetType) - if (declaration.nonEmpty) - return declaration - if ( - targetType.params.isEmpty && - (targetType.outer.isEmpty || targetType.outer.contains(TypeNames.Schema)) + typeName.params.isEmpty && + (typeName.outer.isEmpty || typeName.outer.contains(TypeNames.Schema)) ) { - val encName = EncodedName(targetType.name, namespace) - if (encName.ext.nonEmpty) { - return types.getWithSchema(TypeName(encName.fullName, Nil, None)) - } + if (EncodedName.encodedNeedsNamespace(typeName.name)) + types.getWithSchema(TypeName(Name(namespacePrefix + typeName.name.value), Nil, None)) + else + types.getWithSchema(typeName) + } else { + None } - None } } diff --git a/shared/src/main/scala/com/nawforce/pkgforce/names/EncodedName.scala b/shared/src/main/scala/com/nawforce/pkgforce/names/EncodedName.scala index 8fefd30be..bf1b87aa2 100644 --- a/shared/src/main/scala/com/nawforce/pkgforce/names/EncodedName.scala +++ b/shared/src/main/scala/com/nawforce/pkgforce/names/EncodedName.scala @@ -13,18 +13,17 @@ */ package com.nawforce.pkgforce.names -/** A name with optional namespace prefixing & optional type suffix such as foo__field__c. Only a +/** A name with optional namespace prefixing & optional type suffix such as foo__field__c. Only a * small set of suffixes are supported. It's not clear what the full list used by Salesforce is. - * This class is safe to use on non-encoded names which don't contain \_\_, although you can not + * This class is safe to use on non-encoded names which don't contain __, although you can not * default the namespace on them. * - * Where namespace, name and suffix are provided the code will assert on a bad suffix. Where - * we only have two parts of the name then if the last parts is not a valid suffix the handling - * will assume the first part is a namespace and the second the name, as in Page.pkg1\_\_TestPage. + * Where we only have two parts of the name then if the last parts is not a valid suffix the handling + * will assume the first part is a namespace and the second the name, as in Page.pkg1__TestPage. * - * The code deals with a few exception cases where splitting on \_\_ would give either a wrong + * The code deals with a few exception cases where splitting on __ would give either a wrong * name or ext part. For subfields the subfield name is combined with the extension. For supporting - * SObjects such as MyObject\_\_Feed the 'Feed' is considered an extension in the same way that 'c' + * SObjects such as MyObject__Feed the 'Feed' is considered an extension in the same way that 'c' * would be. */ final case class EncodedName(name: Name, ext: Option[Name], namespace: Option[Name]) { @@ -49,6 +48,10 @@ final case class EncodedName(name: Name, ext: Option[Name], namespace: Option[Na } object EncodedName { + + /** Standard set of extensions, note that the subfield extension "s" is missing as this + * often needs special handling because it does not fit the <ns__>name__ext pattern + */ private final val extensions = Set("c", "r", "e", "b", "mdt", "share", "history", "feed") def apply(name: Name): EncodedName = { @@ -108,4 +111,47 @@ object EncodedName { name.substring(tailSplit + 2) ) } + + /** Test if name contains an encoded name that may need a namespace adding for type + * resolving. This is broken out from the EncodedName handling to improve performance. + */ + def encodedNeedsNamespace(name: Name): Boolean = { + val value = name.value + val suffix = extractSuffixOrReturn(value) + if (suffix eq value) + return false + + val firstSeparator = value.indexOf("__") + if (firstSeparator == value.length - suffix.length - 2) { + true + } else { + val secondSeparator = value.indexOf("__", firstSeparator + 1) + if (secondSeparator == value.length - suffix.length - 2) { + suffix == "s" + } else { + false + } + } + } + + private def extractSuffixOrReturn(name: String): String = { + val len = name.length + if (len >= 4) { + name.charAt(len - 1) match { + case 'c' | 'C' if name.charAt(len - 2) == '_' && name.charAt(len - 3) == '_' => "c" + case 'r' | 'R' if name.charAt(len - 2) == '_' && name.charAt(len - 3) == '_' => "r" + case 'e' | 'E' if name.charAt(len - 2) == '_' && name.charAt(len - 3) == '_' => "e" + case 'b' | 'B' if name.charAt(len - 2) == '_' && name.charAt(len - 3) == '_' => "b" + case 's' | 'S' if name.charAt(len - 2) == '_' && name.charAt(len - 3) == '_' => "s" + case 't' | 'T' if len >= 6 && name.toLowerCase.endsWith("__mdt") => "mdt" + case 'e' | 'E' if len >= 8 && name.toLowerCase.endsWith("__share") => "share" + case 'y' | 'Y' if len >= 10 && name.toLowerCase.endsWith("__history") => "history" + case 'd' | 'D' if len >= 7 && name.toLowerCase.endsWith("__feed") => "feed" + case _ => name + } + } else { + name + } + } + } diff --git a/shared/src/main/scala/com/nawforce/pkgforce/pkgs/TriHierarchy.scala b/shared/src/main/scala/com/nawforce/pkgforce/pkgs/TriHierarchy.scala index 0193418e6..6575f3cfd 100644 --- a/shared/src/main/scala/com/nawforce/pkgforce/pkgs/TriHierarchy.scala +++ b/shared/src/main/scala/com/nawforce/pkgforce/pkgs/TriHierarchy.scala @@ -55,13 +55,13 @@ abstract class TriHierarchy { /** Is this a ghost package, aka it has no modules. */ lazy val isGhosted: Boolean = modules.isEmpty - /* Find first module in search order (may not be in this package) */ - def firstModule: Option[TModule] = { + /** Find first module in search order (may not be in this package) */ + lazy val firstModule: Option[TModule] = { orderedModules.headOption .orElse(basePackages.headOption.flatMap(_.firstModule)) } - /* Check if a type is ghosted in this package */ + /** Check if a type is ghosted in this package */ def isGhostedType(typeName: TypeName): Boolean = { if (typeName.outer.contains(TypeName.Schema)) { val encName = EncodedName(typeName.name) @@ -92,34 +92,30 @@ abstract class TriHierarchy { /** The modules that this module depends on deploy order */ val dependents: ArraySeq[TModule] - /** The module (& owning package namespace) */ + /** The module (& owning package) namespace */ lazy val namespace: Option[Name] = pkg.namespace + /** The module (& owning package) namespace prefix */ + lazy val namespacePrefix: String = namespace.map(ns => ns.toString + "__").getOrElse("") + /** The package the parent package depends on in reverse deploy order */ lazy val basePackages: ArraySeq[TPackage] = pkg.basePackages.reverse /** The modules that this module depends on in reverse deploy order */ lazy val baseModules: ArraySeq[TModule] = dependents.reverse - /** Test if a file is visible to this module, i.e. in scope & not ignored */ - def isVisibleFile(path: PathLike): Boolean - - /* Transitive Modules (dependent modules for this modules & its dependents) */ - def transitiveModules: Set[TModule] = { - namespace - .map(_ => dependents.toSet ++ dependents.flatMap(_.transitiveModules)) - .getOrElse(baseModules.toSet) - } - - /* Find next module in search order */ - def nextModule: Option[TModule] = { + /** Find next module in search order */ + lazy val nextModule: Option[TModule] = { baseModules.headOption.orElse(basePackages.headOption.flatMap(_.firstModule)) } - /* Check if a type name is ghosted in this module */ + /** Test if a file is visible to this module, i.e. in scope & not ignored */ + def isVisibleFile(path: PathLike): Boolean + + /** Check if a type name is ghosted in this module */ def isGhostedType(typeName: TypeName): Boolean = pkg.isGhostedType(typeName) - /* Check if a field name is ghosted in this module */ + /** Check if a field name is ghosted in this module */ def isGhostedFieldName(name: Name): Boolean = pkg.isGhostedFieldName(name) } } diff --git a/shared/src/test/scala/com/nawforce/pkgforce/names/EncodedNameTest.scala b/shared/src/test/scala/com/nawforce/pkgforce/names/EncodedNameTest.scala index ae7a6a8af..a17b513e0 100644 --- a/shared/src/test/scala/com/nawforce/pkgforce/names/EncodedNameTest.scala +++ b/shared/src/test/scala/com/nawforce/pkgforce/names/EncodedNameTest.scala @@ -27,6 +27,7 @@ */ package com.nawforce.pkgforce.names +import org.scalatest.Inspectors.forAll import org.scalatest.funsuite.AnyFunSuite class EncodedNameTest extends AnyFunSuite { @@ -138,4 +139,29 @@ class EncodedNameTest extends AnyFunSuite { assert(testName.fullName == Name("ns__Foo__x")) assert(testName.asTypeName == TypeName(Name("ns__Foo__x"), Nil, None)) } + + forAll(List("c", "r", "e", "b", "mdt", "share", "history", "feed")) { ext => + test(s"needs namespace simple extensions $ext") { + assert(!EncodedName.encodedNeedsNamespace(Name(""))) + assert(!EncodedName.encodedNeedsNamespace(Name(s"$ext"))) + assert(!EncodedName.encodedNeedsNamespace(Name(s"__$ext"))) + assert(EncodedName.encodedNeedsNamespace(Name(s"a__$ext"))) + assert(EncodedName.encodedNeedsNamespace(Name(s"abc__$ext"))) + assert(!EncodedName.encodedNeedsNamespace(Name(s"__abc__$ext"))) + assert(!EncodedName.encodedNeedsNamespace(Name(s"n__abc__$ext"))) + } + } + + test("needs namespace subfield extension") { + assert(!EncodedName.encodedNeedsNamespace(Name(""))) + assert(!EncodedName.encodedNeedsNamespace(Name("s"))) + assert(!EncodedName.encodedNeedsNamespace(Name("__s"))) + assert(EncodedName.encodedNeedsNamespace(Name("a__s"))) + assert(EncodedName.encodedNeedsNamespace(Name("abc__s"))) + assert(EncodedName.encodedNeedsNamespace(Name("__abc__s"))) + assert(EncodedName.encodedNeedsNamespace(Name("a__abc__s"))) + assert(EncodedName.encodedNeedsNamespace(Name("abc__abc__s"))) + assert(!EncodedName.encodedNeedsNamespace(Name("n__abc__abc__s"))) + } + }