Closed
Bug 581585
Opened 14 years ago
Closed 14 years ago
Cache the frame for caret
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
WORKSFORME
mozilla2.0b4
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
(Keywords: perf)
Attachments
(1 file)
(deleted),
patch
|
roc
:
review+
dbaron
:
approval2.0+
|
Details | Diff | Splinter Review |
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•14 years ago
|
||
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).
We already have a line cursor on blocks. Can we use that?
Comment 3•14 years ago
|
||
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.
Assignee | ||
Comment 4•14 years ago
|
||
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?
Assignee | ||
Comment 5•14 years ago
|
||
(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•14 years ago
|
||
> 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?
Assignee | ||
Comment 7•14 years ago
|
||
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.
Assignee | ||
Updated•14 years ago
|
Summary: Optimize nsLineBox::IndexOf → Cache the frame for caret
Assignee | ||
Comment 8•14 years ago
|
||
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.
Attachment #460972 -
Flags: review?(roc)
Attachment #460972 -
Flags: review?(roc) → review+
Assignee | ||
Updated•14 years ago
|
Attachment #460972 -
Flags: approval2.0?
Updated•14 years ago
|
Attachment #460972 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Comment 9•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
Assignee | ||
Comment 10•14 years ago
|
||
It should be noted that the code parts of this bug was backed out as part of bug 593211, which implemented a better optimization.
Resolution: FIXED → WONTFIX
Assignee | ||
Updated•14 years ago
|
Resolution: WONTFIX → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•