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/09/04 09:26:47 UTC
[GitHub] [netbeans] matthiasblaesing opened a new pull request #3153: LSP Client Improvements (Foldmanager, Performance)
matthiasblaesing opened a new pull request #3153:
URL: https://github.com/apache/netbeans/pull/3153
In the last few weeks I noticed two issues in the LSP Client code. One is located in the way background tasks
are queue. From my analysis identical background tasks can be schedules multiple times and executed quickly
ater one another, causing performance issues. The second is error reporting from the fold manager complaining
about being provided with identical folds multiple times.
Please refer to the individual commit messages for more information.
--
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] matthiasblaesing commented on pull request #3153: LSP Client Improvements (Foldmanager, Performance)
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on pull request #3153:
URL: https://github.com/apache/netbeans/pull/3153#issuecomment-918426133
If noone objects, I'll merge this by the end of the week.
--
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] matthiasblaesing commented on a change in pull request #3153: LSP Client Improvements (Foldmanager, Performance)
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on a change in pull request #3153:
URL: https://github.com/apache/netbeans/pull/3153#discussion_r704678192
##########
File path: ide/lsp.client/src/org/netbeans/modules/lsp/client/LSPBindings.java
##########
@@ -432,14 +447,19 @@ public void runOnBackground(Runnable r) {
}
private static void scheduleBackgroundTask(RequestProcessor.Task req) {
- WORKER.post(req, DELAY);
+ synchronized(WORKER_SCHEDULED) {
+ if(! WORKER_SCHEDULED.containsKey(req)) {
+ WORKER_SCHEDULED.put(req, null);
+ WORKER.post(req, DELAY);
Review comment:
I think the behavior of my proposed solution and the your suggestion is slightly different.
From my observation (and my reading of the documentation of RequestProcessor), with RequestProcessor.Task#schedule, if an event occurs inside the 500ms timeframe, the execution of the background update will be postponed again for 500ms, if the the next event happens inside this 500ms frame, updates will be postponed again and so on. So the view is only updated, once the user stops changing things.
My solution would update after 500ms and again 500ms later.
I observed the behavior of the java editor and at least the navigator windows follows the principle described for RequestProcessor.Task#schedule and I never had a problem with that. 500ms seem to be short enough to be a good balance between wait time and resource usage.
I updated this PR accordingly.
--
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] jlahoda commented on a change in pull request #3153: LSP Client Improvements (Foldmanager, Performance)
Posted by GitBox <gi...@apache.org>.
jlahoda commented on a change in pull request #3153:
URL: https://github.com/apache/netbeans/pull/3153#discussion_r703797803
##########
File path: ide/lsp.client/src/org/netbeans/modules/lsp/client/LSPBindings.java
##########
@@ -432,14 +447,19 @@ public void runOnBackground(Runnable r) {
}
private static void scheduleBackgroundTask(RequestProcessor.Task req) {
- WORKER.post(req, DELAY);
+ synchronized(WORKER_SCHEDULED) {
+ if(! WORKER_SCHEDULED.containsKey(req)) {
+ WORKER_SCHEDULED.put(req, null);
+ WORKER.post(req, DELAY);
Review comment:
Hmmm, I apparently forgot how this works - I somehow managed to think `post` will defer the task when the same task is posted again, but that is wrong. But I don't think there is a need to keep track of the tasks manually - the correct idiom is (I hope I am right this time!) `req.schedule(DELAY);`. Should take care of the re-scheduling/moving to a new time. Should work similarly to the map (I hope!) but much simpler.
Sorry for the trouble.
##########
File path: ide/lsp.client/src/org/netbeans/modules/lsp/client/LSPBindings.java
##########
@@ -400,9 +403,17 @@ public InitializeResult getInitResult() {
return initResult;
}
- @SuppressWarnings("AccessingNonPublicFieldOfAnotherObject")
+ @SuppressWarnings({"AccessingNonPublicFieldOfAnotherObject", "NestedSynchronizedStatement"})
Review comment:
Seems like a mistake in the hint, will try to take a look.
--
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] matthiasblaesing merged pull request #3153: LSP Client Improvements (Foldmanager, Performance)
Posted by GitBox <gi...@apache.org>.
matthiasblaesing merged pull request #3153:
URL: https://github.com/apache/netbeans/pull/3153
--
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] matthiasblaesing commented on a change in pull request #3153: LSP Client Improvements (Foldmanager, Performance)
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on a change in pull request #3153:
URL: https://github.com/apache/netbeans/pull/3153#discussion_r704649861
##########
File path: ide/lsp.client/src/org/netbeans/modules/lsp/client/LSPBindings.java
##########
@@ -400,9 +403,17 @@ public InitializeResult getInitResult() {
return initResult;
}
- @SuppressWarnings("AccessingNonPublicFieldOfAnotherObject")
+ @SuppressWarnings({"AccessingNonPublicFieldOfAnotherObject", "NestedSynchronizedStatement"})
Review comment:
You mean the hint is wrong because the synchronisation is inside a lambda, that is not executed in the containing method? My first though was, that the hint is right, but after reading twice, you are right.
With the next push, this will be gone again, as as your proposed solution is much easier and does not need explicit synchronisation.
--
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