Community
Participate
Working Groups
When StyledText is filled with a big content and line spacing is setted with StyledText#setLineSpacing(int lineSpacing), the StyledText us unusable. It freezes every time that you type some character. To reproduce easily the problem, call StyledText#setLineSpacing(50) at the end of the SourceViewer(Composite parent, IVerticalRuler verticalRuler, IOverviewRuler overviewRuler, boolean showAnnotationsOverview, int styles) constructor and opens a Java Editor. The issue is that the call of StyledText#setLineSpacing call setVariableLineHeight which computes line height with StyledTextRenderer#getLineHeight(int lineIndex) instead of using StyledTextRenderer#getLineHeight(). The StyledTextRenderer#getLineHeight(int lineIndex) looks like this: --------------------------------------------- int getLineHeight(int lineIndex) { if (lineHeight[lineIndex] == -1) { calculate(lineIndex, 1); } return lineHeight[lineIndex]; } --------------------------------------------- With Java Editor when you type a character in the editor, the void StyledTextRenderer#reset(int startLine, int lineCount) is called and lineHeight[lineIndex] becomes -1. In other words when getLineHeight(int lineIndex) is called, it computes line height with calculate every time which caues dramatics performance problem. To fix this issue lineHeight should be set to -1 only if it needs and not evry time like when StyleRanges changed. It's a big issue and I hope I will have help for this issue because CodeMining which updates line spacing with StyledtextLineSpacingProvider is unusable with Java Editor which have big content (ex: StyledText.java). @Mickael @Alexander is there any chance that you help me with this issue please? I will try to create a gerrit patch, but I'm afraid to break the StyledText. Any help are welcome!
A solution would be to cache the line height to avoid invoking "calculate" when not necessary. However, if we cache the value, we also need a way to invalidate it and force to re-calculate when necessary. Is there already something like this in place in the LineSpacing api?
> A solution would be to cache the line height to avoid invoking "calculate" when not necessary. The cache already exists with lineHeight[] field. The problem is that this cache is invalidated every time (see in StyledTextRenderer lineHeight[...]=-1)). I think lineHeight[...]=-1 should not done in StyledTextRenderer when style ranges are applied. But the hard part is that lineHeight[...]=-1 is done * in several place: -> void reset(int startLine, int lineCount) (line 1116) -> void textChanging(TextChangingEvent event) (line 1482 & 1485) * reset and textChanging are called in a lot place That's why I say it's a very hard task -( I think we should have a reset(int startLine, int lineCount, boolean resetLineHeight) and set the resetLineHeight to false when line height must not be reseted. for textChanging case, I don't know.
Can you provide an SWT snippet that demonstrates the problem or reproduce the problem using the CustomControlExample?
New Gerrit change created: https://git.eclipse.org/r/115195
> Can you provide an SWT snippet that demonstrates the problem or reproduce the problem using the CustomControlExample? I have tried to do that, but I cannot reproduce the same case than JDT which is complex editor. It seems that problem performance comes from with the "JavaDocScanner" of JDT PresentationReconciler. In otherwise when I comment ------------------------------------------------------- dr= new DefaultDamagerRepairer(getJavaDocScanner()); reconciler.setDamager(dr, IJavaPartitions.JAVA_DOC); reconciler.setRepairer(dr, IJavaPartitions.JAVA_DOC); ------------------------------------------------------- of JDT JavaSourceViewerConfiguration#getPresentationReconciler performance are better. I have tried to fix this performance problem with the patch https://git.eclipse.org/r/#/c/115195/ Now I have better performance but I'm afraid with this patch to broke something. @Sarika please apply this gerrit patch and tell me if performance are better. @Alexander, @Dani, @Mickael goal of my gerrit patch is to start discussion, please help me!
(In reply to Angelo ZERR from comment #5) > @Alexander, @Dani, @Mickael goal of my gerrit patch is to start discussion, > please help me! We really need an SWT snippet or test case.
(In reply to Angelo ZERR from comment #5) > @Sarika please apply this gerrit patch and tell me if performance are better. > Yes, the performance is remarkably better with the patch.
> Yes, the performance is remarkably better with the patch. Thanks @Sarika for your feedback, but my gerrit patch is not very clean -( > We really need an SWT snippet or test case. @Dani after spending so many hours, I have found a snippet which causes the problem! Here the snippet: ------------------------------------------------ import org.eclipse.jface.text.Document; import org.eclipse.jface.text.DocumentEvent; import org.eclipse.jface.text.IDocument; import org.eclipse.jface.text.IDocumentListener; import org.eclipse.jface.text.JFaceTextUtil; import org.eclipse.jface.text.source.ISourceViewer; import org.eclipse.jface.text.source.SourceViewer; import org.eclipse.swt.SWT; import org.eclipse.swt.custom.StyleRange; import org.eclipse.swt.custom.StyledText; import org.eclipse.swt.layout.FillLayout; import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Shell; /** * Snippet which show in action the slow of StyledText when {@link StyledText#setLineSpacing(int)} is called and * {@link JFaceTextUtil#computeLineHeight(StyledText, int, int, int)} is called as soon as document changed. * This case comes from with JDT Java Editor when: * - setLineSpacing is called * - JFaceTextUtil#computeLineHeight is called by OverviewRuler * - JFaceTextUtil.isShowingEntireContents(fCachedTextWidget) is called by LineNumberRulerColumn * */ public class SlowStyledTextWithLineSpacingSnippet { public static void main(String[] args) { Display display = new Display(); Shell shell = new Shell(display); shell.setLayout(new FillLayout()); ISourceViewer sourceViewer = new SourceViewer(shell, null, SWT.V_SCROLL | SWT.BORDER); StyledText styledText = sourceViewer.getTextWidget(); // Comment the following line to have a fast StyledText styledText.setLineSpacing(1); IDocument document = new Document(); document.addDocumentListener(new IDocumentListener() { @Override public void documentChanged(DocumentEvent event) { // colorize the full text to reset the whole lineHeight of StyledTextRenderer asyncColorize(styledText); // here call of StyledText#getLinePixel are: // - fast when styledText.setLineSpacing is NOT called // - very slow when styledText.setLineSpacing is called computeLineHeight(styledText); } private void asyncColorize(StyledText styledText) { styledText.getDisplay().asyncExec(() -> { int length = styledText.getCharCount(); StyleRange style = new StyleRange(); style.start = 0; style.length = length; style.foreground = styledText.getDisplay().getSystemColor(SWT.COLOR_BLUE); styledText.replaceStyleRanges(0, length, new StyleRange[] { style }); }); } private void computeLineHeight(StyledText styledText) { int l = styledText.getLineCount(); System.err.println("Call of JFaceTextUtil.computeLineHeight"); long start = System.currentTimeMillis(); int lineHeight = JFaceTextUtil.computeLineHeight(styledText, 0, l, 0); System.err.println(lineHeight + " in " + (System.currentTimeMillis() - start) + "ms"); } @Override public void documentAboutToBeChanged(DocumentEvent event) { } }); sourceViewer.setDocument(document, null); document.set(getLongText()); shell.open(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } display.dispose(); } private static String getLongText() { StringBuilder s = new StringBuilder(); for (int i = 0; i < 20000; i++) { s.append(i + "\n"); } return s.toString(); } } ------------------------------------------------ I have tried to comment the usecase and code. Don't hesitate to ping me if you want more info. I'm so deseparated because I have not found a clean solution for the moment. Hope you will find a solution! Thanks for your help!
@Dani, @Sarika the performance problem can be done easily without code: * open StyledText.java in the JDT Editor * click on "Toggle Word Wrap (Alt+Shift+Y)" try to open completion , type something, the Java Editor is very slow. It's the same case than line spacing.
@Dani, it's exactly the same problem with word wrap issue https://bugs.eclipse.org/bugs/show_bug.cgi?id=484142 My fear is that it's an hard issue. I'm studying an idea to compute directly line height by using line spacing (and line spacing provider) without calling calculate for each line (which causes problem performance). If I can do something, I will create a gerrit patch with my idea.
@Dani, @Sarika, @Mickael, please see and review my new gerrit patch. It does the same thing than my first gerrit patch but just if line spacing is customized. In other case (like word wrap), it is the same behaviour than before. Thanks for your review!
(In reply to Angelo ZERR from comment #11) > @Dani, @Sarika, @Mickael, please see and review my new gerrit patch. It does > the same thing than my first gerrit patch but just if line spacing is > customized. In other case (like word wrap), it is the same behaviour than > before. > > Thanks for your review! The gerrit must be reviewed by SWT team.
The issue I see with this snippet is that while it allows to understand what can cause the issue (computing all line heights on every change), it's not really showing how the JDT editor or other editor behaves in most cases AFAIK, and if it behaves like this, it should probably be fixed to minimize the number of invocations. If you apply the same initial codemining than the one that highlighted this issue (IIRC JUnit runner or JDT reference count) on the Generic Editor or plain text editor for instance, do you get the same performance issue?
> The issue I see with this snippet is that while it allows to understand what can cause the issue (computing all line heights on every change), it's not really showing how the JDT editor or other editor behaves in most cases AFAIK @Mickael, I have tried to simplify case by computing the whole lines. But it is the case with Java Editor fi you open the StyledText.java file, Styledtext#replaceStyleRanges is called several times with a ling range which reset every time the lineHeight. * when you are in "fixed" line height mode, only renderer.calculate are called for visible lines. For the "non" visible lines renderer.calculate is not called because the compute of line height doesn't use renderer.calculate but it uses . * when you are in "variable" line height mode, the renderer.calculate is called for each lines which causes the problem performance. My gerrit patch tried to be in the same case of "fixed" line height mode for line height by computing directly line height with line spacing for "non visible" lines to avoid calling renderer.calculate. > If you apply the same initial codemining than the one that highlighted this issue (IIRC JUnit runner or JDT reference count) on the Generic Editor or plain text editor for instance, do you get the same performance issue? Try to open with GenericEditor and you will see the same performance problem.
I tried the snippet and can reproduce the issue. It seems to me that if I uncomment the `styledText.replaceStyleRanges(0, length, new StyleRange[] { style });` line, then everything is fine. Is it something everyone can reproduce? If so, and if the styleRanges are the cause of the issue in JDT and Generic Editor (is it in Generic Editor since there is no presentation reconcilier by default?), then I have the impression it would be possible to improve the setStyleRange method so that it wouldn't invalidate cache on all lines, but only on the lines whose height is affected by the style change (ie if the current style on the line has a different font size). If we do that, then in the vast majority of cases, the cache would be mostly unchanged, and the computation of line height should happen way less often.
> I tried the snippet and can reproduce the issue. It seems to me that if I uncomment the `styledText.replaceStyleRanges(0, length, new StyleRange[] { style });` line, then everything is fine. Yes it's the problem that I try to reproduce with JDT Java Editor. > If so, and if the styleRanges are the cause of the issue in JDT and Generic Editor (is it in Generic Editor since there is no presentation reconcilier by default?), Sorry I had forgotten to mention, that I had added a presentation reconciler with Genereic editor (forget my last comment). > then I have the impression it would be possible to improve the setStyleRange method so that it wouldn't invalidate cache on all lines, but only on the lines whose height is affected by the style change (ie if the current style on the line has a different font size). Even if you could fix that, you will have problem when StyledText is loaded and renderer#calculate is called outside the StyledText. It will freeze the editor if you loop for each lines and call renderer#calculate. My snippet reproduce this problem. If you comment the call of computeLineHeight, you will see that there is no freeze on load. The call of JFaceTextUtils#computeLineHeight while StyledText loading (call of the first replaceStyleRange) freezes the StyledWidget. This case emulate the call of JFaceTextUtils#computeLineHeight with IOverviewRuler, and other component. If we do that, then in the vast majority of cases, the cache would be mostly unchanged, and the computation of line height should happen way less often.
(In reply to Angelo ZERR from comment #16) > Even if you could fix that, you will have problem when StyledText is loaded Well, it seems fair that opening a big file can take some time, I don't think it's really the issue we're trying to fix it, nor that the load time is a higher priority issue in the scenarios we're working on. > and renderer#calculate is called outside the StyledText. >It will freeze the editor if you loop for each lines and call renderer#calculate. I have the impression an extender should call renderer.calculate. Instead, the StyledText could provide the resetCache() or a similar method as API if some special code wants to make sure the layout of a line would be recomputed. It seems like instead of forcing a calculate on all lines, a client should smartly evaluate which lines are actually affected by the given operation and reset the cache for this line. Although there is no API to explicitly invalidate cache for one line, calling something like `styledText.setLineAlignment(lineNumber, 1, styledText.getLineAlignment(lineNumber))` should result in cache invalidated only for one line, and than calculate only re-invoked for this line.
To me, the snippet is more about (new) bug 530019. About linespacing, there is not really an issue as far as I understand. The issue is more that either JDT invokes the renderer too much when it could be avoided, and/or it's bug 530019.
New Gerrit change created: https://git.eclipse.org/r/116962
Gerrit change https://git.eclipse.org/r/116962 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=3766440924781bde917d540b5ac6669f02d7ea2a
Thanks Angelo!
> Thanks Angelo! Wow it's a very cool news! Thanks Mickael to help me to improve StyledText. I have continued to study why my CPU grows every time and it seems the problem comes from the call of StyledTextRenderer#calculateIdle() every time, even if you select a line. The call of calculateIdle loops for each lines and call calculate() for the all lines which are not computed (the resest of styles provides this problem, and TextLayout are called for almost the whole lines). As it is done in background, freeze is not visible (a little), but CPU grows everytime. To show that, I suggest you to add --------------------------------------------- System.err.println("calculate ->" + i); --------------------------------------------- in the calculate method: --------------------------------------------- void calculate(int startLine, int lineCount) { int endLine = startLine + lineCount; if (startLine < 0 || endLine > lineSizes.length) { return; } int hTrim = styledText.leftMargin + styledText.rightMargin + styledText.getCaretWidth(); for (int i = startLine; i < endLine; i++) { LineSizeInfo line = getLineSize(i); if (line.needsRecalculateSize()) { System.err.println("calculate ->" + i); --------------------------------------------- You will see that TextLayout is everytime consumed even if you select a line! I would liek to discuss with this problem with you please. Thanks!
Can you please create another bug entry for this specific issue?
> Can you please create another bug entry for this specific issue? Done in https://bugs.eclipse.org/bugs/show_bug.cgi?id=530975 Hope we will able to fix it because it improve a lot CodeMining & Java Editor (load is fast and CPU and memory doesn't grow when caluclateIdle is not called).
