Bug 326419

Comment 1 Dani Megert 2010-10-07 10:00:34 EDT

See also bug 326905 comment 4.

Comment 2 Deepak Azad 2010-10-27 13:09:04 EDT

No time left now for M3, will fix next week.

Comment 3 Deepak Azad 2010-11-22 07:38:39 EST

Created attachment 183566 [details]
fix

Comment 4 Dani Megert 2010-11-24 05:54:00 EST

The test looks good except for the warnings. Please make sure that the new class has no warning when committing to HEAD.

Please post the numbers when running the test with and without whitespace shown, so that we get a feeling how costly the feature is.

Also, this needs to be backported to 'perf_36x'. Let me know if you need help with that.

Comment 5 Deepak Azad 2010-11-25 06:12:36 EST

*Without whitespace characters painted*
testScrollTextEditorPageWise()' (average over 3 samples):
  CPU Time:                  5s         (95% in [4.42s, 5.59s])       

testScrollTextEditorLineWiseMoveCaret2()' (average over 3 samples):
  CPU Time:              11.84s         (95% in [10.56s, 13.13s])     

testScrollTextEditorLineWiseSelect2()' (average over 3 samples):
  CPU Time:               6.88s         (95% in [6.34s, 7.42s])        

testScrollTextEditorLineWise2()' (average over 3 samples):
  CPU Time:               5.23s         (95% in [4.54s, 5.92s])        



*With whitespace characters painted*
testScrollTextEditorPageWise()' (average over 3 samples):
  CPU Time:              12.11s         (95% in [10.68s, 13.54s])      

testScrollTextEditorLineWiseMoveCaret2()' (average over 3 samples):
  CPU Time:              16.14s         (95% in [14.17s, 18.12s])    


testScrollTextEditorLineWiseSelect2()' (average over 3 samples):
  CPU Time:               8.95s         (95% in [8.14s, 9.75s])       

testScrollTextEditorLineWise2()' (average over 3 samples):
  CPU Time:               7.25s         (95% in [6.03s, 8.47s])

Comment 6 Deepak Azad 2010-11-25 07:49:53 EST

Created attachment 183846 [details]
final fi

Comment 7 Deepak Azad 2010-11-25 07:53:31 EST

(In reply to comment #6)
> Created an attachment (id=183846) [details] [diff]
> final fix

Committed this patch to HEAD, and released in perf_36x branch.

(In reply to comment #1)
> See also bug 326905 comment 4.
Just realized that this is not done yet. Will close the bug after finishing this as well.

Comment 8 Deepak Azad 2010-11-26 05:24:25 EST

Created attachment 183907 [details]
additional fix

(In reply to comment #1)
> See also bug 326905 comment 4.

Patch avoids calling StyledTextContent#getTextRange(..) twice.

Dani, can you please commit this to HEAD.

Comment 9 Dani Megert 2010-11-29 06:41:44 EST

(In reply to comment #8)
> Created an attachment (id=183907) [details] [diff]
> additional fix
> 
> (In reply to comment #1)
> > See also bug 326905 comment 4.
> 
> Patch avoids calling StyledTextContent#getTextRange(..) twice.
> 
> Dani, can you please commit this to HEAD.

Does the WhitespaceCharacterPainterTest show an improvement with this fix?

Comment 10 Deepak Azad 2010-11-29 07:18:10 EST

> (In reply to comment #8)
> Does the WhitespaceCharacterPainterTest show an improvement with this fix?
Theoretically it should as one array copy is avoided. But the numbers do not change significantly. I ran the test again today with this fix and here are the numbers. 

*Without whitespace characters painted*
testScrollTextEditorPageWise()' (average over 3 samples):
  CPU Time:               5.76s         (95% in [3.74s, 7.79s])        

testScrollTextEditorLineWiseMoveCaret2()' (average over 3 samples):
  CPU Time:              12.38s         (95% in [11.47s, 13.28s])      

testScrollTextEditorLineWiseSelect2()' (average over 3 samples):
  CPU Time:               7.05s         (95% in [6.23s, 7.88s])        

testScrollTextEditorLineWise2()' (average over 3 samples):
  CPU Time:               5.89s         (95% in [4.73s, 7.05s])       


*With whitespace characters painted*
testScrollTextEditorPageWise()' (average over 3 samples):
  CPU Time:               12.5s         (95% in [12.19s, 12.82s])      

testScrollTextEditorLineWiseMoveCaret2()' (average over 3 samples):
  CPU Time:              16.44s         (95% in [14.77s, 18.1s])      

testScrollTextEditorLineWiseSelect2()' (average over 3 samples):
  CPU Time:               9.52s         (95% in [8.74s, 10.29s])      

testScrollTextEditorLineWise2()' (average over 3 samples):
  CPU Time:                6.7s         (95% in [6.08s, 7.32s])       (In reply to comment #9)

Comment 11 Dani Megert 2010-11-29 08:20:09 EST

(In reply to comment #8)
> Created an attachment (id=183907) [details] [diff]
> additional fix
> 
> (In reply to comment #1)
> > See also bug 326905 comment 4.
> 
> Patch avoids calling StyledTextContent#getTextRange(..) twice.
> 
> Dani, can you please commit this to HEAD.

Done.

Comment 12 Deepak Azad 2010-11-29 11:50:15 EST

(In reply to comment #11)
> Done.
Thanks.

Comment 13 Dani Megert 2010-12-07 06:20:09 EST

Verified in I20101130-0900.