You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2021/02/18 18:07:51 UTC

[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads

JaroslavTulach commented on a change in pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765#discussion_r578534032



##########
File path: extide/gradle/src/org/netbeans/modules/gradle/NbGradleProjectImpl.java
##########
@@ -174,14 +174,76 @@ private Lookup createBasicLookup(ProjectState state, GradleAuxiliaryConfigImpl a
                 state
         );
     }
+    
+    GradleProject getProjectInternal() {
+        synchronized (this) {
+            return project;
+        }
+    }
 
     public GradleProject getGradleProject() {
-        if (project == null) {
-            project = loadProject();
+        GradleProject p = getProjectInternal();
+        if (p != null) {
+            return p;
+        }
+        p = loadProject();
+        synchronized (this) {
+            // avod unnecessary project replacements.
+            if (project != null) {
+                return project;
+            }
+            project = p;
         }
-        return project;
+        return p;
     }
-
+    
+    /**
+     * Records a reloaded project instance. Indicates whether the project was replaced.
+     * The method refuses to replace a project that has been changed since the reload procedure
+     * started. Will also not replace the project, if the original has a better quality
+     * than the reloaded one.
+     * <p>
+     * Pass {@code original = null} to just replace the project without checking.
+     * 
+     * @param original the original project data instance.
+     * @param reloaded the reloaded data.
+     * @param reloadIfMismatch will fire new reload if the project differs from original or quality is not satisfactory.
+     * @return {@code null}, if the project was accepted. {@code Boolean.TRUE}, if the project has
+     * been changed independently; {@code Boolean.FALSE}, if the reloaded project's quality is lower than
+     * the current.
+     */
+    Boolean recordOrReloadProject(GradleProject original, GradleProject reloaded, boolean reloadIfMismatch) {

Review comment:
       The name puzzles me. Do you mean "use or reload"? Or "set or reload"?

##########
File path: extide/gradle/src/org/netbeans/modules/gradle/NbGradleProjectImpl.java
##########
@@ -174,14 +174,76 @@ private Lookup createBasicLookup(ProjectState state, GradleAuxiliaryConfigImpl a
                 state
         );
     }
+    
+    GradleProject getProjectInternal() {
+        synchronized (this) {
+            return project;
+        }
+    }
 
     public GradleProject getGradleProject() {
-        if (project == null) {
-            project = loadProject();
+        GradleProject p = getProjectInternal();
+        if (p != null) {
+            return p;
+        }
+        p = loadProject();
+        synchronized (this) {
+            // avod unnecessary project replacements.
+            if (project != null) {
+                return project;
+            }
+            project = p;
         }
-        return project;
+        return p;
     }
-
+    
+    /**
+     * Records a reloaded project instance. Indicates whether the project was replaced.
+     * The method refuses to replace a project that has been changed since the reload procedure
+     * started. Will also not replace the project, if the original has a better quality
+     * than the reloaded one.
+     * <p>
+     * Pass {@code original = null} to just replace the project without checking.
+     * 
+     * @param original the original project data instance.
+     * @param reloaded the reloaded data.
+     * @param reloadIfMismatch will fire new reload if the project differs from original or quality is not satisfactory.
+     * @return {@code null}, if the project was accepted. {@code Boolean.TRUE}, if the project has
+     * been changed independently; {@code Boolean.FALSE}, if the reloaded project's quality is lower than
+     * the current.
+     */
+    Boolean recordOrReloadProject(GradleProject original, GradleProject reloaded, boolean reloadIfMismatch) {
+        Boolean result = null;

Review comment:
       Why is `result` tri-state? Can it be named `mismatchOfOldAndNewProject`, right?

##########
File path: extide/gradle/src/org/netbeans/modules/gradle/NbGradleProjectImpl.java
##########
@@ -174,14 +174,76 @@ private Lookup createBasicLookup(ProjectState state, GradleAuxiliaryConfigImpl a
                 state
         );
     }
+    
+    GradleProject getProjectInternal() {
+        synchronized (this) {
+            return project;
+        }
+    }
 
     public GradleProject getGradleProject() {
-        if (project == null) {
-            project = loadProject();
+        GradleProject p = getProjectInternal();
+        if (p != null) {
+            return p;
+        }
+        p = loadProject();
+        synchronized (this) {
+            // avod unnecessary project replacements.
+            if (project != null) {
+                return project;
+            }
+            project = p;
         }
-        return project;
+        return p;
     }
-
+    
+    /**
+     * Records a reloaded project instance. Indicates whether the project was replaced.
+     * The method refuses to replace a project that has been changed since the reload procedure
+     * started. Will also not replace the project, if the original has a better quality
+     * than the reloaded one.
+     * <p>
+     * Pass {@code original = null} to just replace the project without checking.
+     * 
+     * @param original the original project data instance.
+     * @param reloaded the reloaded data.
+     * @param reloadIfMismatch will fire new reload if the project differs from original or quality is not satisfactory.
+     * @return {@code null}, if the project was accepted. {@code Boolean.TRUE}, if the project has
+     * been changed independently; {@code Boolean.FALSE}, if the reloaded project's quality is lower than
+     * the current.
+     */
+    Boolean recordOrReloadProject(GradleProject original, GradleProject reloaded, boolean reloadIfMismatch) {
+        Boolean result = null;
+        synchronized (this) {
+            if (original != null) {
+                if (project != original) {
+                    result = true;
+                } else if (original.getQuality().betterThan(reloaded.getQuality())) {
+                    result = false;
+                }
+            }
+            if (result == null) {

Review comment:
       this could be `} else if` and then `result` can be just plain `boolean` I think.

##########
File path: extide/gradle/src/org/netbeans/modules/gradle/NbGradleProjectImpl.java
##########
@@ -220,7 +282,9 @@ void detachAllUpdater() {
     }
 
     void dumpProject() {

Review comment:
       dump!? clear? release?

##########
File path: extide/gradle/src/org/netbeans/modules/gradle/NbGradleProjectImpl.java
##########
@@ -174,14 +174,76 @@ private Lookup createBasicLookup(ProjectState state, GradleAuxiliaryConfigImpl a
                 state
         );
     }
+    
+    GradleProject getProjectInternal() {
+        synchronized (this) {
+            return project;
+        }
+    }
 
     public GradleProject getGradleProject() {
-        if (project == null) {
-            project = loadProject();
+        GradleProject p = getProjectInternal();
+        if (p != null) {
+            return p;
+        }
+        p = loadProject();
+        synchronized (this) {
+            // avod unnecessary project replacements.
+            if (project != null) {
+                return project;
+            }
+            project = p;
         }
-        return project;
+        return p;
     }
-
+    
+    /**
+     * Records a reloaded project instance. Indicates whether the project was replaced.
+     * The method refuses to replace a project that has been changed since the reload procedure
+     * started. Will also not replace the project, if the original has a better quality
+     * than the reloaded one.
+     * <p>
+     * Pass {@code original = null} to just replace the project without checking.
+     * 
+     * @param original the original project data instance.
+     * @param reloaded the reloaded data.
+     * @param reloadIfMismatch will fire new reload if the project differs from original or quality is not satisfactory.
+     * @return {@code null}, if the project was accepted. {@code Boolean.TRUE}, if the project has

Review comment:
       Returned value seems to be unused in all invocations.

##########
File path: extide/gradle/src/org/netbeans/modules/gradle/NbGradleProjectImpl.java
##########
@@ -174,14 +174,76 @@ private Lookup createBasicLookup(ProjectState state, GradleAuxiliaryConfigImpl a
                 state
         );
     }
+    
+    GradleProject getProjectInternal() {
+        synchronized (this) {
+            return project;
+        }
+    }
 
     public GradleProject getGradleProject() {
-        if (project == null) {
-            project = loadProject();
+        GradleProject p = getProjectInternal();
+        if (p != null) {
+            return p;
+        }
+        p = loadProject();
+        synchronized (this) {
+            // avod unnecessary project replacements.

Review comment:
       typo




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists