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/16 18:46:54 UTC
[GitHub] [netbeans] sdedic opened a new pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
sdedic opened a new pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765
Originally submitted in PR #2764, but separated for easier integration into its own PR.
## Prevent unnecessary reloads
When the project problem is being fixed, the project model is loaded again, but then the model is thrown away and loaded once again (unnecessarily). In 22424fb I've changed the behaviour to reuse the model - if another action did not alter the model in the meantime.
## Synchronization fixes
The `project` variable is accessed from several threads, at least from RELOAD_RP and any random thread that calls `NbGradleProjectImpl` getters. I've tried to synchronize that so the reference is replaced, but the `loadProject` is still executed outside mutexes and events are fired in RELOAD_RP.
----------------------------------------------------------------
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
[GitHub] [netbeans] sdedic commented on a change in pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765#discussion_r579035714
##########
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:
Originally I wanted to propagate the action to the caller: accepted (null), changed by other party (true), original valid, but replacement rejected (false).
----------------------------------------------------------------
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
[GitHub] [netbeans] lkishalmi commented on a change in pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
lkishalmi commented on a change in pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765#discussion_r579757411
##########
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:
Yes, tri-state looks bad, also, I do not understand the logic here, how does it fit in the picture. I feel I need more explanation. Maybe over some IM (Slack, Telegram, Zoom...)
My use case in question would be: There is a good resolvable Gradle project loaded, the user tweaks something in it's build.gradle file, then saves it. There is a syntax error in the new file. The loader, would return the previous GradleProject, but decrease it's quality from FULL -> EVALUATED, marking, that we cannot completely trust our current state, but that's the best we know so far. As far as I see, this code would prevent that behavior.
----------------------------------------------------------------
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
[GitHub] [netbeans] sdedic commented on a change in pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765#discussion_r579191044
##########
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:
Fixed in 2177f44a16
----------------------------------------------------------------
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
[GitHub] [netbeans] sdedic closed pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
sdedic closed pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765
--
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
[GitHub] [netbeans] sdedic commented on pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
sdedic commented on pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765#issuecomment-824086112
Changes in this PR became obsolete after #2747 - must be redone from scratch.
--
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
[GitHub] [netbeans] sdedic commented on a change in pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765#discussion_r579036504
##########
File path: extide/gradle/src/org/netbeans/modules/gradle/NbGradleProjectImpl.java
##########
@@ -220,7 +282,9 @@ void detachAllUpdater() {
}
void dumpProject() {
Review comment:
`dumpProject` name was not changed in this PR.
----------------------------------------------------------------
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
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [netbeans] sdedic commented on pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
sdedic commented on pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765#issuecomment-801113433
.... 26 days without review from the component owner; I remember there's a work underway that may cause conflicts; clashes - any ETA where the competing PR could be merged so I can finish this 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.
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 change in pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765#discussion_r579189471
##########
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:
tri-state was explained below. If seems reasonable I would just leave it as it is; javadoc documents all three possible return values.
----------------------------------------------------------------
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
[GitHub] [netbeans] sdedic commented on a change in pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765#discussion_r597605530
##########
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:
@lkishalmi I've checked the code again, recalling what I was thinking about that month ago: the code was written in such a way, as there can be multiple `loadProject` calls (potentialy) that would lead to `impl.project` replacement. The logic in the `recordOrReloadProject` basically ensures that if a project has been replaced **since the loadProject started** (the caller needs to save a reference), the `impl.project` will not be replaced for a worse version. But this is just for **competing** reloads.
Imagine a user clicking on Resolve problems, which will not make the project just `FULL` (whatever reason), but at the same time the background reload task will load the project and reaches `FULL_ONLINE`. If the final calls to `recordOrReloadProject` are reordered (different parallel threads) in an unfortunate sequence, the FULL_ONLINE read `impl.project` first, then it would degrade to FULL unnecessarily. The method's logic prevents that: if two replace processes are competing (caller has recorded `project` instance and passed it), the 'better of them' wins. Similar to AtomicReference.compareAndSet().
To be conservative, I've enabled this logic just from `GradleProjectProblemProvider` call. The other invocations pass `null` for `original`, so the comparison is disabled. So Reload action, reload based on desired quality change will always replace the project: I did not want to affect those occurrences, since I don't fully understand their semantics. I thought that it would not be good if "priming build" actually degraded already well-loaded project.
We can chat over the IM as you suggested or continue writing here. The logic to solve competing operations may be unnecessary.
--
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
[GitHub] [netbeans] sdedic commented on a change in pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765#discussion_r597605530
##########
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:
@lkishalmi I've checked the code again, recalling what I was thinking about that month ago: the code was written in such a way, as there can be multiple `loadProject` calls (potentialy) that would lead to `impl.project` replacement. The logic in the `recordOrReloadProject` basically ensures that if a project has been replaced **since the loadProject started** (the caller needs to save a reference), the `impl.project` will not be replaced for a worse version. But this is just for **competing** reloads.
Imagine a user clicking on Resolve problems, which will not make the project just `FULL` (whatever reason), but at the same time the background reload task will load the project and reaches `FULL_ONLINE`. If the final calls to `recordOrReloadProject` are reordered (different parallel threads) in an unfortunate sequence, the FULL_ONLINE read `impl.project` first, then it would degrade to FULL unnecessarily. The method's logic prevents that: if two replace processes are competing (caller has recorded `project` instance and passed it), the 'better of them' wins. Similar to AtomicReference.compareAndSet().
To be conservative, I've enabled this logic just from `GradleProjectProblemProvider` call. The other invocations pass `null` for `original`, so the comparison is disabled. So Reload action, reload based on desired quality change will always replace the project: I did not want to affect those occurrences, since I don't fully understand their semantics. I thought that it would not be good if "priming build" actually degraded already well-loaded project.
We can chat over the IM as you suggested or continue writing here. The logic to solve competing operations may be unnecessary; please decide.
--
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
[GitHub] [netbeans] sdedic commented on a change in pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765#discussion_r579034065
##########
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:
possibly. I was never strong in naming things.
----------------------------------------------------------------
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
[GitHub] [netbeans] sdedic commented on a change in pull request #2765: [NETBEANS-5318] Prevent unnecessary Gradle model reloads
Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2765:
URL: https://github.com/apache/netbeans/pull/2765#discussion_r596104532
##########
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:
OK, so degrading after reload is permitted. That's broken with this implementation - will revisit. 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.
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