Bug 575864 - CoreASTProvider.getAST() / Reconciler freezes are sometimes just waits
Summary: CoreASTProvider.getAST() / Reconciler freezes are sometimes just waits
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 4.20   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.23 M3   Edit
Assignee: Jörg Kubitz CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
: 509999 (view as bug list)
Depends on:
Blocks: 509999
  Show dependency tree
 
Reported: 2021-09-07 10:52 EDT by Jörg Kubitz CLA
Modified: 2022-02-15 05:23 EST (History)
8 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jörg Kubitz CLA 2021-09-07 10:52:50 EDT
Up to 1 seconds (2* AbstractReconciler.fDelay) the reconciler just waits in

	at java.base@11.0.9.1/java.lang.Object.wait(Native Method)
	at org.eclipse.jface.text.reconciler.AbstractReconciler$BackgroundThread.run(AbstractReconciler.java:183)

and
	AbstractReconciler.java:169

before he even starts his work.

That stacktrace looks so innocent small - while the UI waits

 at CoreASTProvider.getAST(CoreASTProvider.java:174)

until CoreASTProvider.reconciled().
Comment 1 Jörg Kubitz CLA 2021-09-08 04:40:47 EDT
As far understand a single 0.5 second delay after a source change is intentional (to queue up multiple changes?).
But if some thread calls getAST() and waits for the reconcilation it would make sense to abort (notify) the wait. 
I do not have an idea yet how the getAst() could notify "the" (which?) AbstractReconciler he is waiting for. Any idea?
Comment 2 Sarika Sinha CLA 2021-09-08 04:56:01 EDT
Moving to UI for comments.
Comment 3 Noopur Gupta CLA 2021-09-08 07:35:52 EDT
(In reply to Jörg Kubitz from comment #1)
> I do not have an idea yet how the getAst() could notify "the" (which?)
> AbstractReconciler he is waiting for. 

That would probably need some mechanism to notify in org.eclipse.jface.text.reconciler.AbstractReconciler.
Comment 4 Jörg Kubitz CLA 2021-09-09 08:12:56 EDT
(In reply to Noopur Gupta from comment #3)
> That would probably need some mechanism to notify in
> org.eclipse.jface.text.reconciler.AbstractReconciler.

We neet to call a 

synchronized (fDirtyRegionQueue) {
 fDirtyRegionQueue.notifyAll();
}

in AbstractReconciler (which is not done for fDelay > 0 in AbstractReconciler.BackgroundThread.reset())

But how do we get the needed AbstractReconciler at CoreASTProvider.getAst()?
Comment 5 Andrey Loskutov CLA 2021-09-09 08:27:41 EDT
I haven't looked into details yet, but isn't this a duplicate of bug 575870? There is also a proposal for the fix, not sure if that also fixes this one?
Comment 6 Jörg Kubitz CLA 2021-09-09 08:49:38 EDT
(In reply to Andrey Loskutov from comment #5)
> I haven't looked into details yet, but isn't this a duplicate of bug 575870?
> There is also a proposal for the fix, not sure if that also fixes this one?

This change could lower the freeze in bug 575870 by up to 1 sec. However it is not clear if there still are other issues for freezes.

Proposed gerrit 185179 would just skip waiting (but only for hyperlinks) instead of interrupting the reconciler. (i do not know if we lose features by that)
Comment 7 Jörg Kubitz CLA 2021-10-21 16:37:32 EDT
the effect can be seen in org.eclipse.jdt.text.tests.contentassist.CodeCompletionTest

Most test takes ~ 0.5 second. 
7/28 seconds in "main" thread's getAst(), 
16 seconds in "JavaReconciler" threads's wait()
Comment 8 Eclipse Genie CLA 2021-10-24 12:03:40 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.text/+/186867
Comment 9 Eclipse Genie CLA 2021-10-24 12:15:54 EDT
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/186868
Comment 10 Mickael Istria CLA 2022-02-01 06:17:26 EST
Do you see this issue as an annoying behavior with regular usage of the IDE, or is it "only" during tests? If it's for tests, maybe we could just enable some internal option to set the reconcilier delay to 0 in tests.
Comment 11 Jörg Kubitz CLA 2022-02-01 06:24:11 EST
(In reply to Mickael Istria from comment #10)
> Do you see this issue as an annoying behavior with regular usage of the IDE,
> or is it "only" during tests? If it's for tests, maybe we could just enable
> some internal option to set the reconcilier delay to 0 in tests.

This is the root cause of annoying UI lags in the java IDE.
Comment 12 Mickael Istria CLA 2022-02-01 10:20:14 EST
(In reply to Jörg Kubitz from comment #11)
> This is the root cause of annoying UI lags in the java IDE.

Do you have steps to reproduce?
I tried running the completion tests locally and they all succeed in ~350ms. Am I in a lucky case? Any idea how I can trigger an unlucky case?
Comment 13 Jörg Kubitz CLA 2022-02-01 11:23:05 EST
(In reply to Mickael Istria from comment #12)
> Do you have steps to reproduce?

i added a junit for that! At master you could take AbstractReconcilerTest and delay to fReconciler.setDelay(5000) and then see all tests timeout.

> I tried running the completion tests locally and they all succeed in ~350ms.
> Am I in a lucky case? Any idea how I can trigger an unlucky case?

just run all test in org.eclipse.jdt.text.tests.contentassist.CodeCompletionTest.
for me without patch =24s. with patch=20s.
I do not know which particular test consumes most time in the wait. but for all together the reduction is pretty deterministic.
Comment 16 Lars Vogel CLA 2022-02-14 07:03:38 EST
Thanks, Jörg. If this also fixes Bug 509999, may daily work should be UI freeze free after which will significantly improve my Eclipse user experience.
Comment 17 Jörg Kubitz CLA 2022-02-14 07:21:31 EST
*** Bug 509999 has been marked as a duplicate of this bug. ***
Comment 18 Andrey Loskutov CLA 2022-02-14 08:44:06 EST
This causes permanent CPU load with latest build, looks like endless loop in

"org.eclipse.jdt.internal.ui.text.JavaReconciler" #183 daemon prio=1 os_prio=0 cpu=161402.45ms elapsed=162.43s tid=0x00007ffff22c7800 nid=0xcef9 runnable  [0x00007ffec8877000]
   java.lang.Thread.State: RUNNABLE
        at org.eclipse.jface.text.reconciler.AbstractReconciler$BackgroundThread.run(AbstractReconciler.java:176)

Please fix or revert ASAP.
Comment 19 Andrey Loskutov CLA 2022-02-14 08:47:19 EST
(In reply to Andrey Loskutov from comment #18)
> This causes permanent CPU load with latest build, looks like endless loop in
> 
> "org.eclipse.jdt.internal.ui.text.JavaReconciler" #183 daemon prio=1
> os_prio=0 cpu=161402.45ms elapsed=162.43s tid=0x00007ffff22c7800 nid=0xcef9
> runnable  [0x00007ffec8877000]
>    java.lang.Thread.State: RUNNABLE
>         at
> org.eclipse.jface.text.reconciler.AbstractReconciler$BackgroundThread.
> run(AbstractReconciler.java:176)
> 
> Please fix or revert ASAP.

Steps to reproduce: use I20220214-0600 build and open any Java editor. One CPU core will show constant load with stack above. Close editor => load is gone.

Note: if you plan to revert, please don't downgrade bundle version.
Comment 20 Jörg Kubitz CLA 2022-02-14 08:51:39 EST
(In reply to Andrey Loskutov from comment #18)
> This causes permanent CPU load with latest build, looks like endless loop in
> Please fix or revert ASAP.

i'll take a look.
Comment 21 Jörg Kubitz CLA 2022-02-14 09:09:24 EST
(In reply to Jörg Kubitz from comment #20)
> (In reply to Andrey Loskutov from comment #18)
> > This causes permanent CPU load with latest build, looks like endless loop in
> > Please fix or revert ASAP.
> 
> i'll take a look.

reproduce
1. new workspace
2. create java project
3. create java class => spinning with "waitFinish==true" but isDirty==false

when i then
4. actually edit something=> spinning with delay (waitFinish=false) and isDirty==false
Comment 22 Eclipse Genie CLA 2022-02-14 09:41:16 EST
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.text/+/190780
Comment 24 Eclipse Genie CLA 2022-02-14 11:16:15 EST
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/190792