Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 362086

Summary: Compare Editor not performing/scaling well with large documents
Product: [ECD] Orion Reporter: Simon Kaegi <simon_kaegi>
Component: ClientAssignee: libing wang <libingw>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: eclipse.felipe, Silenio_Quarti, tomasz.zarna
Version: 0.3Keywords: performance
Target Milestone: 0.4   
Hardware: PC   
OS: Windows 7   
Whiteboard:

Description Simon Kaegi CLA 2011-10-26 11:10:49 EDT
Take a look at the git log on something like textview.js and then use the arrows to move from diff to diff quickly. The browser lags and on lesser hardware will hang. This is going to be a problem on devices like tablets so we need to understand why things are slow here.
Comment 1 Felipe Heidrich CLA 2011-10-26 12:04:09 EDT
I didn't some profiling this moring, here is the trancript of my talk with Simon:
Felipe Heidrich/Ottawa/IBM: something is wrong with his rullers
Felipe Heidrich/Ottawa/IBM: redrawing too much
Felipe Heidrich/Ottawa/IBM: he only has line numbering ruler
Felipe Heidrich/Ottawa/IBM: it should not kill performance
Felipe Heidrich/Ottawa/IBM: unfortunately the redraws are asynchronous
Felipe Heidrich/Ottawa/IBM: so I can't tell (looking at the profiler) from where they are coming


--- Libing, try removing the rulers and see if that makes the compare faster, just to make sure the problem in in the rulers indeed.
Comment 2 libing wang CLA 2011-10-26 12:17:26 EDT
(In reply to comment #1)
> I didn't some profiling this moring, here is the trancript of my talk with
> Simon:
> Felipe Heidrich/Ottawa/IBM: something is wrong with his rullers
> Felipe Heidrich/Ottawa/IBM: redrawing too much
> Felipe Heidrich/Ottawa/IBM: he only has line numbering ruler
> Felipe Heidrich/Ottawa/IBM: it should not kill performance
> Felipe Heidrich/Ottawa/IBM: unfortunately the redraws are asynchronous
> Felipe Heidrich/Ottawa/IBM: so I can't tell (looking at the profiler) from
> where they are coming
> 
> 
> --- Libing, try removing the rulers and see if that makes the compare faster,
> just to make sure the problem in in the rulers indeed.

Thanks Felipe, I will give it a shot.
Comment 3 libing wang CLA 2011-10-26 13:52:56 EDT
Yeah, I removed the overview ruler (the one that highlights the diffs) and it's getting much faster.
I will narrow down ON my ruler implementation and see what I can do there.
Comment 4 libing wang CLA 2011-10-26 14:15:24 EDT
(In reply to comment #3)
> Yeah, I removed the overview ruler (the one that highlights the diffs) and it's
> getting much faster.
> I will narrow down ON my ruler implementation and see what I can do there.

Some updates:
I just simplified the ruler implementation by just barely do anything. Still very slow. The I realized that the ruler.getStyle(lineIndex) got called 5000+ times every time when I do next/prev. Tracing back to textView.js @ line 5231, where textView detects a ruler change when handling scroll event.
I think we need a smarter generic way to update rulers.
Comment 5 libing wang CLA 2011-10-26 15:59:17 EDT
Talked to Silenio, the getStyle function should not be called anymore since editor introduced annotation. I have to re-implement the compare rulers. This will affect the inline compare editor as well.
Comment 6 libing wang CLA 2011-10-26 16:40:01 EDT
fixed with 03f33be552721932485cbd34e26aeea1d0691db9.
The return value of compare rulers has to a spars array.
Thanks Felipe and Silenio for helping helping this out.