Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poor Performance of Quick Outline View #1922

Open
fedejeanne opened this issue Jan 10, 2025 · 7 comments
Open

Poor Performance of Quick Outline View #1922

fedejeanne opened this issue Jan 10, 2025 · 7 comments
Labels
performance Issues related to performance.

Comments

@fedejeanne
Copy link
Contributor

(See issue #2 our internal backlog)

Current Behavior

The quick outline view (Ctrl+O) opens very slowly when the opened class is part of a large class hierarchy. The root cause seems to be a combination of two things:

  1. Loading the complete view is performed synchronously, thus the UI freezes while loading it.
  2. The action calculates the complete inheritance hierarchy of involved classes in order to show the "method overwrite indicator" labels for methods that overwrite methods of superclasses. This information is not cached in the AST and is calculated everytime the quick outline view is opened.

The ordinary outline view calculates the overwrite indicator labels asynchronously by spawning a decoration job and thus provides the same information but without blocking the UI.

Expected Behavior

The quick outline view shall open instantly and calculate and add additional information asynchronously. It should use the same behavior as in the ordinary outline view to improve responsiveness of the UI.
While loading the information in background, there should be an indicator that the view has not completely loaded all expected information in the quick outline view yet.

Existing Workaround

Disable "Java Method Overwrite Indicator" label.

8455de56-acf9-4dab-a1ad-1c94e1bfcced

Samples and reproducer

To reproduce, do the following:

  • Import this Java project: reproducer.zip
  • Make sure the "Java Method Overwrite Indicator" is active
  • Go to the class Broke1 and click Ctrl + o twice

This is the sample I got: Broke1-I20241230-1800.zip

I also sampled in our big WS (~2.000 plugins and fragments), it takes 757s to open the quick outline view:
MyECU_2025-03-M1.zip

@jukzi
Copy link
Contributor

jukzi commented Jan 10, 2025

Can you please try to give a simple selfcontaining reproducer like eclipse-jdt/eclipse.jdt.core#3327 (comment) instead of attachments. Also please add screenshots of sampling result (call tree + hotspots), ideally add stacktrace of hotspot as text.

@jukzi jukzi added the performance Issues related to performance. label Jan 10, 2025
@fedejeanne
Copy link
Contributor Author

I wasn't able to provide a self contained reproducer since the problem here only arises because of the sheer amount of classes and methods that need to be analyzed. Just to give you an idea of the complexity, the ZIP I provided (which is just a small part of the real issue) contains 1.138 of our own classes which are discoverable by navigating 1 class up the hierarchy and gathering (most of) the classes that the classes in the hierarchy know and need (again, our own classes, no JDK/Eclipse classes).

Some screenshots

Small reproducer

The problem seems to be deep down in the java.lang.ThreadLocal$ThreadLocalMap.getEntryAfterMiss () which is called from org.eclipse.jdt.internal.core.JavaModelManager.getInfo ()

image

Bigger sample (in our WS)

The sample from our big WS paints more or less the same picture but it only goes as deep as org.eclipse.jdt.internal.core.JavaModelManager.getInfo ()

image

@jukzi
Copy link
Contributor

jukzi commented Jan 10, 2025

A thousand classes does not sound like really many. It rather sounds that there is some loop in the hierarchy such that same Classes are visited multiple times. The shown hotspots do not include any I/O which is unlikely. Maybe you have excluded java.* from sampling?

@jukzi
Copy link
Contributor

jukzi commented Jan 10, 2025

i also do not understand that getEntryAfterMiss shows the same Total time as Self Time while it calls a lot of other staff. somethings wrong with that sampling data, All selftimes should add up to the total time.

@jukzi
Copy link
Contributor

jukzi commented Jan 10, 2025

if you add System.out.println(element.getElementName()); to org.eclipse.jdt.internal.core.JavaModelCache.getInfo(IJavaElement) you see that the same elements are evaluated over and over. You can simpler freeze the IDE if you just press ctrl+t once on Broke1. see also #1830
You could fix it probably like eclipse-jdt/eclipse.jdt.core@7ebcecc to avoid repeated evalutions.
Note that jdt can also show you the underlying problem in your type hierarchy by enabling redundant superinterface warning:
image
image

@fedejeanne
Copy link
Contributor Author

A thousand classes does not sound like really many. It rather sounds that there is some loop in the hierarchy such that same Classes are visited multiple times.

I added this output to org.eclipse.jdt.internal.core.JavaModelManager.getInfo(IJavaElement)

image

And pressed Ctrl + o twice in this class (A.java):

class A {
	public void myMethod() {}
}

class A2 extends A{
	@Override public void myMethod() {}
}

And then grouped the results and counted them with Excel. Here's the result:

Grouped calls

Column1;Column2
JavaModelManager.getInfo(module-info.class);484
JavaModelManager.getInfo(Object);432
JavaModelManager.getInfo(Object.class);170
JavaModelManager.getInfo(java.base);152
JavaModelManager.getInfo(wait);132
JavaModelManager.getInfo(myMethod);123
JavaModelManager.getInfo(SlowQuickOutline);114
JavaModelManager.getInfo(Override);109
JavaModelManager.getInfo();106
JavaModelManager.getInfo(A);84
JavaModelManager.getInfo(A2);75
JavaModelManager.getInfo(clone);44
JavaModelManager.getInfo(equals);44
JavaModelManager.getInfo(finalize);44
JavaModelManager.getInfo(getClass);44
JavaModelManager.getInfo(hashCode);44
JavaModelManager.getInfo(notify);44
JavaModelManager.getInfo(notifyAll);44
JavaModelManager.getInfo(toString);44
JavaModelManager.getInfo(wait0);42
JavaModelManager.getInfo(java);40
JavaModelManager.getInfo(java.lang);38
JavaModelManager.getInfo(A.java);22
JavaModelManager.getInfo(src);20
JavaModelManager.getInfo(java.datatransfer);16
JavaModelManager.getInfo(java.desktop);16
JavaModelManager.getInfo(java.logging);16
JavaModelManager.getInfo(java.net.http);16
JavaModelManager.getInfo(java.prefs);16
JavaModelManager.getInfo(java.rmi);16
JavaModelManager.getInfo(java.sql);16
JavaModelManager.getInfo(java.instrument);14
JavaModelManager.getInfo(java.management);14
JavaModelManager.getInfo(jdk.accessibility);6
JavaModelManager.getInfo(jdk.attach);6
JavaModelManager.getInfo(jdk.compiler);6
JavaModelManager.getInfo(jdk.dynalink);6
JavaModelManager.getInfo(jdk.httpserver);6
JavaModelManager.getInfo(jdk.incubator.vector);6
JavaModelManager.getInfo(jdk.internal.ed);6
JavaModelManager.getInfo(jdk.internal.jvmstat);6
JavaModelManager.getInfo(jdk.internal.le);6
JavaModelManager.getInfo(jdk.internal.opt);6
JavaModelManager.getInfo(jdk.jartool);6
JavaModelManager.getInfo(jdk.javadoc);6
JavaModelManager.getInfo(jdk.jconsole);6
JavaModelManager.getInfo(jdk.jdi);6
JavaModelManager.getInfo(jdk.jdwp.agent);6
JavaModelManager.getInfo(jdk.jfr);6
JavaModelManager.getInfo(jdk.jshell);6
JavaModelManager.getInfo(jdk.jsobject);6
JavaModelManager.getInfo(jdk.management);6
JavaModelManager.getInfo(jdk.management.agent);6
JavaModelManager.getInfo(jdk.management.jfr);6
JavaModelManager.getInfo(jdk.net);6
JavaModelManager.getInfo(jdk.nio.mapmode);6
JavaModelManager.getInfo(jdk.sctp);6
JavaModelManager.getInfo(jdk.security.auth);6
JavaModelManager.getInfo(jdk.security.jgss);6
JavaModelManager.getInfo(jdk.unsupported);6
JavaModelManager.getInfo(jdk.unsupported.desktop);6
JavaModelManager.getInfo(jdk.xml.dom);6
JavaModelManager.getInfo(jdk.zipfs);6
JavaModelManager.getInfo(java.transaction.xa);4
JavaModelManager.getInfo(java.xml);4
JavaModelManager.getInfo(jdk.charsets);4
JavaModelManager.getInfo(jdk.crypto.cryptoki);4
JavaModelManager.getInfo(jdk.crypto.ec);4
JavaModelManager.getInfo(jdk.crypto.mscapi);4
JavaModelManager.getInfo(jdk.editpad);4
JavaModelManager.getInfo(jdk.hotspot.agent);4
JavaModelManager.getInfo(jdk.internal.vm.ci);4
JavaModelManager.getInfo(jdk.internal.vm.compiler);4
JavaModelManager.getInfo(jdk.internal.vm.compiler.management);4
JavaModelManager.getInfo(jdk.jcmd);4
JavaModelManager.getInfo(jdk.jdeps);4
JavaModelManager.getInfo(jdk.jlink);4
JavaModelManager.getInfo(jdk.jpackage);4
JavaModelManager.getInfo(jdk.jstatd);4
JavaModelManager.getInfo(jdk.localedata);4
JavaModelManager.getInfo(jdk.naming.dns);4
JavaModelManager.getInfo(jdk.naming.rmi);4
JavaModelManager.getInfo(jdk.random);4
JavaModelManager.getInfo(B.java);2
JavaModelManager.getInfo(C.java);2
JavaModelManager.getInfo(I1.java);2
JavaModelManager.getInfo(java.compiler);2
JavaModelManager.getInfo(java.management.rmi);2
JavaModelManager.getInfo(java.naming);2
JavaModelManager.getInfo(java.scripting);2
JavaModelManager.getInfo(java.se);2
JavaModelManager.getInfo(java.security.jgss);2
JavaModelManager.getInfo(java.security.sasl);2
JavaModelManager.getInfo(java.sql.rowset);2
JavaModelManager.getInfo(java.xml.crypto);2
JavaModelManager.getInfo(module-info.java);2
JavaModelManager.getInfo(package-info);2
JavaModelManager.getInfo(ReproducerGitHub1922SlowQuickOutline.java);2

What surprises me is that there are several unnecessary things in there, like other classes/files (I1.java) and java/jdk modules. Also, even the necessary checks (JavaModelManager.getInfo(myMethod);123, JavaModelManager.getInfo(SlowQuickOutline);114) are done several times.

The shown hotspots do not include any I/O which is unlikely. Maybe you have excluded java.* from sampling?

I resampled the big WS and I see more or less the same as before (and still no reference to ThreadLocalMap.getEntryAfterMiss ()):

image

I didn't skip java.**

image

jukzi added a commit to jukzi/eclipse.jdt.ui that referenced this issue Jan 10, 2025
don't visit redundant superinterfaces more then once

eclipse-jdt#1922
fedejeanne added a commit to fedejeanne/eclipse.jdt.ui that referenced this issue Jan 10, 2025
fedejeanne added a commit to fedejeanne/eclipse.jdt.ui that referenced this issue Jan 10, 2025
fedejeanne added a commit to fedejeanne/eclipse.jdt.ui that referenced this issue Jan 10, 2025
HeikoKlare added a commit to HeikoKlare/eclipse.jdt.ui that referenced this issue Jan 11, 2025
…pse-jdt#1922

The Quick Outline View currently applies (i.e., calculates and draws)
decorations for override indicators synchronously. In case the
calculation of the indicator takes long, it blocks the UI.
The ordinary Outline View defers the responsibility of calculating and
applying decorations to the DecorationManager. This ensures that (1)
that task is performed asynchronously via a dedicated job and (2) that
the decoration manager takes care of the current user configuration
(i.e., whether those indicators shall be shown or not), which the Quick
Outline View currently replicates.

With this change, the Quick Outline View uses the same label decorator
as the ordinary Outline View, which makes the DecorationManagager
process the override label decorator asynchronously instead of applying
it synchronously.

Contributes to eclipse-jdt#1922
HeikoKlare added a commit to HeikoKlare/eclipse.jdt.ui that referenced this issue Jan 11, 2025
…pse-jdt#1922

The Quick Outline View currently applies (i.e., calculates and draws)
decorations for override indicators synchronously. In case the
calculation of the indicator takes long, it blocks the UI.
The ordinary Outline View defers the responsibility of calculating and
applying decorations to the DecorationManager. This ensures that (1)
that task is performed asynchronously via a dedicated job and (2) that
the decoration manager takes care of the current user configuration
(i.e., whether those indicators shall be shown or not), which the Quick
Outline View currently replicates.

With this change, the Quick Outline View uses the same label decorator
as the ordinary Outline View, which makes the DecorationManager
process the override label decorator asynchronously instead of applying
it synchronously.

Contributes to eclipse-jdt#1922
HeikoKlare added a commit to HeikoKlare/eclipse.jdt.ui that referenced this issue Jan 11, 2025
…pse-jdt#1922

The Quick Outline View currently applies (i.e., calculates and draws)
decorations for override indicators synchronously. In case the
calculation of the indicator takes long, it blocks the UI.
The ordinary Outline View defers the responsibility of calculating and
applying decorations to the DecorationManager. This ensures that (1)
that task is performed asynchronously via a dedicated job and (2) that
the decoration manager takes care of the current user configuration
(i.e., whether those indicators shall be shown or not), which the Quick
Outline View currently replicates.

With this change, the Quick Outline View uses the same label decorator
as the ordinary Outline View, which makes the DecorationManager
process the override label decorator asynchronously instead of applying
it synchronously.

Contributes to eclipse-jdt#1922
jukzi added a commit to jukzi/eclipse.jdt.ui that referenced this issue Jan 13, 2025
don't visit redundant superinterfaces more then once

eclipse-jdt#1922
@fedejeanne fedejeanne changed the title Poor Performance of Quick Outline View #2 Poor Performance of Quick Outline View Jan 13, 2025
@jukzi
Copy link
Contributor

jukzi commented Jan 13, 2025

Reproducer: one needs 2 files, the ReproducerGitHub1922RedundantTypeHierarchy must not be open in editor (to not use AST for computation).

package a;

interface TopLevel  extends ReproducerGitHub1922RedundantTypeHierarchy.I34 {} // Ctrl+O twice on TopLevel  
package a;

class ReproducerGitHub1922RedundantTypeHierarchy {

	interface I0 {}
	interface I1 extends I0 {}
	interface I2 extends I1, I0 {}
	interface I3 extends I2, I1 {}
	interface I4 extends I3, I2 {}
	interface I5 extends I4, I3 {}
	interface I6 extends I5, I4 {}
	interface I7 extends I6, I5 {}
	interface I8 extends I7, I6 {}
	interface I9 extends I8, I7 {}
	interface I10 extends I9, I8 {}
	interface I11 extends I10, I9 {}
	interface I12 extends I11, I10 {}
	interface I13 extends I12, I11 {}
	interface I14 extends I13, I12 {}
	interface I15 extends I14, I13 {}
	interface I16 extends I15, I14 {}
	interface I17 extends I16, I15 {}
	interface I18 extends I17, I16 {}
	interface I19 extends I18, I17 {}
	interface I20 extends I19, I18 {}
	interface I21 extends I20, I19 {}
	interface I22 extends I21, I20 {}
	interface I23 extends I22, I21 {}
	interface I24 extends I23, I22 {}
	interface I25 extends I24, I23 {}
	interface I26 extends I25, I24 {}
	interface I27 extends I26, I25 {}
	interface I28 extends I27, I26 {}
	interface I29 extends I28, I27 {}
	interface I30 extends I29, I28 {}
	interface I31 extends I30, I29 {}
	interface I32 extends I31, I30 {}
	interface I33 extends I32, I31 {}
	interface I34 extends I33, I32 {public abstract void x();}
}

jukzi added a commit that referenced this issue Jan 13, 2025
don't visit redundant superinterfaces more then once

#1922
@fedejeanne fedejeanne reopened this Jan 13, 2025
jukzi pushed a commit that referenced this issue Jan 14, 2025
The Quick Outline View currently applies (i.e., calculates and draws)
decorations for override indicators synchronously. In case the
calculation of the indicator takes long, it blocks the UI.
The ordinary Outline View defers the responsibility of calculating and
applying decorations to the DecorationManager. This ensures that (1)
that task is performed asynchronously via a dedicated job and (2) that
the decoration manager takes care of the current user configuration
(i.e., whether those indicators shall be shown or not), which the Quick
Outline View currently replicates.

With this change, the Quick Outline View uses the same label decorator
as the ordinary Outline View, which makes the DecorationManager
process the override label decorator asynchronously instead of applying
it synchronously.

Contributes to #1922
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Issues related to performance.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants