mbien commented on code in PR #7705:
URL: https://github.com/apache/netbeans/pull/7705#discussion_r1740222524
##########
ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/ProjectReloadInternal.java:
##########
@@ -843,32 +891,34 @@ private void endOperation(Project p, Reloader reload) {
} catch (InterruptedException ex) {
}
}
-
+
RequestProcessor floader = loader;
- CompletableFuture<ProjectState> f = CompletableFuture.runAsync(()
-> nextReloader.initRound(), loader).
- thenCompose((v) -> nextReloader.start(floader));
- f.whenComplete((a, b) -> {
+ CompletableFuture<ProjectState> f = CompletableFuture.runAsync(()
-> fNextReloader.initRound(), loader).
+ thenCompose((v) -> fNextReloader.start(floader));
+ // run this cleanup in the dispatcher thread
+ f.whenCompleteAsync((a, b) -> {
Review Comment:
a is the state and b an exception. Please give things names, this makes it
more difficult to review on github than necessary.
simplified the block a bit
```java
CompletableFuture.runAsync(() -> fNextReloader.initRound(),
loader)
.thenCompose((v) ->
fNextReloader.start(floader))
.whenCompleteAsync((projectState, throwable) ->
{ // run this cleanup in the dispatcher thread
```
##########
ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/ProjectReloadInternal.java:
##########
@@ -743,32 +744,64 @@ public void runProjectAction(Project p, Runnable r) {
public void assertNoOperations() {
synchronized (this) {
- if (!pendingOperations.isEmpty()) {
+ Map<Project, ProjectOperations> ops = new
HashMap<>(this.pendingOperations);
+ ops.values().removeAll(this.terminatingOperations);
+ if (!ops.isEmpty()) {
System.err.println("Pending operations detected");
- for (Map.Entry<Project, ProjectOperations> en :
pendingOperations.entrySet()) {
+ for (Map.Entry<Project, ProjectOperations> en :
ops.entrySet()) {
ProjectOperations op = en.getValue();
System.err.println(en.getKey() + ": usage " + op.usage +
", pendingReloads: " + op.pendingReloads.size() + ", actions: " +
op.postponedActions.size());
for (Reloader r : op.pendingReloads) {
r.getOriginTrace().printStackTrace();
}
}
}
- if (!pendingOperations.isEmpty() || this.loaderProcessors.size()
!= PROJECT_RELOAD_CONCURRENCY) {
+ if (!ops.isEmpty() || this.loaderProcessors.size() !=
PROJECT_RELOAD_CONCURRENCY) {
throw new IllegalStateException();
}
}
}
+ private Collection<IdentityHolder> collectRelaeases(ProjectOperations op) {
+ Collection<IdentityHolder> releases = op.releases;
+ // this is copied from postCleanup, but can be done in batch without
+ // checking the project operation is in progress for each reference.
These are already removed
+ // from stateIdentity, so just check they did not obtain another one:
+ for (Iterator<IdentityHolder> it = releases.iterator(); it.hasNext();
) {
+ IdentityHolder expired = it.next();
+ ProjectStateData d = expired.state.get();
+ if (d == null) {
+ it.remove();
+ }
+ IdentityHolder h = stateIdentity.get(d);
+ if (h != null && h != expired) {
+ it.remove();
+ }
+ }
+ return releases;
+ }
Review Comment:
Relaeases -> Releases
this copies the releases list and simplifies a few things:
```java
private Collection<IdentityHolder> collectReleases(ProjectOperations op)
{
List<IdentityHolder> releases = new ArrayList<>(op.releases);
// this is copied from postCleanup, but can be done in batch without
// checking the project operation is in progress for each reference.
These are already removed
// from stateIdentity, so just check they did not obtain another one:
releases.removeIf(expired -> {
ProjectStateData d = expired.state.get();
if (d == null) {
return true;
}
IdentityHolder h = stateIdentity.get(d);
return h != null && h != expired;
});
return releases;
}
```
alternative don't copy the list but also return nothing, to make it clear
that the update is in-place
##########
ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/ProjectReloadInternal.java:
##########
@@ -843,32 +891,34 @@ private void endOperation(Project p, Reloader reload) {
} catch (InterruptedException ex) {
}
}
-
+
RequestProcessor floader = loader;
- CompletableFuture<ProjectState> f = CompletableFuture.runAsync(()
-> nextReloader.initRound(), loader).
- thenCompose((v) -> nextReloader.start(floader));
- f.whenComplete((a, b) -> {
+ CompletableFuture<ProjectState> f = CompletableFuture.runAsync(()
-> fNextReloader.initRound(), loader).
+ thenCompose((v) -> fNextReloader.start(floader));
+ // run this cleanup in the dispatcher thread
+ f.whenCompleteAsync((a, b) -> {
+ LOG.log(Level.FINER, "Return RP to the pool", new Object[] {
floader });
+ loaderProcessors.offer(floader);
try {
- LOG.log(Level.FINE, "Load end project {0} with request
{1}", new Object[] { nextReloader.project, nextReloader.request });
- if (b == null) {
- nextReloader.completePending.completeAsync(() -> a,
RELOAD_RP);
- } else {
- RELOAD_RP.post(() -> {
-
nextReloader.completePending.completeExceptionally(b);
- });
- }
- // this will eventually fire postponed events.
- endOperation(nextReloader.project, nextReloader);
- LOG.log(Level.FINER, "Return RP to the pool", new Object[]
{ floader });
+ LOG.log(Level.FINE, "Load end project {0} with request
{1}", new Object[] { fNextReloader.project, fNextReloader.request });
+ // postpone event delivery so that the events observers
see the Future as completed.
+ endOperation(fNextReloader.project, fNextReloader, () -> {;
+ if (b == null) {
+ fNextReloader.completePending.completeAsync(() ->
a, RELOAD_RP);
+ } else {
+ RELOAD_RP.post(() -> {
+
fNextReloader.completePending.completeExceptionally(b);
+ });
+ }
+ });
} catch (ThreadDeath td) {
throw td;
} catch (Throwable t) {
Exceptions.printStackTrace(t);
} finally {
- loaderProcessors.offer(floader);
}
Review Comment:
finally block can now be removed
##########
ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/ProjectReloadInternal.java:
##########
@@ -843,32 +891,34 @@ private void endOperation(Project p, Reloader reload) {
} catch (InterruptedException ex) {
}
}
-
+
RequestProcessor floader = loader;
- CompletableFuture<ProjectState> f = CompletableFuture.runAsync(()
-> nextReloader.initRound(), loader).
- thenCompose((v) -> nextReloader.start(floader));
- f.whenComplete((a, b) -> {
+ CompletableFuture<ProjectState> f = CompletableFuture.runAsync(()
-> fNextReloader.initRound(), loader).
+ thenCompose((v) -> fNextReloader.start(floader));
+ // run this cleanup in the dispatcher thread
+ f.whenCompleteAsync((a, b) -> {
+ LOG.log(Level.FINER, "Return RP to the pool", new Object[] {
floader });
+ loaderProcessors.offer(floader);
try {
- LOG.log(Level.FINE, "Load end project {0} with request
{1}", new Object[] { nextReloader.project, nextReloader.request });
- if (b == null) {
- nextReloader.completePending.completeAsync(() -> a,
RELOAD_RP);
- } else {
- RELOAD_RP.post(() -> {
-
nextReloader.completePending.completeExceptionally(b);
- });
- }
- // this will eventually fire postponed events.
- endOperation(nextReloader.project, nextReloader);
- LOG.log(Level.FINER, "Return RP to the pool", new Object[]
{ floader });
+ LOG.log(Level.FINE, "Load end project {0} with request
{1}", new Object[] { fNextReloader.project, fNextReloader.request });
+ // postpone event delivery so that the events observers
see the Future as completed.
+ endOperation(fNextReloader.project, fNextReloader, () -> {;
Review Comment:
redundant semicolon
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
