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

Asynchronously decorate quick outline view via DecorationManager #1922 #1929

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

HeikoKlare
Copy link
Contributor

@HeikoKlare HeikoKlare commented Jan 11, 2025

What it does

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 DecorationManager 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

This complements #1926, which improves performance of the business logic used for calculating labels of the view, while this one ensures that the business logic is completely moved outside of the UI thread to be executed asynchronously.

How to test

A reproducer has been kindly provided by @fedejeanne in:

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

Without this patch, the quick outline view will take a high amout of time to open and block the UI while with this patch it will open immediately and stay responsible.

Author checklist

…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 HeikoKlare force-pushed the quick-outline-async-decoration branch from 8689feb to 7547bcc Compare January 11, 2025 09:22
@HeikoKlare HeikoKlare linked an issue Jan 11, 2025 that may be closed by this pull request
2 tasks
@HeikoKlare HeikoKlare marked this pull request as ready for review January 11, 2025 09:48
@HeikoKlare
Copy link
Contributor Author

@jukzi @fedejeanne this one complements #1926, which improves business logic of the quick outline view image calculation, while this one extracts the business logic out of the UI thread. Would be great if you could have a look and integrate this as well.

@jukzi
Copy link
Contributor

jukzi commented Jan 13, 2025

I am not familiar with that code, does this PR still fix https://bugs.eclipse.org/bugs/show_bug.cgi?id=62896 (which introduced that code)?

@HeikoKlare
Copy link
Contributor Author

Yes, it still fixes that PR. Here is a screencast to demonstrate the behavior:
quick_outline_override_indicator

The asynchronously executed decorator has been introduced to the ordinary outline view in 2008, so quite some time after the mentioned issue has been resolved:

fOutlineViewer.setLabelProvider(new DecoratingJavaLabelProvider(lprovider));

Maybe the quick outline was just forgotten to be adapted as well.

@jukzi
Copy link
Contributor

jukzi commented Jan 13, 2025

ok, if nobody complains i would merge it tomorrow.

@jukzi jukzi added the performance Issues related to performance. label Jan 13, 2025
@jukzi jukzi merged commit 42dc611 into eclipse-jdt:master Jan 14, 2025
10 checks passed
@HeikoKlare HeikoKlare deleted the quick-outline-async-decoration branch January 14, 2025 08:12
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 this pull request may close these issues.

Poor Performance of Quick Outline View
2 participants