Bugzilla – Full Text Bug Listing
| Summary: | Improve support for files with very long lines | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Felipe Heidrich <eclipse.felipe> | ||||
| Component: | Editor | Assignee: | Project Inbox <orion.editor-inbox> | ||||
| Status: | REOPENED --- | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | bugzilla, chris, christophe.cornu+eclipse, john.camelon, kid, mihai.sucan, pyvesdev, rnikiten, Silenio_Quarti, simon_kaegi, vparfono | ||||
| Version: | unspecified | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Felipe Heidrich
Created attachment 201277 [details]
Zip file contains a 3MB text file - that text file contains 3 lines with extremely long lines, that hang the web browser when user clicks inside the orion editor
Pathological text file that contains a few lines with millions of characters
We hit this pb in RTC Web UI, with the attached file (inside the zip file). I know it is absurd to have a line with millions of characters, but some text files are machine generated. If handling that kind of file isn't realistic, we can also check line length and truncate content, letting user know. But if you think there is some way to handle this better, we'll be very interested. For our own reference, we're tracking this in https://jazz.net/jazz/resource/itemName/com.ibm.team.workitem.WorkItem/173822 On the iPad, opennig a file with a very long line (dojo.js), crashes the browser. We have a bug for this in Mozilla's bugzilla as well: https://bugzilla.mozilla.org/show_bug.cgi?id=687861 Note that that bug also has a suggestion of where the problem may lie: From Boris Zbarsky: I profiled this on Mac. Specifically, I held my mouse down on the right scroll arrow and then started the profiler, waited a bit, and stopped it, using keyboard shortcuts (Option+esc). The short of it is that 84% of the time is under reflow triggered from getBoundingClientRect calls coming off a scroll event. I have no idea what the heck Orion is doing from horizontal scroll events, but it may want to stop doing it. Looking at the profile, it includes at least lots of calls to getBoundingClentRect interspersed with .length gets on textnodes, .firstChild and .nextSibling gets on nodes, CSS property setting, CSS property getting, .scrollLeft gets, .style gets, createDocumentFragment calls (!), .clientHeight gets, .rows gets on tables, .selection gets on windows, .clientWidth gets, .scrollTop gets, .scrollWidth gets, .cells gets on table rows, .documentElement gets, .previousSibling gets, .style.height sets, etc, etc, etc. The combination of style changes and reflow flushes is presumably what really kills performance here. Why is Orion doing that, exactly? Also note that insertion with long lines does not perform well: https://bugzilla.mozilla.org/show_bug.cgi?id=687865 with another comment there from Boris on where the problem likely can be found. so far we have fixed: Bug 358645 - Line insertion is long lines is slow Bug 358628 - Horizontal scrolling with long lines is slow and choppy I'll try the attachment from comment 1 tomorrow. Maybe a silly question: would word wrapping help? At least it would make the file more readable. Word wrapping would most likely help. Editing really big files with many lines works fine. The problem is with really long lines, so if really long lines are rendered on many lines, things could improve a lot. At this point we are strongly hit by these performance issues. We are preparing the JavaScript debugger for a release to the public in Firefox and we use Orion. The major problem is that complex web applications like Twitter, Github and many others hold huge scripts (several MB) on a few lines. These make the JS debugger freeze the whole Firefox UI because Orion renders those extremely lines very slow. We would very much appreciate if the Orion team could spend more time on making Orion faster in such cases. We are disabling JS syntax highlighting for big files, but even with no syntax highlighting displaying and scrolling through very big files is very slow - we still get UI freezes. Is there ongoing work to fix this bug? Thank you! I started adding support for line wrapping. It is still in the beginnings. Work is being done in this branch: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/log/?h=editorWrapping Thanks for your quick reply! (In reply to comment #9) > I started adding support for line wrapping. It is still in the beginnings. Work > is being done in this branch: > > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/log/?h=editorWrapping Any bug I can CC myself to? So I know when this lands. When is this work planned to land in the main branch, stable/ready for use? Rob, from the devtools team, did some profiling of Orion and here are his results: https://bugzilla.mozilla.org/show_bug.cgi?id=748512#c3 Looking into the _updatePage() implementation I wonder if this could be restructured to do what it does in separate steps: 1. create any of the new elements in document fragments. do not touch the actual page. 2. update the page. 3. update sizes as needed for everything. Currently there's a strong mix between partial page updates, create/remove elements, update .styles and so on, mixed with getBoundingClientRect(), clientWidth/height and other similar actions which force the browser to reflow. If a clearer separation between these steps could be achieved Orion's performance would improve a lot, forcing far fewer reflows. Thanks! Isn't this how the _updatePage() is done already? There is a loop to remove old unnecessary lines, a loop to create new consecutive lines in a document fragment, a loop to measure the new lines, and then code that updates styling values. Maybe I am missing some case which is not surprising since that method is awful to read. Could you point me to some problem? (In reply to comment #12) > Isn't this how the _updatePage() is done already? There is a loop to remove > old unnecessary lines, a loop to create new consecutive lines in a document > fragment, a loop to measure the new lines, and then code that updates > styling values. > > Maybe I am missing some case which is not surprising since that method is > awful to read. Could you point me to some problem? The loops look like that, but I see .style being touched and it's surprising we get so many calls to getBoundingClientRect(). As you point out, that method is an awful read and hard to find which of those methods called in the tight loops causes reflows. (In reply to comment #13) > (In reply to comment #12) > > Isn't this how the _updatePage() is done already? There is a loop to remove > > old unnecessary lines, a loop to create new consecutive lines in a document > > fragment, a loop to measure the new lines, and then code that updates > > styling values. > > > > Maybe I am missing some case which is not surprising since that method is > > awful to read. Could you point me to some problem? > > The loops look like that, but I see .style being touched and it's surprising > we get so many calls to getBoundingClientRect(). As you point out, that > method is an awful read and hard to find which of those methods called in > the tight loops causes reflows. There are only two .style being touched in between the loops. They both do not run for FF (first is a hack for Safari and second for WebKit). -------------------------1---------------------- if (this._mouseWheelLine === child) { child.style.display = "none"; //$NON-NLS-0$ child.lineIndex = -1; } else { clientDiv.removeChild(child); } ----------------------2------------------------- if (isWebkit) { clientDiv.style.width = "auto"; //$NON-NLS-0$ } ---------------------------------------- I just noticed that the calls to _updateRuler() should be done before the loop to measure the lines. I will probably make that change but I did not see much of a difference in the "Page Down" performance test of demo.html. I've pushed a very simple change suggested by Bogdan where we will only style lines shorter than 2000 characters. This of course immediately improves the performance of long lines but is a temporary measure until we address the problem properly. http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=b45d660c55aca7dc9f305f39087680dfc7c561a3 With this patch looking at files with very long lines... 1) Firefox works correctly. 2) Chrome runs into trouble around 65536 characters and then slows down on input and shows no characters after that point 3) IE10 begins to behave strangely around 8000 characters with off by one errors on input In all cases behavior for the first 1000 or so characters on the line was fine. It was only once one went to the end of lines that problems began to occur. I still there is value in this patch as nothing incorrect is being introduced by this temporary workaround -- we're just now beginning to see the problems associated with long lines. Closing as part of a mass clean up of inactive bugs. Please reopen if this problem still occurs or is relevant to you. For more details see: https://dev.eclipse.org/mhonarc/lists/orion-dev/msg03444.html Still can reproduce this bug on https://orionhub.org. We have a bug for this one as well: https://github.com/eclipse/che/issues/5189 Steps to reproduce: Upload the file below and try to open it. https://raw.githubusercontent.com/aws/aws-sdk-php/master/src/data/iam/2010-05-08/api-2.json.php I ran into something like this while exploring the limits of the .class file constant pool. I wrote a Java program to dump-out a Java source file that looks like this:
public class Foo {
public static final String BAR = "" ... ;
}
The "..." includes a total of 65535 instances of the string '+ "a"' (without the single-quotes), so the line is quite long. But the file itself has very few other lines, so this is not a multi-megabyte file or anything like that.
Eclipse Oxygen does not seem to be able to open this source file. I'm on Mac and I get the color-wheel immediately and have to kill the process after waiting a few minutes.
Any news about that? > The "..." includes a total of 65535 instances of the string '+ "a"' (without > the single-quotes), so the line is quite long. Creating lines that long is simply kind of ähm... wrong. What about inserting returns / line feeds? > But the file itself has very > few other lines, so this is not a multi-megabyte file or anything like that. Doesn't change the above issue. (In reply to Vitalii Parfonov from comment #19) > Any news about that? Obviously not. A bit annoying, ignoring over 10 (in words: TEN!) year old issues. Sorry for being in bad mood. I just tried to open an over 100.000 lines long TypeScript file. (Really: Why do people create this?) Eclipse just hangs. My machine has 128g ram and 8 (x2) cores. It's 2023. What's wrong? |