You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "sdedic (via GitHub)" <gi...@apache.org> on 2023/05/29 14:25:09 UTC

[GitHub] [netbeans] sdedic opened a new pull request, #5998: Maven/priming optimization

sdedic opened a new pull request, #5998:
URL: https://github.com/apache/netbeans/pull/5998

   First of all, `log4j` is not a good example. The project is set up in a way that **requires** that JDK8 is **on the PATH** while maven `toolchains.xml` define JDK 9+. The project will not compile if (e.g.) JDK11 is on PATH or JDK8 is defined in toolchains.
   But the example surfaced several interesting things. Most importantly, maven processes for the parent *and subprojects* are launched in parallel. In reality, parent priming build is most likely to fetch all artifacts for all projects in the reactor - so running maven for a subproject is not necessary.  So it is worth to wait for preceding priming builds to complete before trying another one.
   Next bug is that, after priming, the problem reporter does not refresh early - so `SanityBuildAction` could not even re-check the project is broken.
   
   This PR:
   - serializes maven priming builds. There will be at most 1 priming build running. It could be relaxed to allow 1 priming build for a reactor; we'll see from how this first change will affect perceived IDE performance.
   - each priming build first checks, if the _updated_ maven model still reports missing artifacts. If not, it won't execute maven. It may be still reported through Progress API, but at least the costly external process (+ maven project model evaluation) will not happen
   - an artifact might be reported as missing in the model, when it is, actually downloaded already. If this is detected - when the now-not-missing artifact is going to be reported, the current evaluation restarts. This is attempted 3 times at most so no endless loop should happen.
   
   During prototyping, I've tried to avoid Java compilation - since for IDE purposes, only dependencies are important, not correctness of the project's sources. That would allow speedup of the priming process - and also better detection if the priming build fails: currently there's no failure detection, as work-in-progress sources can easily fail "priming" compilation. We would need to only report artifact download failures.
   Sadly with Maven builtin goals, it does not seem possible - I tried to use `dependency` plugin, but that one treats even intra-reactor dependencies (e.g. snapshots) the same way as external ones and attempts to resolve them from an external repository (instead of the reactor contents). So I reverted back to `install`. 
   
   There's important peculiarity: while `-SNAPSHOT` projects are `install`ed, nonsnapshot projects are just `package`d. This difference (based on top project) is applied to the whole reactor. In a case where the root project defines a non-snapshot version while the modules are actually snapshot versions, the priming fails.
   The difference is IMHO negligible - so I made the goals settable through `Preferences` (although this is not exposed in the UI) and `vscode` extension makes an experiment on its users to use `install` for both situations.
   
   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] sdedic commented on a diff in pull request #5998: #5978: Maven/priming optimization

Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #5998:
URL: https://github.com/apache/netbeans/pull/5998#discussion_r1226441444


##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -133,70 +151,108 @@ public Collection<? extends ProjectProblem> getProblems() {
      * @return project problems.
      */
     Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
