Closed Bug 429595 Opened 16 years ago Closed 16 years ago

nsTextFrame::UnionTextDecorationOverflow is a performance nightmare

Categories

(Core :: Layout: Text and Fonts, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

(Keywords: perf, regression)

Attachments

(1 file)

See bug 428608 comment 13.

In particular, for a textarea (which has a lot of selections due to spell-check), this function does a lot of work it shouldn't be doing, as far as I can tell.
Flags: blocking1.9?
Attached patch Possible fixSplinter Review
Or was there a reason to not check this flag?
Attachment #316341 - Flags: superreview?(roc)
Attachment #316341 - Flags: review?(masayuki)
Comment on attachment 316341 [details] [diff] [review]
Possible fix

That should be fine. Although since the testcase you care about *does* have selections, I wonder why this helps.
Attachment #316341 - Flags: superreview?(roc)
Attachment #316341 - Flags: superreview+
Attachment #316341 - Flags: review?(masayuki)
Attachment #316341 - Flags: review+
I think spell-check stops highlighting stuff once it's done a thousand or so, so in this case there are lots of textframes that are not selected.

For 2.0 we should rethink this selection stuff; the selection details thing should really be O(N) for getting it for N textframes all in a row, not the O(N^2) it seems to be right now.
Comment on attachment 316341 [details] [diff] [review]
Possible fix

Simple fix for a performance regression for long textareas.  Should be reasonably safe.
Attachment #316341 - Flags: approval1.9?
OS: Mac OS X → All
Hardware: PC → All
Comment on attachment 316341 [details] [diff] [review]
Possible fix

a1.9=beltzner
Attachment #316341 - Flags: approval1.9? → approval1.9+
Wouldn't hold back the release, but we're taking the patch.  :)

Flags: blocking1.9? → blocking1.9-
Checked in.
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Boris I tried to load my given Socorro URL with the huge textarea content. I cannot recognize a difference between the builds before the checkin and now. My compiled debug trunk build from before some minutes still totally hangs. Can I use it to verify this bug or do you have a better testcase?
That testcase is the relevant one, though I ran into this bug when saving it to disk and loading from there.  Loading from the web, I still suspect the table layout is what kills us.

Also, note that in a debug build, some parts of Gecko have different algorithmic complexity from opt builds, so something that is a hotspot in an opt build may not be the most expensive thing in a debug build.  Hence fixing it might have no effect on debug performance, while improving opt performance.  I usually don't bother to performance-test with debug builds as a result.
I had saved the crash report to disk some days ago to be sure that no modification by ted will affect the behavior when trying to verify your patch. 

Loading it from disk hangs my debug build for about 2 minutes with 100% cpu usage. After that the unresponsive script warning pops-up. Doing the same with the latest nightly build I can see a better perf. After about 20 seconds the cpu usage goes down again. Even the menu is accessible most of the time while the page is loaded.

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008042204 Minefield/3.0pre ID:2008042204
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla1.9
Assignee: nobody → bzbarsky
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: