581585 - Cache the frame for caret

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[(no longer active)]

We really need to make sure that we cache the computation results of nsLineBox::IndexOf as a frame property.  This would win us another 15% on the editing performance of large textareas.  This should be easy (and similar to bug 580869).  The only thing that I'm not sure of is when that cache should be cleared.  Roc, bz, do you have any ideas?

Comment 1

[Boris Zbarsky [:bzbarsky]]

The return value of IndexOf depends on the line, no? 

And in this case we're actually trying to optimize the results of findlinecontaining, right?

I would be fine with caching an nsLineBox* for the last FindLineContaining result and then starting the next search at that line.  We'd need to clear the cache when lines go away (possibly only when the cached line does, if it's cheap enough to do that test).

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We already have a line cursor on blocks. Can we use that?

Comment 3

[Boris Zbarsky [:bzbarsky]]

That line cursor has various limitations in it in terms of what it can be used for... and is invalidated on any frametree change.  Might be ok for editor's purposes, though.

Comment 4

[(no longer active)]

I don't really know anything about the line cursor.  Could you please point me to some relevant code so I can start learning my way through that code?

Comment 5

[(no longer active)]

(In reply to comment #1)
> The return value of IndexOf depends on the line, no? 

I'm not sure what you mean.

> And in this case we're actually trying to optimize the results of
> findlinecontaining, right?

Not necessarily.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> I'm not sure what you mean.

IndexOf returns the index of a given frame in a given line (possibly -1 if the frame is not in that line).  This operation is really only slow if we have lots of frames in the line, which usually only happens if we have lots of placeholders.  The other reason IndexOf shows up in profiles is if we're calling IndexOf on _lots_ of lines.  At that point, the call costs (can we inline it?) and other overhead add up.

As I understand, you'd like to optimize the latter case, right?  What do you propose to cache?

Comment 7

[(no longer active)]

I was actually misreading the profiles.  We're indeed making lots of nsLineBox::IndexOf calls, each of which takes very little time to finish.  I think a good way to mitigate this problem is to make the caret code (which is the largest caller of IndexOf in this case) cache the frame and frame offset that it has found instead of trying to compute them over and over.

Comment 8

[(no longer active)]

Attachment: Patch (v1)

With this patch, nsLineBox::IndexOf is completely removed from the profiles when pressing and holding down a key (or typing very quickly).  With this patch and the rest of the patches, we're 70-80% faster than what we were before the optimizations that I've made.  I'm continuing to profile different cases and see what other gains we can get.

Comment 9

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/4ee2f3cf11d0

Comment 10

[(no longer active)]

It should be noted that the code parts of this bug was backed out as part of bug 593211, which implemented a better optimization.