Reduce scope of sync blocks to reduce thread contention (finos#2953)

goldmansachs · Jul 10, 2024 · 5688cd8 · 5688cd8
1 parent d38ac81
commit 5688cd8
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 10 deletions.
diff --git a/...java/org/finos/legend/engine/language/pure/compiler/toPureGraph/HelperMeasureBuilder.java b/...java/org/finos/legend/engine/language/pure/compiler/toPureGraph/HelperMeasureBuilder.java
@@ -30,9 +30,11 @@ public static org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.type.Uni
         final GenericType genericType = new Root_meta_pure_metamodel_type_generics_GenericType_Impl("", null, context.pureModel.getClass("meta::pure::metamodel::type::generics::GenericType"))._rawType(targetUnit);
         context.pureModel.typesGenericTypeIndex.put(context.pureModel.buildPackageString(unit._package, unit.name), genericType);
         GenericType unitGenericType = new Root_meta_pure_metamodel_type_generics_GenericType_Impl("", null, context.pureModel.getClass("meta::pure::metamodel::type::generics::GenericType"))._rawType(context.pureModel.getType("meta::pure::metamodel::type::Unit"));
-        synchronized (context.pureModel)
+
+        org.finos.legend.pure.m3.coreinstance.Package pack = context.pureModel.getOrCreatePackage(unit._package);
+
+        synchronized (pack)
         {
-            org.finos.legend.pure.m3.coreinstance.Package pack = context.pureModel.getOrCreatePackage(unit._package);
             org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.type.Unit res = targetUnit
                     ._name(unit.name)
                     ._classifierGenericType(unitGenericType)

diff --git a/.../legend/engine/language/pure/compiler/toPureGraph/PackageableElementFirstPassBuilder.java b/.../legend/engine/language/pure/compiler/toPureGraph/PackageableElementFirstPassBuilder.java
@@ -330,10 +330,11 @@ private <T extends PackageableElement> T setNameAndPackage(T pureElement, String
         }
         pureElement._name(name);
 
-        synchronized (this.context.pureModel)
+        // Validate and set package
+        Package pack = this.context.pureModel.getOrCreatePackage(packagePath);
+
+        synchronized (pack)
         {
-            // Validate and set package
-            Package pack = this.context.pureModel.getOrCreatePackage(packagePath);
             if (pack._children().anySatisfy(c -> name.equals(c._name())))
             {
                 throw new EngineException("An element named '" + name + "' already exists in the package '" + packagePath + "'", sourceInformation, EngineErrorType.COMPILATION);

diff --git a/...r/src/main/java/org/finos/legend/engine/language/pure/compiler/toPureGraph/PureModel.java b/...r/src/main/java/org/finos/legend/engine/language/pure/compiler/toPureGraph/PureModel.java
@@ -1281,7 +1281,7 @@ private org.finos.legend.pure.m3.coreinstance.Package getOrCreatePackage_int(org
         return getOrCreatePackage_int(parent, pack, insert, 0);
     }
 
-    private synchronized org.finos.legend.pure.m3.coreinstance.Package getOrCreatePackage_int(org.finos.legend.pure.m3.coreinstance.Package parent, String pack, boolean insert, int start)
+    private org.finos.legend.pure.m3.coreinstance.Package getOrCreatePackage_int(org.finos.legend.pure.m3.coreinstance.Package parent, String pack, boolean insert, int start)
     {
         int end = pack.indexOf(':', start);
         String name = (end == -1) ? pack.substring(start) : pack.substring(start, end);
@@ -1299,8 +1299,16 @@ private synchronized org.finos.legend.pure.m3.coreinstance.Package getOrCreatePa
             {
                 throw new EngineException("Can't create package with reserved name '" + name + "'");
             }
-            child = new Package_Impl(name, null, this.getClass("Package"))._name(name)._package(parent);
-            parent._childrenAdd(child);
+
+            synchronized (parent)
+            {
+                child = findChildPackage(parent, name);
+                if (child == null)
+                {
+                    child = new Package_Impl(name, null, this.getClass("Package"))._name(name)._package(parent);
+                    parent._childrenAdd(child);
+                }
+            }
         }
 
         return (end == -1) ? child : getOrCreatePackage_int(child, pack, insert, end + 2);
@@ -1360,14 +1368,17 @@ public RichIterable<? extends Type> getModelClasses()
         return this.typesIndex.valuesView().reject(t -> (t == null) || (t instanceof Root_meta_pure_metamodel_type_Class_LazyImpl) || (t instanceof Root_meta_pure_metamodel_type_PrimitiveType_LazyImpl));
     }
 
-    public synchronized void loadModelFromFunctionHandler(FunctionHandler f)
+    public void loadModelFromFunctionHandler(FunctionHandler f)
     {
         if (!(f instanceof UserDefinedFunctionHandler))
         {
             String pkg = HelperModelBuilder.getElementFullPath(((org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.PackageableElement) f.getFunc())._package(), this.getExecutionSupport());
             org.finos.legend.pure.m3.coreinstance.Package n = getOrCreatePackage(root, pkg);
             org.finos.legend.pure.m3.coreinstance.Package o = getPackage((org.finos.legend.pure.m3.coreinstance.Package) METADATA_LAZY.getMetadata(M3Paths.Package, M3Paths.Root), pkg);
-            n._childrenAdd(o._children().detect(c -> f.getFunctionSignature().equals(c._name())));
+            synchronized (n)
+            {
+                n._childrenAdd(o._children().detect(c -> f.getFunctionSignature().equals(c._name())));
+            }
         }
     }