pressing Enter in a large textarea is very CPU intensive with Firefox 4.0
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Mozilla Firefox |
Fix Released
|
Medium
|
|||
firefox (Ubuntu) |
Fix Released
|
High
|
Chris Coulson | ||
Natty |
Fix Released
|
High
|
Chris Coulson |
Bug Description
Binary package hint: firefox
On up to date natty with Firefox 4.0, pressing Enter in a large textarea is very CPU intensive.
Go to:
https:/
Edit the page, and then click 'Enter' a few times at the top of the editing textarea. The CPU will spike. Holding 'Enter' pegs the CPU entirely (there is no need to commit your changes). This is a regression over 3.6 on Ubuntu 10.10.
In Mozilla Bugzilla #597627, Jonas-sicking (jonas-sicking) wrote : | #2 |
Actually, it appears adding enough text on one line that it wraps also slows things down. So it's not the newline character in itself that's the culprit.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #3 |
Right; it's the fact that we end up having to reflow over to the next line at all. We should still be terminating that reflow at the _next_ newline we hit. So if you have:
AAAAAAA
CCCCCCC
on two adjacent lines and you type a bunch of Bs after the As until you line-wrap, we shouldn't reflow past the end of the Cs if we're doing it right. Imo.
In Mozilla Bugzilla #597627, Roc-ocallahan (roc-ocallahan) wrote : | #4 |
One problem is that we can't insert a frame into the middle of the continuation chain. So if you need to split one line into two, the continuation frame for each later line N+1 becomes the continuation for line N (which requires reflow of course).
Should only be O(N) per split though.
In Mozilla Bugzilla #597627, Jonas-sicking (jonas-sicking) wrote : | #5 |
You seem to be right in that it's not n^2. If I duplicate the size of a 100KB text area the time it takes to add a newline early in the area goes from 3 seconds to 6 seconds on a reasonably new macbook pro.
Still very slow though. Slow enough that it's annoying to review big patches directly in bugzilla.
In Mozilla Bugzilla #597627, Roc-ocallahan (roc-ocallahan) wrote : | #6 |
We can probably insert a text frame into the continuation chain, but it's a little tricky.
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #7 |
(In reply to comment #6)
> We can probably insert a text frame into the continuation chain, but it's a
> little tricky.
Hmm, aren't text frame continuations stored as a linked list? Is there some other data structure which needs to get updated as well? Would you elaborate please?
In Mozilla Bugzilla #597627, Roc-ocallahan (roc-ocallahan) wrote : | #8 |
Actually I guess it might just work.
In Mozilla Bugzilla #597627, Jonas-sicking (jonas-sicking) wrote : | #9 |
What's very weird to me is that when doing the steps in comment 0, step 2 is much faster than step 4. Doesn't both (re)create the full set of line frames?
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #10 |
I'd think so, yes.... That's odd, indeed.
In Mozilla Bugzilla #597627, Jonas-sicking (jonas-sicking) wrote : | #11 |
Similarly, copying the full contents of the textarea, moving to the end, and pasting is also basically instant. Even though I would expect that to create/reflow just as many inline frames.
In fact, pasting a full copy the end is about 30 times faster than inserting a newline at the beginning.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #12 |
Yeah, that's really weird. roc, any idea what's going on there?
In Mozilla Bugzilla #597627, Roc-ocallahan (roc-ocallahan) wrote : | #13 |
Not without debugging it.
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #14 |
It seems to me that the majority of the problem here is textrun reconstruction for the entire textarea.
I saw this comment <http://
I didn't understand the comment though, and I'm not sure how to do what it says, but I'm pretty sure that we'll get a huge perf benefit if we can avoid invalidating those textruns unnecessarily.
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #15 |
Moving to Layout: Text which is probably a better component for this bug.
In Mozilla Bugzilla #597627, Roc-ocallahan (roc-ocallahan) wrote : | #16 |
Yeah, that is exactly what we need to do here. See
http://
to about
http://
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #17 |
That hypothesis doesn't really address the question from comment 9 or comment 11, as far as I can tell...
Also, that code runs when _any_ char is typed, no? Why is typing non-newlines fast? Just because we only reconstruct the textruns for the line that was edited?
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #18 |
(In reply to comment #17)
> That hypothesis doesn't really address the question from comment 9 or comment
> 11, as far as I can tell...
It doesn't. I tried figuring it out today but couldn't. Maybe tomorrow I'll notice something that I've missed today.
> Also, that code runs when _any_ char is typed, no? Why is typing non-newlines
> fast? Just because we only reconstruct the textruns for the line that was
> edited?
On second look, this code should have the same effect no matter what character is typed, which means that the textruns for the following lines are invalidated either way, which makes my theory in comment 14 invalid perhaps.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #19 |
Well, textrun invalidation is (relatively) cheap. If we then don't need to _reconstruct_ the textruns for the text outside the visible part of the textarea, things would be fast.
But give comment 9 and comment 11, it sounds to me like we reconstruct textruns multiple times when enter is pressed. Or something.
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #20 |
So, here's what's happening. We're actually reflowing three times:
1. Once from nsRefreshDriver
2. Once from nsTypedSelectio
3. Once from nsViewManager:
Why we reflow three times really puzzles me...
In Mozilla Bugzilla #597627, Tnikkel (tnikkel) wrote : | #21 |
Can you track what is requesting these reflows by seeing what is calling PresShell:
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #22 |
Also, I tried taking out the loop in question in comment 14 out, and it indeed fixes the perf problem.
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #23 |
(In reply to comment #21)
> Can you track what is requesting these reflows by seeing what is calling
> PresShell:
Probably... But given comment 22, I don't think that's going to be too useful now...
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #24 |
Created attachment 479929
Don't invalidate textruns
So, this is what I could make out of the comment in the code, but the code is very broken, in that it seems to update the frame content offsets to invalidate values. Does anybody have any idea what I've been missing here?
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #25 |
Again, typing _anything_ does the textrun invalidation. Pasting the text in starts with invalid textruns.
Give me a bit to look into this.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #26 |
So a bit of testing with a 4-line textarea....
I confirmed that typing non-newline characters reflows just the line they're typed on (as long as it doesn't wrap) while typing enter reflows all the lines.
I also confirmed that typing enter a single time clears textruns once via that loop and reflows each line exactly once.
I also confirmed that cutting and then pasting all the text in the textarea reflows each line exactly once. Also that it's much much faster than hitting enter.
I added a printf in ClearAllTextRun
if (aFrame-
printf(
}
and one at the beginning of nsTextFrame:
if (mContent-
mContent-
mContent-
printf(
}
Here's what the output looks like if I paste 4 lines into a textarea (the lines contain one char each: "a", "b", "c", and "d"):
Reflowing: 0x103bd8990
Reflowing: 0x103bd93a8
Reflowing: 0x103bd92f0
Reflowing: 0x103bd9198
Here's what happens if I hit space on the first line before the "a":
Clearing: 0x103bd8990
Clearing: 0x103bd93a8
Clearing: 0x103bd92f0
Clearing: 0x103bd9198
Reflowing: 0x103bd8990Here's what happens if I hit enter before the "d":
Clearing: 0x103bd9198
Reflowing: 0x103bd92f0
Reflowing: 0x103bd9198
Reflowing: 0x1041b06d8
Here's what happens if I hit enter before the "c":
Clearing: 0x103bd92f0
Clearing: 0x103bd9198
Reflowing: 0x103bd93a8
Reflowing: 0x103bd92f0
Clearing: 0x103bd92f0
Clearing: 0x103bd9198
Reflowing: 0x103bd9198
Reflowing: 0x1041b06d8Here's what I get if I hit enter before the "a":
Clearing: 0x103bd8990
Clearing: 0x103bd93a8
Clearing: 0x103bd92f0
Clearing: 0x103bd9198
Reflowing: 0x103bd8990
Clearing: 0x103bd8990
Clearing: 0x103bd93a8
Reflowing: 0x103bd93a8
Clearing: 0x103bd93a8
Clearing: 0x103bd92f0
Reflowing: 0x103bd92f0
Clearing: 0x103bd92f0
Clearing: 0x103bd9198
Reflowing: 0x103bd9198
Reflowing: 0x1041b06d8
So it looks like we clear all the textruns for the textarea from the CharacterDataCh
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #27 |
Er... some of the newlines in the previous comment disappeared :(
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #28 |
nsTextFrame:
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #29 |
Wait. When I paste that huge textarea, I see something like 3 textruns being constructed... (as measured by a breakpoint in gfxTextRun:
If I hit enter a few lines from the end I get two textruns constructed per line between me and the end. roc, is that expected?
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #30 |
(In reply to comment #27)
> Er... some of the newlines in the previous comment disappeared :(
Hmm, dbaron also told me yesterday that he's seen this as well. Do you happen to have STRs?
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #31 |
(In reply to comment #26)
> So it looks like we clear all the textruns for the textarea from the
> CharacterDataCh
> reflowing each line we clear its textrun, which is non-null at that point. So
> presumably we're constructing all those textruns twice... That still doesn't
> quite explain the behavior I see, where the difference is a lot more than 2x
> between paste and hitting enter.
Why is the textrun non-null when reflowing? We've already cleared them all, right?
Also, from what I read in this comment, it seems to me that we don't need to clear the textruns at all in CharacterDataCh
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #32 |
(In reply to comment #28)
> nsTextFrame:
> doing some of that textrun clearing on next-in-flows and the like.
Yes, and I think we're actually relying on that: <http://
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #33 |
(In reply to comment #29)
> Wait. When I paste that huge textarea, I see something like 3 textruns being
> constructed... (as measured by a breakpoint in gfxTextRun:
How many lines are you pasting? (We should create at least one textrun per line, right?)
> If I hit enter a few lines from the end I get two textruns constructed per line
> between me and the end. roc, is that expected?
Looks like we should be able to get away without constructing that many textruns on pressing Enter then, right? Of course, I have no idea how... :(
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #34 |
> Do you happen to have STRs?
Not offhand.... there was cutting and pasting involved, though. Past that... I'll see what I can dig up.
> Why is the textrun non-null when reflowing? We've already cleared them all,
> right?
We cleared them. Presumably something regenerated them!
> Yes, and I think we're actually relying on that:
Right. The point is we end up clearing+recreating there at least twice.
And again, that doesn't account for all of the slowdown.
> How many lines are you pasting?
I'm using the testcase linked from comment 0. So 4379 lines.
> (We should create at least one textrun per line, right?)
An excellent question!
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #35 |
And the real issue here is roc's on vacation. He might know the answer to that question, and my question from comment 29.
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #36 |
(In reply to comment #34)
> > Do you happen to have STRs?
>
> Not offhand.... there was cutting and pasting involved, though. Past that...
> I'll see what I can dig up.
Great, thanks. I did some rough testing with cutting and pasting and couldn't repro...
> > Why is the textrun non-null when reflowing? We've already cleared them all,
> > right?
>
> We cleared them. Presumably something regenerated them!
That's... sad.
> > Yes, and I think we're actually relying on that:
>
> Right. The point is we end up clearing+recreating there at least twice.
Yep.
> And again, that doesn't account for all of the slowdown.
Hmm, I think it should. See comment 22.
> > How many lines are you pasting?
>
> I'm using the testcase linked from comment 0. So 4379 lines.
>
> > (We should create at least one textrun per line, right?)
>
> An excellent question!
Well, that basically contradicts all that I know about textruns (which isn't that much, but still!)
(In reply to comment #35)
> And the real issue here is roc's on vacation. He might know the answer to that
> question, and my question from comment 29.
Yeah. Unless someone has a better idea, let's wait for roc to come back.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #37 |
> Hmm, I think it should. See comment 22.
Huh. That's... odd. Really odd, since we're clearing the textruns as we reflow too, right?
> Well, that basically contradicts all that I know about textruns
Note that it's _possible_ I was getting hits on the textrun cache. Again, I was just looking at the constructor invocations.
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #38 |
(In reply to comment #37)
> > Hmm, I think it should. See comment 22.
>
> Huh. That's... odd. Really odd, since we're clearing the textruns as we
> reflow too, right?
Yes. I observed things getting a lot faster when I took out that loop, but I can't really explain it.
> > Well, that basically contradicts all that I know about textruns
>
> Note that it's _possible_ I was getting hits on the textrun cache. Again, I
> was just looking at the constructor invocations.
Hmm, IINM we remove textruns from the cache when we're clearing them (why are we doing that?!).
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #39 |
Excellent question. If you leave the clearing code but take out the cache-removal bit, are things fast?
In Mozilla Bugzilla #597627, Roc-ocallahan (roc-ocallahan) wrote : | #40 |
Are you talking about ClearTextRun?
if (!(textRun-
// Remove it now because it's not doing anything useful
gTextRuns-
delete textRun;
So we only remove it from gTextRuns (an nsExpirationTra
(In reply to comment #29)
> If I hit enter a few lines from the end I get two textruns constructed per line
> between me and the end. roc, is that expected?
No, that sounds like a definite problem. What should happen is that in CharacterDataCh
In Mozilla Bugzilla #597627, Carlos-alen (carlos-alen) wrote : | #41 |
Sorry to butt in.
I can see this bug in Windows 7 with Fx 3.6.10. Perhaps the Platform field for this bug should be changed to "x86 all" or "all all" to better reflect the breadth of affected systems.
Cheers.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #42 |
Whatever you're seeing in 3.6.10 is unrelated to the issue this bug is about...
In Mozilla Bugzilla #597627, Carlos-alen (carlos-alen) wrote : | #43 |
Well, using the STR in comment 0 I see exactly what is described there: utter slowness, big lag. If you say it's unrelated, so be it.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #44 |
Well, see "This is a recent regression so I'm guessing it's a regression from bug 240933." in comment 0.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #45 |
OK, I tried another experiment. First, breakpoint in gfxTextRun:
1
2
3
4
5
6
7
8
That leads to two calls to the ctor, with lengths 16 and 15 respectively (both under ReflowText).
Then I focused the textarea, put the caret before the "1" and hit enter. This led to 27 calls to the constructor, with the following lengths: 3, 2, 2, 2, 2, 2, 2, 2, 1, 4, 2, 4, 2, 2, 4, 2, 2, 4, 2, 2, 4, 2, 2, 4, 2, 2, 4.
While some of these (near the beginning) were from ReflowText, the stuff starting with "1" seems to be under nsTextFrame:
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #46 |
Created attachment 481884
Log showing this behavior
That's a log of the paste and single enter press I described, generated by using these commands at the breakpoint:
p aLength
x/4c aText
cont
In Mozilla Bugzilla #597627, Roc-ocallahan (roc-ocallahan) wrote : | #47 |
Let me take this.
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #48 |
OK, I still don't know why we create different textruns after pasting and pressing Enter, but what bz saw in comment 45 is not surprising at all.
ClearTextRun sets mTextRun on the frames to null. And we invalidate all of the lines following the entered newline, which means that the textframes inside them are all reflown, at which time a new textrun is created because, well, mTextRun is null!
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #49 |
(In reply to comment #47)
> Let me take this.
OK, thanks! I was kind of lost here. :-)
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #50 |
> at which time a new textrun is created
That would be fine... But we have 8 textframes, and create _27_ textruns. Even ignoring the difference vs paste, that means we're creating and destroying as we go; that's the problem.
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #51 |
Ah, there's something going on here which is sort of my fault!
See this code: <http://
IINM, if there is a single numeric character in the textarea, it causes us to not cache the textrun when first the text, for example... My tests show that adding a digit to a textarea makes things noticeably slower in it :(
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #52 |
That's only if we're transforming digits, no? I dunno whether we are in your case, but we aren't in mine...
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #53 |
(In reply to comment #52)
> That's only if we're transforming digits, no? I dunno whether we are in your
> case, but we aren't in mine...
Right... It's possible that the transformation was actually happening in my case, since I was playing with the bidi.numeral pref value...
Jamie Strandboge (jdstrand) wrote : pressing Enter in a large textarea is very CPU intensive | #54 |
Binary package hint: firefox
On up to date natty with Firefox 4.0, pressing Enter in a large textarea is very CPU intensive
Go to:
https:/
Edit the page, and then click 'Enter' a few times. The CPU will spike. Holding 'Enter' pegs the CPU entirely (there is no need to commit your changes). This is a regression over 3.6 on Ubuntu 10.10.
description: | updated |
description: | updated |
summary: |
- pressing Enter in a large textarea is very CPU intensive + pressing Enter in a large textarea is very CPU intensive with Firefox + 4.0 |
description: | updated |
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #55 |
Here's what I've found. In the insertion case we adjust the content offset
so that the remainder of the text run becomes part of the next one...
then in Reflow we have "1\n2\n" and we do a SetLength after the first \n --
now SetLength adjusts the content offset again so that the next one becomes
"2\n3\n" and so forth. This ripples through all the lines and when SetLength
does this it calls ClearTextRun() on both the current frame and the
next-in-flow...
Deleting a newline looks like a different problem.
Boris, did you investigate this case?
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #56 |
Created attachment 497875
WIP
This fixes the insertion case, but not the delete case.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #57 |
> Boris, did you investigate this case?
No, I had just looked at insertions....
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #58 |
Mats, are you still working on this? This was marked as a softblocker, but I think shipping without this fixed is really bad. In a sense, it's a regression compared to 3.6: textarea editing is now much slower on medium sized textareas...
In Mozilla Bugzilla #597627, Olli-pettay (olli-pettay) wrote : | #59 |
Why is this a softblocker o_O
In Mozilla Bugzilla #597627, Justin-lebar+bug (justin-lebar+bug) wrote : | #60 |
FWIW it's very easy to encounter this bug when replying to messages in Gmail, since it auto-quotes the whole conversation.
In Mozilla Bugzilla #597627, RobertSayre (sayrer) wrote : | #61 |
yeah, we may want to take another look at this one as a hardblocker. dbaron/roc, what was your rationale?
In Mozilla Bugzilla #597627, L. David Baron (dbaron) wrote : | #62 |
How big is "big" and how slow is "slow"?
I'd note there are no duplicates or comments indicating anybody other than platform developers are seeing the bug; that made it seem like an edge case.
In Mozilla Bugzilla #597627, Justin-lebar+bug (justin-lebar+bug) wrote : | #63 |
(In reply to comment #61)
> How big is "big" and how slow is "slow"?
Try Jonas's testcase in comment 0. It's just 4000 lines -- not impossibly large for an e-mail thread -- and inserting a newline takes about 5s on my machine.
In Mozilla Bugzilla #597627, Shaver (shaver) wrote : | #64 |
http://
there's a thread in d-a-f and dev-planning where a user reported both slow URL-bar typing and slow text-entry typing (likely unrelated to each other)
http://
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #65 |
For a 140kb textarea (the testcase in comment 0), inserting or deleting a line takes about a second or two for me, on a laptop that was brand-new-and-fast in July.
The behavior is quadratic, so doing it on something like http://
The really long wikipedia articles seem to be locked down so I can't edit them. ;)
As for why no one has reported this... my best guess is that people generally assume that our editing performance is sucky and so think this is normal. :(
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #66 |
Mike, if the text is twitter-sized this bug is really not an issue. It starts to be a problem once you have tens of kilobytes or more of text.
In Mozilla Bugzilla #597627, Tnikkel (tnikkel) wrote : | #67 |
He's probably referring to bug 579488, typing in the tweet box being slow. It most definitely is slow, but for a completely different reason than this bug.
In Mozilla Bugzilla #597627, Shaver (shaver) wrote : | #68 |
People are reporting it also for facebook comment entry, and entering a comment in for input.mozilla.org, but if there's another bug for "slow on small text fields too" then I'm happy to take it there.
(I do notice that input.m.o also has a character-counter, hmm.)
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #69 |
Slowness of typing into the location bar isn't this bug (it could be places).
Also note that in order to hit this bug, you have to have many lines in the textarea, and also edit something near the beginning which causes new lines to be added. In such a case, trunk performs noticeably worse than 3.6. bz's suspicion in comment 64 is also correct, IMO (the general suckiness of our editing performance), but this could cause Firefox 4 for many wikipedia editors. One other set of users which may be affected by this are those running intranet web based apps. Many of the old textarea editing perf bugs we have is coming from those people, and I suspect that those people might not be running our beta's for other compatibility concerns.
In Mozilla Bugzilla #597627, Tnikkel (tnikkel) wrote : | #70 |
(In reply to comment #67)
> People are reporting it also for facebook comment entry, and entering a comment
> in for input.mozilla.org, but if there's another bug for "slow on small text
> fields too" then I'm happy to take it there.
>
> (I do notice that input.m.o also has a character-counter, hmm.)
The twitter issue is somewhat specific, so if there are issues with entering text in other small text boxes then I would like to investigate each of them.
I tried a random facebook comment field, seemed fine to me.
I can't find an input box on input.m.o that has a character-counter, where do I look?
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #71 |
(In reply to comment #69)
> (In reply to comment #67)
> > People are reporting it also for facebook comment entry, and entering a comment
> > in for input.mozilla.org, but if there's another bug for "slow on small text
> > fields too" then I'm happy to take it there.
> >
> > (I do notice that input.m.o also has a character-counter, hmm.)
>
> The twitter issue is somewhat specific, so if there are issues with entering
> text in other small text boxes then I would like to investigate each of them.
>
> I tried a random facebook comment field, seemed fine to me.
Those are textareas, so they're affected by this bug.
> I can't find an input box on input.m.o that has a character-counter, where do I
> look?
Why is a character counter important?
In Mozilla Bugzilla #597627, Shaver (shaver) wrote : | #72 |
Because the twitter issue is caused by reflow caused by the change in width of the character counter as people type in it, IIRC.
In Mozilla Bugzilla #597627, Tnikkel (tnikkel) wrote : | #73 |
(In reply to comment #70)
> (In reply to comment #69)
> > (In reply to comment #67)
> > > People are reporting it also for facebook comment entry, and entering a comment
> > > in for input.mozilla.org, but if there's another bug for "slow on small text
> > > fields too" then I'm happy to take it there.
> > >
> > > (I do notice that input.m.o also has a character-counter, hmm.)
> >
> > The twitter issue is somewhat specific, so if there are issues with entering
> > text in other small text boxes then I would like to investigate each of them.
> >
> > I tried a random facebook comment field, seemed fine to me.
>
> Those are textareas, so they're affected by this bug.
But people don't typically enter comments long enough in them to see the effect of this bug, do they?
In Mozilla Bugzilla #597627, RobertSayre (sayrer) wrote : | #74 |
(In reply to comment #60)
> yeah, we may want to take another look at this one as a hardblocker.
> dbaron/roc, what was your rationale?
I still don't see a rationale.
(In reply to comment #64)
>
> The behavior is quadratic...
I think users would find that order of growth to be a reasonable proxy for "Firefox sucks". Anyone disagree?
In Mozilla Bugzilla #597627, Tnikkel (tnikkel) wrote : | #75 |
The root problem with http://
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #76 |
(In reply to comment #72)
> (In reply to comment #70)
> > (In reply to comment #69)
> > > (In reply to comment #67)
> > > > People are reporting it also for facebook comment entry, and entering a comment
> > > > in for input.mozilla.org, but if there's another bug for "slow on small text
> > > > fields too" then I'm happy to take it there.
> > > >
> > > > (I do notice that input.m.o also has a character-counter, hmm.)
> > >
> > > The twitter issue is somewhat specific, so if there are issues with entering
> > > text in other small text boxes then I would like to investigate each of them.
> > >
> > > I tried a random facebook comment field, seemed fine to me.
> >
> > Those are textareas, so they're affected by this bug.
>
> But people don't typically enter comments long enough in them to see the effect
> of this bug, do they?
Maybe... Anyway we have legit use cases for textareas which lets them grow enough for this bug to show (web mail apps, for example).
(In reply to comment #73)
> I think users would find that order of growth to be a reasonable proxy for
> "Firefox sucks". Anyone disagree?
I second that.
In Mozilla Bugzilla #597627, Olli-pettay (olli-pettay) wrote : | #77 |
Reasking, Why is this a softblocker?
In Mozilla Bugzilla #597627, pd (an0n1m0us) wrote : | #78 |
Not sure if this is related but the textarea on the web page that is displayed when clicking the Minefield "Feedback" button (Firefox Input?) is also very slow to type characters from the first to last characters regardless of line feeds.
I guess it is possible that the 'characters remaining' code is slowing this down however I've seen similar code run substantially faster (though never at the same speed as normal typing, *sigh*).
Hope this feedback is not just noise.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #79 |
pd, that sounds like a separate issue. The "characters remaining" thing is very likely part of the problem in your case, but the details could depend on what styles are used, etc. Can you please file a bug on your problem with detailed steps to reproduce, as well as details about your OS and the graphics information from about:support, and cc me on that bug?
In Mozilla Bugzilla #597627, Mounir-lamouri (mounir-lamouri) wrote : | #80 |
A user was complaining that he/she had to wait 10 minutes when entering a new line in a textarea sometimes. How is that a softblocker? I mean, according to Johnath blog post [1] a softblocker is a "visual polish, strange edge cases, optional aspects of new specs, or opportunistic performance wins". This bug is none of them. However, it matches the description of a hardblocker which can be a "performance hit".
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #81 |
Yeah, I think we need to retriage this.
Mats, did you ever get anywhere on the deletion case?
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #82 |
Not really, I'm still at the debugging stage with the deletion case.
Any help with that would be appreciated. I have a couple of hardblockers
left but they should be done in a day or two I hope...
In Mozilla Bugzilla #597627, Khuey (khuey) wrote : | #83 |
Created attachment 507129
Possible patch
I don't purport to fully understand what this code actually does, but following the same idea for the growing case as the shrinking case makes deletion fast for me.
In Mozilla Bugzilla #597627, Jst (jst) wrote : | #84 |
Agreed, this sounds more like a hardblocker to me as well, given the recent comments. Roc, if you or someone else disagrees, feel free to mark otherwise.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #85 |
No, that patch is definitely wrong, since it'll mess with frames whose content offset is too big (and incidentally doesn't make deletion fast for me).
In Mozilla Bugzilla #597627, Khuey (khuey) wrote : | #86 |
Comment on attachment 507129
Possible patch
Yeah, it blew up in crashtests on try too.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #87 |
So what I think happens in the deletion case I'm testing is that we delete the newline char, end up with an empty text frame, reflow, and effectively move each bit of text to the previous textframe... which means rebuilding text runs for all of them.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #88 |
Created attachment 507163
This might work, on the other hand
This seems to fix the deleting case for me too. Mats, what do you think?
In Mozilla Bugzilla #597627, Ehsan-mozilla (ehsan-mozilla) wrote : | #89 |
Comment on attachment 507163
This might work, on the other hand
This works nicely for me. Is there anything else remaining to be done here? Or could we enter the review stage?
(Note: s/Whateve/
In Mozilla Bugzilla #597627, Roc-ocallahan (roc-ocallahan) wrote : | #90 |
Comment on attachment 507163
This might work, on the other hand
This is scary but it *should* work.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #91 |
Mats, you want me to land this? Or do you want to?
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #92 |
There was an assertion on TryServer in one of the reftests when I tested my
patch, in nsFirstLetterFrame IIRC. Feel free to land it if that doesn't occur
anymore. (sorry, I should have noted that on the bug at the time)
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #93 |
Ah.... I have no idea. Pushing this to try right now, to see.
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #94 |
When I run reftests locally on Linux64 I get:
###!!! ASSERTION: null frame is not allowed: 'aFrame',
file layout/
followed by a crash. The call comes from nsFirstLetterFr
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #95 |
Removing the last hunk fixes that. I'm re-running the tests again to see
if I can catch the assertion I saw with my part...
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #96 |
Yeah, from the first part we get:
###!!! ASSERTION: bad overflow list: 'mFrames.
file layout/
when loading layout/
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #97 |
OK. For the second hunk, we should probably only do the removal if we're not changing parent in the process. That should fix the comment 93 assert+crash.
We can probably fix the comment 95 assert by checking whether the parent is a letter frame... but could other inline frames have similar issues, or just the weirdness that is letterframe?
In Mozilla Bugzilla #597627, Roc-ocallahan (roc-ocallahan) wrote : | #98 |
Do you know what the actual issue is with the letterframe?
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #99 |
Not yet.
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #100 |
Created attachment 507510
Patch v2
This is with Boris' suggested fixes. It still asserts though:
###!!! ASSERTION: overflow list w/o frames: 'mFrames.
file layout/
when loading layout/
I'm debugging these assertions to see what's going on...
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #101 |
Created attachment 507580
Framedump (for the asserion in comment 99)
The last assertion comes from removing the child of a first-letter frame
(0x7fffde659138 blue), on the next reflow we call DrainOverflowFrames
which asserts that the principal list is non-empty when there is an
non-empty overflow list.
I think this is just my fault of not implementing Boris' suggested fix
quite right...
+ next->GetParent() == f->GetParent()) {
should have been
+ GetParent() == f->GetParent()) {
which would avoid doing the RemoveFrame optimization in this case.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #102 |
No, I'd in fact been thinking of comparing the parents of next and f.
If we remove |f|, which is the child of a first-letter frame, but next->GetParent() == f->GetParent(), then |next| is a child of that first-letter too. So how does the principal child list get empty? Do we end up pushing |next| for some reason?
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #103 |
Created attachment 507585
Patch v3
This is the fix Boris' suggested I think.
I'll submit it to the TryServer and ask for review if it pass unit tests...
Meanwhile, I'll debug the assertions in comment 93/95.
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #104 |
You're fast ;-)
The principal list becomes empty because 'next' is on the overflow list.
I guess we could just remove that assertion; I mean the important
thing is that the principal list is non-empty after draining overflow.
Or we could just avoid doing the second optimization at all when a
first-letter frame is onvolved, just to be on the safe side...
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #105 |
Oh, |next| is already on the overflow list but |f| is not while we're reflowing |f|?
I think a simple solution would be to only do this optimization when f->GetNextSibling() == next. That ensures that they're in the same child list, etc.
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #106 |
Yes. Ok, that sounds good.
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #107 |
(Well, to be precise, "while we're reflowing |f|" isn't correct - we're
reflowing its prev-in-flow (0x7fffde6532e0) which has a different
first-letter parent.)
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #108 |
Created attachment 507599
Patch v4
This is with the "f->GetNextSibl
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #109 |
> we're reflowing its prev-in-flow
Aha, I see. OK, that makes perfect sense!
We should document why the GetNextSibling() check is there.
For the other, do we know why there's a problem with first-letter? Is it really first-letter specific?
Changed in firefox (Ubuntu): | |
importance: | Undecided → Medium |
assignee: | nobody → Chris Coulson (chrisccoulson) |
Changed in firefox (Ubuntu): | |
status: | New → Confirmed |
affects: | firefox (Ubuntu) → ubuntu |
affects: | Ubuntu Natty → firefox (Ubuntu Natty) |
Changed in firefox (Ubuntu Natty): | |
importance: | Medium → High |
status: | Confirmed → Triaged |
Changed in firefox: | |
importance: | Unknown → Medium |
status: | Unknown → Confirmed |
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #110 |
Created attachment 507697
Framedump (for the assertion in comment 93)
The cause of the assertion/crash in comment 93 is the RemoveFrame we added
causes a lot of first-letter frames to become empty. Then we reflow one
of those and it can't handle that. (See bug 578977.)
The GetNextSibling() check we added should prevent this.
The assertion in comment 95 looks like a bogus assert. I think it's a typo
that was really meant to be "overflowFrames
http://
I propose we simply remove it since StealOverflowFr
http://
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #111 |
Created attachment 507698
Patch v5
Add comment about GetNextSibling() check. Remove bogus assert.
(v4 looks good on Try so far...)
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #112 |
I meant "!overflowFrame
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #113 |
After thinking some more about that assertion; it's probably not a typo
but intended to catch an empty first-letter frame that should have been
removed (as in bug 578977). So let's keep it.
In Mozilla Bugzilla #597627, Bzbarsky (bzbarsky) wrote : | #114 |
Comment on attachment 507698
Patch v5
This looks fine if we document that parent frame type check. Have roc look at this too?
In Mozilla Bugzilla #597627, Roc-ocallahan (roc-ocallahan) wrote : | #115 |
Comment on attachment 507698
Patch v5
Assuming you leave in the assert in nsFirstLetterFrame.
In Mozilla Bugzilla #597627, Mats Palmgren (matspal) wrote : | #116 |
With additional comment and assert in nsFirstLetterFrame intact.
http://
Changed in firefox: | |
status: | Confirmed → Fix Released |
In Mozilla Bugzilla #597627, Aleksej P. (chaos8) wrote : | #117 |
Verified with Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre
Jamie Strandboge (jdstrand) wrote : | #118 |
This was fixed in 4.0~b11+
Changed in firefox (Ubuntu Natty): | |
status: | Triaged → Fix Released |
Profile (on Mac) says 95% of the time is spent in the reflow that we trigger from EndUpdateViewBatch. Even more interestingly, 93% is under nsLineLayout: :TrimTrailingWh iteSpace and almost all of this is under EnsureTextRun. Sounds like we're recreating textruns for the whole thing, presumably because inserting some text marked them all dirty?
Oh, and we're spending most of our time here on line-breaking in the textrun code.
Are we trying to trim trailing whitespace all down the line, or are we just creating huge textrun? I thought we limited textrun length or something... and we shouldn't have to trim trailing whitespace all down the line (and in fact, should be able to stop the reflow at the next line that ends in a newline char, yes?)