-        final MavenProject prj = project.getLookup().lookup(NbMavenProject.class).getMavenProject();
+        return doGetProblems1(sync).first();
+    }
+        
+    /**
+     * Analyzes problem, returns list of problems and priming build status. The returned {@link Pair}
+     * contains the list of problems and true/false whether the priming build seems necessary. The last result
+     * is cached for the given maven model instance. If the project was reloaded, the problems will be computed
+     * again for the new project instance. The call might block waiting on the pending project reload. If `sync' 
+     * is false, the method will just post in request processor and return {@code null}.
+     * 
+     * @param sync if the call should complete synchronously
+     */
+    private Pair<Collection<ProjectProblem>, Boolean> doGetProblems1(boolean sync) {
+        final MavenProject updatedPrj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+        Callable<Pair<Collection<ProjectProblem>, Boolean>> c;
+    
         synchronized (this) {
-            LOG.log(Level.FINER, "Called getProblems for {0}", project);
             //lazy adding listener only when someone asks for the problems the first time
-            if (projectListenerSet.compareAndSet(false, true)) {
+            if (!projectListenerSet) {
+                projectListenerSet = true;
                 //TODO do we check only when the project is opened?
                 problemReporter = project.getLookup().lookup(NbMavenProjectImpl.class).getProblemReporter();
                 assert problemReporter != null;
                 project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
             
             }
-            
+            MavenProject o = analysedProject.get();
+            LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1}, current = {2}", 
+                    new Object[] { project, o == null ? 0 : System.identityHashCode(o), System.identityHashCode(updatedPrj) });
             //for non changed project models, no need to recalculate, always return the cached value
-            Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-            if (wasprocessed != null) {
-                Collection<ProjectProblem> cached = problemsCache.get();
-                LOG.log(Level.FINER, "Project was processed, cached is: {0}", cached);
+            Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+            if (o == updatedPrj && wasprocessed != null) {
+                Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                LOG.log(Level.FINER, "getProblems: Project was processed, cached is: {0}", cached);
                 if (cached != null) {
                     return cached;
                 }
             } 
-            Callable<Collection<? extends ProjectProblem>> c = new Callable<Collection<? extends ProjectProblem>>() {
-                @Override
-                public Collection<? extends ProjectProblem> call() throws Exception {
-                    Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-                    if (wasprocessed != null) {
-                        Collection<ProjectProblem> cached = problemsCache.get();
-                        LOG.log(Level.FINER, "Project was processed #2, cached is: {0}", cached);
-                        if (cached != null) {                            
-                            return cached;
+            
+            SanityBuildAction sba = cachedSanityBuild.get();
+            if (sba != null && sba.getPendingResult() == null) {
+                cachedSanityBuild.clear();
+            }
+            c = () -> {
+                // double check, the project may be invalidated during the time.
+                MavenProject prj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+                Object wasprocessed2 = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+                if (o == updatedPrj && wasprocessed2 != null) {
+                    Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                    LOG.log(Level.FINER, "getProblems: Project was processed #2, cached is: {0}", cached);
+                    if (cached != null) {                            
+                        return cached;
+                    }

Review Comment:
   > @sdedic this snippet isn't synchronized since it is in the callable block which can run in a different thread.
   
   Uh-oh. Thanks. Addressed in b8fb7c01d244



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] sdedic commented on a diff in pull request #5998: #5978: Maven/priming optimization

Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #5998:
URL: https://github.com/apache/netbeans/pull/5998#discussion_r1226441444


##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -133,70 +151,108 @@ public Collection<? extends ProjectProblem> getProblems() {
      * @return project problems.
      */
     Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
-        final MavenProject prj = project.getLookup().lookup(NbMavenProject.class).getMavenProject();
+        return doGetProblems1(sync).first();
+    }
+        
+    /**
+     * Analyzes problem, returns list of problems and priming build status. The returned {@link Pair}
+     * contains the list of problems and true/false whether the priming build seems necessary. The last result
+     * is cached for the given maven model instance. If the project was reloaded, the problems will be computed
+     * again for the new project instance. The call might block waiting on the pending project reload. If `sync' 
+     * is false, the method will just post in request processor and return {@code null}.
+     * 
+     * @param sync if the call should complete synchronously
+     */
+    private Pair<Collection<ProjectProblem>, Boolean> doGetProblems1(boolean sync) {
+        final MavenProject updatedPrj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+        Callable<Pair<Collection<ProjectProblem>, Boolean>> c;
+    
         synchronized (this) {
-            LOG.log(Level.FINER, "Called getProblems for {0}", project);
             //lazy adding listener only when someone asks for the problems the first time
-            if (projectListenerSet.compareAndSet(false, true)) {
+            if (!projectListenerSet) {
+                projectListenerSet = true;
                 //TODO do we check only when the project is opened?
                 problemReporter = project.getLookup().lookup(NbMavenProjectImpl.class).getProblemReporter();
                 assert problemReporter != null;
                 project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
             
             }
-            
+            MavenProject o = analysedProject.get();
+            LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1}, current = {2}", 
+                    new Object[] { project, o == null ? 0 : System.identityHashCode(o), System.identityHashCode(updatedPrj) });
             //for non changed project models, no need to recalculate, always return the cached value
-            Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-            if (wasprocessed != null) {
-                Collection<ProjectProblem> cached = problemsCache.get();
-                LOG.log(Level.FINER, "Project was processed, cached is: {0}", cached);
+            Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+            if (o == updatedPrj && wasprocessed != null) {
+                Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                LOG.log(Level.FINER, "getProblems: Project was processed, cached is: {0}", cached);
                 if (cached != null) {
                     return cached;
                 }
             } 
-            Callable<Collection<? extends ProjectProblem>> c = new Callable<Collection<? extends ProjectProblem>>() {
-                @Override
-                public Collection<? extends ProjectProblem> call() throws Exception {
-                    Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-                    if (wasprocessed != null) {
-                        Collection<ProjectProblem> cached = problemsCache.get();
-                        LOG.log(Level.FINER, "Project was processed #2, cached is: {0}", cached);
-                        if (cached != null) {                            
-                            return cached;
+            
+            SanityBuildAction sba = cachedSanityBuild.get();
+            if (sba != null && sba.getPendingResult() == null) {
+                cachedSanityBuild.clear();
+            }
+            c = () -> {
+                // double check, the project may be invalidated during the time.
+                MavenProject prj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+                Object wasprocessed2 = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+                if (o == updatedPrj && wasprocessed2 != null) {
+                    Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                    LOG.log(Level.FINER, "getProblems: Project was processed #2, cached is: {0}", cached);
+                    if (cached != null) {                            
+                        return cached;
+                    }

Review Comment:
   > @sdedic this snippet isn't synchronized since it is in the callable block which can run in a different thread.
   
   Uh-oh. Thanks.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] mbien commented on a diff in pull request #5998: #5978: Maven/priming optimization

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5998:
URL: https://github.com/apache/netbeans/pull/5998#discussion_r1218326592


##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -133,70 +151,108 @@ public Collection<? extends ProjectProblem> getProblems() {
      * @return project problems.
      */
     Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
-        final MavenProject prj = project.getLookup().lookup(NbMavenProject.class).getMavenProject();
+        return doGetProblems1(sync).first();
+    }
+        
+    /**
+     * Analyzes problem, returns list of problems and priming build status. The returned {@link Pair}
+     * contains the list of problems and true/false whether the priming build seems necessary. The last result
+     * is cached for the given maven model instance. If the project was reloaded, the problems will be computed
+     * again for the new project instance. The call might block waiting on the pending project reload. If `sync' 
+     * is false, the method will just post in request processor and return {@code null}.
+     * 
+     * @param sync if the call should complete synchronously
+     */
+    private Pair<Collection<ProjectProblem>, Boolean> doGetProblems1(boolean sync) {
+        final MavenProject updatedPrj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+        Callable<Pair<Collection<ProjectProblem>, Boolean>> c;
+    
         synchronized (this) {
-            LOG.log(Level.FINER, "Called getProblems for {0}", project);
             //lazy adding listener only when someone asks for the problems the first time
-            if (projectListenerSet.compareAndSet(false, true)) {
+            if (!projectListenerSet) {
+                projectListenerSet = true;
                 //TODO do we check only when the project is opened?
                 problemReporter = project.getLookup().lookup(NbMavenProjectImpl.class).getProblemReporter();
                 assert problemReporter != null;
                 project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
             
             }
-            
+            MavenProject o = analysedProject.get();
+            LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1}, current = {2}", 
+                    new Object[] { project, o == null ? 0 : System.identityHashCode(o), System.identityHashCode(updatedPrj) });
             //for non changed project models, no need to recalculate, always return the cached value
-            Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-            if (wasprocessed != null) {
-                Collection<ProjectProblem> cached = problemsCache.get();
-                LOG.log(Level.FINER, "Project was processed, cached is: {0}", cached);
+            Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+            if (o == updatedPrj && wasprocessed != null) {
+                Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                LOG.log(Level.FINER, "getProblems: Project was processed, cached is: {0}", cached);
                 if (cached != null) {
                     return cached;
                 }
             } 
-            Callable<Collection<? extends ProjectProblem>> c = new Callable<Collection<? extends ProjectProblem>>() {
-                @Override
-                public Collection<? extends ProjectProblem> call() throws Exception {
-                    Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-                    if (wasprocessed != null) {
-                        Collection<ProjectProblem> cached = problemsCache.get();
-                        LOG.log(Level.FINER, "Project was processed #2, cached is: {0}", cached);
-                        if (cached != null) {                            
-                            return cached;
+            
+            SanityBuildAction sba = cachedSanityBuild.get();
+            if (sba != null && sba.getPendingResult() == null) {
+                cachedSanityBuild.clear();
+            }
+            c = () -> {
+                // double check, the project may be invalidated during the time.
+                MavenProject prj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+                Object wasprocessed2 = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+                if (o == updatedPrj && wasprocessed2 != null) {
+                    Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                    LOG.log(Level.FINER, "getProblems: Project was processed #2, cached is: {0}", cached);
+                    if (cached != null) {                            
+                        return cached;
+                    }

Review Comment:
   if you synchronize this bit, the `problemsCache` can become a regular reference.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] sdedic commented on a diff in pull request #5998: #5978: Maven/priming optimization

Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #5998:
URL: https://github.com/apache/netbeans/pull/5998#discussion_r1226227763


##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -133,70 +151,108 @@ public Collection<? extends ProjectProblem> getProblems() {
      * @return project problems.
      */
     Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
-        final MavenProject prj = project.getLookup().lookup(NbMavenProject.class).getMavenProject();
+        return doGetProblems1(sync).first();
+    }
+        
+    /**
+     * Analyzes problem, returns list of problems and priming build status. The returned {@link Pair}
+     * contains the list of problems and true/false whether the priming build seems necessary. The last result
+     * is cached for the given maven model instance. If the project was reloaded, the problems will be computed
+     * again for the new project instance. The call might block waiting on the pending project reload. If `sync' 
+     * is false, the method will just post in request processor and return {@code null}.
+     * 
+     * @param sync if the call should complete synchronously
+     */
+    private Pair<Collection<ProjectProblem>, Boolean> doGetProblems1(boolean sync) {
+        final MavenProject updatedPrj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+        Callable<Pair<Collection<ProjectProblem>, Boolean>> c;
+    
         synchronized (this) {
-            LOG.log(Level.FINER, "Called getProblems for {0}", project);
             //lazy adding listener only when someone asks for the problems the first time
-            if (projectListenerSet.compareAndSet(false, true)) {
+            if (!projectListenerSet) {
+                projectListenerSet = true;
                 //TODO do we check only when the project is opened?
                 problemReporter = project.getLookup().lookup(NbMavenProjectImpl.class).getProblemReporter();
                 assert problemReporter != null;
                 project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
             
             }
-            
+            MavenProject o = analysedProject.get();
+            LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1}, current = {2}", 
+                    new Object[] { project, o == null ? 0 : System.identityHashCode(o), System.identityHashCode(updatedPrj) });
             //for non changed project models, no need to recalculate, always return the cached value
-            Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-            if (wasprocessed != null) {
-                Collection<ProjectProblem> cached = problemsCache.get();
-                LOG.log(Level.FINER, "Project was processed, cached is: {0}", cached);
+            Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+            if (o == updatedPrj && wasprocessed != null) {
+                Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                LOG.log(Level.FINER, "getProblems: Project was processed, cached is: {0}", cached);
                 if (cached != null) {
                     return cached;
                 }
             } 
-            Callable<Collection<? extends ProjectProblem>> c = new Callable<Collection<? extends ProjectProblem>>() {
-                @Override
-                public Collection<? extends ProjectProblem> call() throws Exception {
-                    Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-                    if (wasprocessed != null) {
-                        Collection<ProjectProblem> cached = problemsCache.get();
-                        LOG.log(Level.FINER, "Project was processed #2, cached is: {0}", cached);
-                        if (cached != null) {                            
-                            return cached;
+            
+            SanityBuildAction sba = cachedSanityBuild.get();
+            if (sba != null && sba.getPendingResult() == null) {
+                cachedSanityBuild.clear();
+            }
+            c = () -> {
+                // double check, the project may be invalidated during the time.
+                MavenProject prj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+                Object wasprocessed2 = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+                if (o == updatedPrj && wasprocessed2 != null) {
+                    Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                    LOG.log(Level.FINER, "getProblems: Project was processed #2, cached is: {0}", cached);
+                    if (cached != null) {                            
+                        return cached;
+                    }

Review Comment:
   Good, there's already `synchronized (MavenModelProblemsProvider.this)` around all uses.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] mbien commented on a diff in pull request #5998: #5978: Maven/priming optimization

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5998:
URL: https://github.com/apache/netbeans/pull/5998#discussion_r1227304507


##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -90,10 +91,19 @@ public class MavenModelProblemsProvider implements ProjectProblemsProvider, Inte
     
     private final PropertyChangeSupport support = new PropertyChangeSupport(this);
     private final Project project;
-    private final AtomicBoolean projectListenerSet = new AtomicBoolean(false);
-    private final AtomicReference<Collection<ProjectProblem>> problemsCache = new AtomicReference<Collection<ProjectProblem>>();
     private final PrimingActionProvider primingProvider = new PrimingActionProvider();
+
     private ProblemReporterImpl problemReporter;
+
+    // @GuardedBy(this)
+    private Pair<Collection<ProjectProblem>, Boolean> problemsCache = null;
+    // @GuardedBy(this)
+    private boolean projectListenerSet;
+
+    /**
+     * The Maven project that has been processed already.
+     */
+    private volatile Reference<MavenProject> analysedProject = new WeakReference<>(null);

Review Comment:
   volatile can be removed, since guarded by this.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] sdedic merged pull request #5998: #5978: Maven/priming optimization

Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic merged PR #5998:
URL: https://github.com/apache/netbeans/pull/5998


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] sdedic commented on a diff in pull request #5998: #5978: Maven/priming optimization

Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #5998:
URL: https://github.com/apache/netbeans/pull/5998#discussion_r1218304169


##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -144,59 +172,87 @@ Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
                 project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
             
             }
-            
+            MavenProject o = analysedProject.get();
+            LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1}, current = {2}", 
+                    new Object[] { project, o == null ? 0 : System.identityHashCode(o), System.identityHashCode(updatedPrj) });
             //for non changed project models, no need to recalculate, always return the cached value
-            Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-            if (wasprocessed != null) {
-                Collection<ProjectProblem> cached = problemsCache.get();
-                LOG.log(Level.FINER, "Project was processed, cached is: {0}", cached);
+            Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+            if (o == updatedPrj && wasprocessed != null) {
+                Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                LOG.log(Level.FINER, "getProblems: Project was processed, cached is: {0}", cached);
                 if (cached != null) {
                     return cached;
                 }
             } 
-            Callable<Collection<? extends ProjectProblem>> c = new Callable<Collection<? extends ProjectProblem>>() {
+            
+            SanityBuildAction sba = cachedSanityBuild.get();
+            if (sba != null && sba.getPendingResult() == null) {
+                cachedSanityBuild.clear();
+            }
+            c = new Callable<Pair<Collection<ProjectProblem>, Boolean>>() {
                 @Override
-                public Collection<? extends ProjectProblem> call() throws Exception {
-                    Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-                    if (wasprocessed != null) {
-                        Collection<ProjectProblem> cached = problemsCache.get();
-                        LOG.log(Level.FINER, "Project was processed #2, cached is: {0}", cached);
+                public Pair<Collection<ProjectProblem>, Boolean> call() throws Exception {
+                    // double check, the project may be invalidated during the time.
+                    MavenProject prj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+                    Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+                    if (o == updatedPrj && wasprocessed != null) {
+                        Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                        LOG.log(Level.FINER, "getProblems: Project was processed #2, cached is: {0}", cached);
                         if (cached != null) {                            
                             return cached;
                         }
                     } 
-                    List<ProjectProblem> toRet = new ArrayList<>();
-                    MavenExecutionResult res = MavenProjectCache.getExecutionResult(prj);
-                    if (res != null && res.hasExceptions()) {
-                        toRet.addAll(reportExceptions(res));
+                    int round = 0;
+                    List<ProjectProblem> toRet = null;
+                    while (round < 3) {
+                        try {
+                            synchronized (MavenModelProblemsProvider.this) {
+                                sanityBuildStatus = false;
+                                toRet = new ArrayList<>();
+                                MavenExecutionResult res = MavenProjectCache.getExecutionResult(prj);
+                                if (res != null && res.hasExceptions()) {
+                                    toRet.addAll(reportExceptions(res));
+                                }
+                                //#217286 doArtifactChecks can call FileOwnerQuery and attempt to aquire the project mutex.
+                                toRet.addAll(doArtifactChecks(prj));
+                                //mark the project model as checked once and cached
+                                prj.setContextValue(MavenModelProblemsProvider.class.getName(), new Object());
+                                synchronized(MavenModelProblemsProvider.this) {

Review Comment:
   Good catch ... in addition, the `firePropertyChange` should go outside.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] mbien commented on a diff in pull request #5998: #5978: Maven/priming optimization

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5998:
URL: https://github.com/apache/netbeans/pull/5998#discussion_r1226388091


##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -133,70 +151,108 @@ public Collection<? extends ProjectProblem> getProblems() {
      * @return project problems.
      */
     Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
-        final MavenProject prj = project.getLookup().lookup(NbMavenProject.class).getMavenProject();
+        return doGetProblems1(sync).first();
+    }
+        
+    /**
+     * Analyzes problem, returns list of problems and priming build status. The returned {@link Pair}
+     * contains the list of problems and true/false whether the priming build seems necessary. The last result
+     * is cached for the given maven model instance. If the project was reloaded, the problems will be computed
+     * again for the new project instance. The call might block waiting on the pending project reload. If `sync' 
+     * is false, the method will just post in request processor and return {@code null}.
+     * 
+     * @param sync if the call should complete synchronously
+     */
+    private Pair<Collection<ProjectProblem>, Boolean> doGetProblems1(boolean sync) {
+        final MavenProject updatedPrj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+        Callable<Pair<Collection<ProjectProblem>, Boolean>> c;
+    
         synchronized (this) {
-            LOG.log(Level.FINER, "Called getProblems for {0}", project);
             //lazy adding listener only when someone asks for the problems the first time
-            if (projectListenerSet.compareAndSet(false, true)) {
+            if (!projectListenerSet) {
+                projectListenerSet = true;
                 //TODO do we check only when the project is opened?
                 problemReporter = project.getLookup().lookup(NbMavenProjectImpl.class).getProblemReporter();
                 assert problemReporter != null;
                 project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
             
             }
-            
+            MavenProject o = analysedProject.get();
+            LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1}, current = {2}", 
+                    new Object[] { project, o == null ? 0 : System.identityHashCode(o), System.identityHashCode(updatedPrj) });
             //for non changed project models, no need to recalculate, always return the cached value
-            Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-            if (wasprocessed != null) {
-                Collection<ProjectProblem> cached = problemsCache.get();
-                LOG.log(Level.FINER, "Project was processed, cached is: {0}", cached);
+            Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+            if (o == updatedPrj && wasprocessed != null) {
+                Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                LOG.log(Level.FINER, "getProblems: Project was processed, cached is: {0}", cached);
                 if (cached != null) {
                     return cached;
                 }
             } 
-            Callable<Collection<? extends ProjectProblem>> c = new Callable<Collection<? extends ProjectProblem>>() {
-                @Override
-                public Collection<? extends ProjectProblem> call() throws Exception {
-                    Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-                    if (wasprocessed != null) {
-                        Collection<ProjectProblem> cached = problemsCache.get();
-                        LOG.log(Level.FINER, "Project was processed #2, cached is: {0}", cached);
-                        if (cached != null) {                            
-                            return cached;
+            
+            SanityBuildAction sba = cachedSanityBuild.get();
+            if (sba != null && sba.getPendingResult() == null) {
+                cachedSanityBuild.clear();
+            }
+            c = () -> {
+                // double check, the project may be invalidated during the time.
+                MavenProject prj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+                Object wasprocessed2 = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+                if (o == updatedPrj && wasprocessed2 != null) {
+                    Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                    LOG.log(Level.FINER, "getProblems: Project was processed #2, cached is: {0}", cached);
+                    if (cached != null) {                            
+                        return cached;
+                    }

Review Comment:
   @sdedic this snippet isn't synchronized since it is in the callable block which can run in a different thread.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] mbien commented on a diff in pull request #5998: #5978: Maven/priming optimization

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5998:
URL: https://github.com/apache/netbeans/pull/5998#discussion_r1227190025


##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -133,70 +151,108 @@ public Collection<? extends ProjectProblem> getProblems() {
      * @return project problems.
      */
     Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
-        final MavenProject prj = project.getLookup().lookup(NbMavenProject.class).getMavenProject();
+        return doGetProblems1(sync).first();
+    }
+        
+    /**
+     * Analyzes problem, returns list of problems and priming build status. The returned {@link Pair}
+     * contains the list of problems and true/false whether the priming build seems necessary. The last result
+     * is cached for the given maven model instance. If the project was reloaded, the problems will be computed
+     * again for the new project instance. The call might block waiting on the pending project reload. If `sync' 
+     * is false, the method will just post in request processor and return {@code null}.
+     * 
+     * @param sync if the call should complete synchronously
+     */
+    private Pair<Collection<ProjectProblem>, Boolean> doGetProblems1(boolean sync) {
+        final MavenProject updatedPrj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+        Callable<Pair<Collection<ProjectProblem>, Boolean>> c;
+    
         synchronized (this) {
-            LOG.log(Level.FINER, "Called getProblems for {0}", project);
             //lazy adding listener only when someone asks for the problems the first time
-            if (projectListenerSet.compareAndSet(false, true)) {
+            if (!projectListenerSet) {
+                projectListenerSet = true;
                 //TODO do we check only when the project is opened?
                 problemReporter = project.getLookup().lookup(NbMavenProjectImpl.class).getProblemReporter();
                 assert problemReporter != null;
                 project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
             
             }
-            
+            MavenProject o = analysedProject.get();
+            LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1}, current = {2}", 
+                    new Object[] { project, o == null ? 0 : System.identityHashCode(o), System.identityHashCode(updatedPrj) });
             //for non changed project models, no need to recalculate, always return the cached value
-            Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-            if (wasprocessed != null) {
-                Collection<ProjectProblem> cached = problemsCache.get();
-                LOG.log(Level.FINER, "Project was processed, cached is: {0}", cached);
+            Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+            if (o == updatedPrj && wasprocessed != null) {
+                Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                LOG.log(Level.FINER, "getProblems: Project was processed, cached is: {0}", cached);
                 if (cached != null) {
                     return cached;
                 }
             } 
-            Callable<Collection<? extends ProjectProblem>> c = new Callable<Collection<? extends ProjectProblem>>() {
-                @Override
-                public Collection<? extends ProjectProblem> call() throws Exception {
-                    Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-                    if (wasprocessed != null) {
-                        Collection<ProjectProblem> cached = problemsCache.get();
-                        LOG.log(Level.FINER, "Project was processed #2, cached is: {0}", cached);
-                        if (cached != null) {                            
-                            return cached;
+            
+            SanityBuildAction sba = cachedSanityBuild.get();
+            if (sba != null && sba.getPendingResult() == null) {
+                cachedSanityBuild.clear();
+            }
+            c = () -> {
+                // double check, the project may be invalidated during the time.
+                MavenProject prj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+                Object wasprocessed2 = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+                if (o == updatedPrj && wasprocessed2 != null) {
+                    Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                    LOG.log(Level.FINER, "getProblems: Project was processed #2, cached is: {0}", cached);
+                    if (cached != null) {                            
+                        return cached;
+                    }

Review Comment:
   thanks, probably my fault since I also suggested to make the callable a lambda :)



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] sdedic commented on a diff in pull request #5998: #5978: Maven/priming optimization

Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #5998:
URL: https://github.com/apache/netbeans/pull/5998#discussion_r1218306469


##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -133,9 +147,23 @@ public Collection<? extends ProjectProblem> getProblems() {
      * @return project problems.
      */
     Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
-        final MavenProject prj = project.getLookup().lookup(NbMavenProject.class).getMavenProject();
+        return doGetProblems1(sync).first();
+    }
+        
+    /**
+     * Analyzes problem, returns list of problems and priming build status. The returned {@link Pair}
+     * contains the list of problems and true/false whether the priming build seems necessary. The last result
+     * is cached for the given maven model instance. If the project was reloaded, the problems will be computed
+     * again for the new project instance. The call might block waiting on the pending project reload. If `sync' 
+     * is false, the method will just post in request processor and return {@code null}.
+     * 
+     * @param sync if the call should complete synchronously
+     */
+    private Pair<Collection<ProjectProblem>, Boolean> doGetProblems1(boolean sync) {
+        final MavenProject updatedPrj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+        Callable<Pair<Collection<ProjectProblem>, Boolean>> c;
+    
         synchronized (this) {
-            LOG.log(Level.FINER, "Called getProblems for {0}", project);
             //lazy adding listener only when someone asks for the problems the first time
             if (projectListenerSet.compareAndSet(false, true)) {

Review Comment:
   Addressed in be43c873a7cd



##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -144,59 +172,87 @@ Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
                 project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
             
             }
-            
+            MavenProject o = analysedProject.get();
+            LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1}, current = {2}", 
+                    new Object[] { project, o == null ? 0 : System.identityHashCode(o), System.identityHashCode(updatedPrj) });
             //for non changed project models, no need to recalculate, always return the cached value
-            Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-            if (wasprocessed != null) {
-                Collection<ProjectProblem> cached = problemsCache.get();
-                LOG.log(Level.FINER, "Project was processed, cached is: {0}", cached);
+            Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+            if (o == updatedPrj && wasprocessed != null) {
+                Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                LOG.log(Level.FINER, "getProblems: Project was processed, cached is: {0}", cached);
                 if (cached != null) {
                     return cached;
                 }
             } 
-            Callable<Collection<? extends ProjectProblem>> c = new Callable<Collection<? extends ProjectProblem>>() {
+            
+            SanityBuildAction sba = cachedSanityBuild.get();
+            if (sba != null && sba.getPendingResult() == null) {
+                cachedSanityBuild.clear();
+            }
+            c = new Callable<Pair<Collection<ProjectProblem>, Boolean>>() {
                 @Override
-                public Collection<? extends ProjectProblem> call() throws Exception {
-                    Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-                    if (wasprocessed != null) {
-                        Collection<ProjectProblem> cached = problemsCache.get();
-                        LOG.log(Level.FINER, "Project was processed #2, cached is: {0}", cached);
+                public Pair<Collection<ProjectProblem>, Boolean> call() throws Exception {
+                    // double check, the project may be invalidated during the time.
+                    MavenProject prj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+                    Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+                    if (o == updatedPrj && wasprocessed != null) {
+                        Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                        LOG.log(Level.FINER, "getProblems: Project was processed #2, cached is: {0}", cached);
                         if (cached != null) {                            
                             return cached;
                         }
                     } 
-                    List<ProjectProblem> toRet = new ArrayList<>();
-                    MavenExecutionResult res = MavenProjectCache.getExecutionResult(prj);
-                    if (res != null && res.hasExceptions()) {
-                        toRet.addAll(reportExceptions(res));
+                    int round = 0;
+                    List<ProjectProblem> toRet = null;
+                    while (round < 3) {
+                        try {
+                            synchronized (MavenModelProblemsProvider.this) {
+                                sanityBuildStatus = false;
+                                toRet = new ArrayList<>();
+                                MavenExecutionResult res = MavenProjectCache.getExecutionResult(prj);
+                                if (res != null && res.hasExceptions()) {
+                                    toRet.addAll(reportExceptions(res));
+                                }
+                                //#217286 doArtifactChecks can call FileOwnerQuery and attempt to aquire the project mutex.
+                                toRet.addAll(doArtifactChecks(prj));
+                                //mark the project model as checked once and cached
+                                prj.setContextValue(MavenModelProblemsProvider.class.getName(), new Object());
+                                synchronized(MavenModelProblemsProvider.this) {

Review Comment:
   Addressed in be43c873a7cd



##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -144,59 +172,87 @@ Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
                 project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
             
             }
-            
+            MavenProject o = analysedProject.get();
+            LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1}, current = {2}", 
+                    new Object[] { project, o == null ? 0 : System.identityHashCode(o), System.identityHashCode(updatedPrj) });
             //for non changed project models, no need to recalculate, always return the cached value
-            Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-            if (wasprocessed != null) {
-                Collection<ProjectProblem> cached = problemsCache.get();
-                LOG.log(Level.FINER, "Project was processed, cached is: {0}", cached);
+            Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+            if (o == updatedPrj && wasprocessed != null) {
+                Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                LOG.log(Level.FINER, "getProblems: Project was processed, cached is: {0}", cached);
                 if (cached != null) {
                     return cached;
                 }
             } 
-            Callable<Collection<? extends ProjectProblem>> c = new Callable<Collection<? extends ProjectProblem>>() {
+            
+            SanityBuildAction sba = cachedSanityBuild.get();
+            if (sba != null && sba.getPendingResult() == null) {
+                cachedSanityBuild.clear();
+            }
+            c = new Callable<Pair<Collection<ProjectProblem>, Boolean>>() {

Review Comment:
   Addressed in be43c873a7cd



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] mbien commented on a diff in pull request #5998: #5978: Maven/priming optimization

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5998:
URL: https://github.com/apache/netbeans/pull/5998#discussion_r1212409120


##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -133,9 +147,23 @@ public Collection<? extends ProjectProblem> getProblems() {
      * @return project problems.
      */
     Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
-        final MavenProject prj = project.getLookup().lookup(NbMavenProject.class).getMavenProject();
+        return doGetProblems1(sync).first();
+    }
+        
+    /**
+     * Analyzes problem, returns list of problems and priming build status. The returned {@link Pair}
+     * contains the list of problems and true/false whether the priming build seems necessary. The last result
+     * is cached for the given maven model instance. If the project was reloaded, the problems will be computed
+     * again for the new project instance. The call might block waiting on the pending project reload. If `sync' 
+     * is false, the method will just post in request processor and return {@code null}.
+     * 
+     * @param sync if the call should complete synchronously
+     */
+    private Pair<Collection<ProjectProblem>, Boolean> doGetProblems1(boolean sync) {
+        final MavenProject updatedPrj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+        Callable<Pair<Collection<ProjectProblem>, Boolean>> c;
+    
         synchronized (this) {
-            LOG.log(Level.FINER, "Called getProblems for {0}", project);
             //lazy adding listener only when someone asks for the problems the first time
             if (projectListenerSet.compareAndSet(false, true)) {

Review Comment:
   While looking through this I noticed that `projectListenerSet` is an atomic primitive but also accessed in a synchronized block. Looks like it can be a regular `boolean` since i don't think it is used anywhere else.
   
   Same for `problemsCache` which is an `AtomicReference` but could be a regular reference.
   
   It looks like the `synchronized` blocks were added later without removing the original concurrency mechanism.



##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -144,59 +172,87 @@ Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
                 project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
             
             }
-            
+            MavenProject o = analysedProject.get();
+            LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1}, current = {2}", 
+                    new Object[] { project, o == null ? 0 : System.identityHashCode(o), System.identityHashCode(updatedPrj) });
             //for non changed project models, no need to recalculate, always return the cached value
-            Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-            if (wasprocessed != null) {
-                Collection<ProjectProblem> cached = problemsCache.get();
-                LOG.log(Level.FINER, "Project was processed, cached is: {0}", cached);
+            Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+            if (o == updatedPrj && wasprocessed != null) {
+                Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                LOG.log(Level.FINER, "getProblems: Project was processed, cached is: {0}", cached);
                 if (cached != null) {
                     return cached;
                 }
             } 
-            Callable<Collection<? extends ProjectProblem>> c = new Callable<Collection<? extends ProjectProblem>>() {
+            
+            SanityBuildAction sba = cachedSanityBuild.get();
+            if (sba != null && sba.getPendingResult() == null) {
+                cachedSanityBuild.clear();
+            }
+            c = new Callable<Pair<Collection<ProjectProblem>, Boolean>>() {

Review Comment:
   could be a lambda if you want. Conserves some vertical space :)



##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -144,59 +172,87 @@ Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
                 project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
             
             }
-            
+            MavenProject o = analysedProject.get();
+            LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1}, current = {2}", 
+                    new Object[] { project, o == null ? 0 : System.identityHashCode(o), System.identityHashCode(updatedPrj) });
             //for non changed project models, no need to recalculate, always return the cached value
-            Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-            if (wasprocessed != null) {
-                Collection<ProjectProblem> cached = problemsCache.get();
-                LOG.log(Level.FINER, "Project was processed, cached is: {0}", cached);
+            Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+            if (o == updatedPrj && wasprocessed != null) {
+                Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                LOG.log(Level.FINER, "getProblems: Project was processed, cached is: {0}", cached);
                 if (cached != null) {
                     return cached;
                 }
             } 
-            Callable<Collection<? extends ProjectProblem>> c = new Callable<Collection<? extends ProjectProblem>>() {
+            
+            SanityBuildAction sba = cachedSanityBuild.get();
+            if (sba != null && sba.getPendingResult() == null) {
+                cachedSanityBuild.clear();
+            }
+            c = new Callable<Pair<Collection<ProjectProblem>, Boolean>>() {
                 @Override
-                public Collection<? extends ProjectProblem> call() throws Exception {
-                    Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-                    if (wasprocessed != null) {
-                        Collection<ProjectProblem> cached = problemsCache.get();
-                        LOG.log(Level.FINER, "Project was processed #2, cached is: {0}", cached);
+                public Pair<Collection<ProjectProblem>, Boolean> call() throws Exception {
+                    // double check, the project may be invalidated during the time.
+                    MavenProject prj = ((NbMavenProjectImpl)project).getFreshOriginalMavenProject();
+                    Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+                    if (o == updatedPrj && wasprocessed != null) {
+                        Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                        LOG.log(Level.FINER, "getProblems: Project was processed #2, cached is: {0}", cached);
                         if (cached != null) {                            
                             return cached;
                         }
                     } 
-                    List<ProjectProblem> toRet = new ArrayList<>();
-                    MavenExecutionResult res = MavenProjectCache.getExecutionResult(prj);
-                    if (res != null && res.hasExceptions()) {
-                        toRet.addAll(reportExceptions(res));
+                    int round = 0;
+                    List<ProjectProblem> toRet = null;
+                    while (round < 3) {
+                        try {
+                            synchronized (MavenModelProblemsProvider.this) {
+                                sanityBuildStatus = false;
+                                toRet = new ArrayList<>();
+                                MavenExecutionResult res = MavenProjectCache.getExecutionResult(prj);
+                                if (res != null && res.hasExceptions()) {
+                                    toRet.addAll(reportExceptions(res));
+                                }
+                                //#217286 doArtifactChecks can call FileOwnerQuery and attempt to aquire the project mutex.
+                                toRet.addAll(doArtifactChecks(prj));
+                                //mark the project model as checked once and cached
+                                prj.setContextValue(MavenModelProblemsProvider.class.getName(), new Object());
+                                synchronized(MavenModelProblemsProvider.this) {

Review Comment:
   one of the two `synchronized` blocks is redundant, probably the inner one?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] mbien commented on a diff in pull request #5998: #5978: Maven/priming optimization

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5998:
URL: https://github.com/apache/netbeans/pull/5998#discussion_r1212428487


##########
java/maven/src/org/netbeans/modules/maven/problems/MavenModelProblemsProvider.java:
##########
@@ -144,59 +172,87 @@ Collection<? extends ProjectProblem> doGetProblems(boolean sync) {
                 project.getLookup().lookup(NbMavenProject.class).addPropertyChangeListener(projectListener);
             
             }
-            
+            MavenProject o = analysedProject.get();
+            LOG.log(Level.FINER, "Called getProblems for {0}, analysed = {1}, current = {2}", 
+                    new Object[] { project, o == null ? 0 : System.identityHashCode(o), System.identityHashCode(updatedPrj) });
             //for non changed project models, no need to recalculate, always return the cached value
-            Object wasprocessed = prj.getContextValue(MavenModelProblemsProvider.class.getName());
-            if (wasprocessed != null) {
-                Collection<ProjectProblem> cached = problemsCache.get();
-                LOG.log(Level.FINER, "Project was processed, cached is: {0}", cached);
+            Object wasprocessed = updatedPrj.getContextValue(MavenModelProblemsProvider.class.getName());
+            if (o == updatedPrj && wasprocessed != null) {
+                Pair<Collection<ProjectProblem>, Boolean> cached = problemsCache.get();
+                LOG.log(Level.FINER, "getProblems: Project was processed, cached is: {0}", cached);
                 if (cached != null) {
                     return cached;
                 }
             } 
-            Callable<Collection<? extends ProjectProblem>> c = new Callable<Collection<? extends ProjectProblem>>() {
+            
+            SanityBuildAction sba = cachedSanityBuild.get();
+            if (sba != null && sba.getPendingResult() == null) {
+                cachedSanityBuild.clear();
+            }
+            c = new Callable<Pair<Collection<ProjectProblem>, Boolean>>() {

Review Comment:
   could be a lambda if you want. Conserves some vertical and horizontal space :)



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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