Open Bug 109181 Opened 24 years ago Updated 3 years ago

{ib}ReframeContainingBlock is called too often

Categories

(Core :: Layout: Block and Inline, defect, P4)

defect

Tracking

()

Future

People

(Reporter: attinasi, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: perf)

Especially nasty are the problems where we have a 'special' frame and then some content gets inserted into it. Code like this (nsCSSFrameConstructor): // If the frame we are manipulating is a special frame then do // something different instead of just inserting newly created // frames. if (IsFrameSpecial(parentFrame)) { // We are pretty harsh here (and definitely not optimal) -- we // wipe out the entire containing block and recreate it from // scratch. The reason is that because we know that a special // inline frame has propogated some of its children upward to be // children of the block and that those frames may need to move // around. This logic guarantees a correct answer. #ifdef DEBUG if (gNoisyContentUpdates) { printf("nsCSSFrameConstructor::ContentInserted: parentFrame="); nsFrame::ListTag(stdout, parentFrame); printf(" is special\n"); } #endif return ReframeContainingBlock(aPresContext, parentFrame); } Some day we need to fix this, because so many sites amke this happen and it is really really slow. I intend this to be a META bug, so feel free to dup specific cases of this problem to this bug.
Accepting. I looked for another bug on this but could not find one, which was surprising...
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.0.1
*** Bug 100359 has been marked as a duplicate of this bug. ***
I optimized ContentAppended a while back. Should be possible (if trickier) to do similar stuff here.
Blocks: 71668
Chris, was that the stuff I see about detecting when we are Appending (or in this case Inserting) into the anonymous block rather than the 'special' frame itself? If so, I think you are right that we can do this here (and maybe in ContentRemoved as well, though that is probably called far less often).
Yes. It's pretty easy to do in the ContentAppended case -- although now that I'm thinking about it, we may not be doing it right...urgh! IIRC, when the {ib} hierarchy is initially created, we'll: 1. Take all the inlines preceding the block and lump them under the original inline. 2. Create an anonymous block that is a ``special ib sibling'' of the original inline frame. This will parent any frames between the first and last block child of the original inline (inclusive). 3. Optionally create a trailing inline frame that is a special ib sibling of the anonymous block. This will parent any inline frames after the last block child. e.g., <span>one<p>two</p>three<p>four</p>five</span> would yield inline(span)< text<one> > block(span)< block(p)< text<two> > text<three> block(p)< text<four> > > inline(span)< text<five> > In this case, the first inline(span) is marked ``special ib'', and it's ``special ib sibling'' is the block(span). Likewise, the block(span) is marked special ib with its special ib sibling as the last inline(span). The last inline(span) is marked special ib with no special ib sibling. Stuff like FindPrimaryFrameFor knows to look through the special sibling list when trying to find the frame for content that is the child of the <span>. (For example, GPFF on the ``three'' text node would bomb if we only searched through the immediate child frames of the first inline<span> frame.) FWIW, The problem I'm seeing with the ContentAppended code is that it may yield: inline(span)< text<one> > block(span)< block(p)< text<two> > text<three> block(p)< text<four> > text<five> > I guess this really isn't a big deal; I can't imagine that it would cause any problems. Anywhoo, the trick with ContentInserted will be to figure out if you're inserting into the block (in which case, who cares: it can hold either a block or an inline), or into the leading or trailing inline frame (in which case we probably should punt and RecreateFramesForContent). That said, there's no reason that we couldn't allow the {ib} stuff to proceed; e.g., we could append another block ``special ib sibling'' and stuff ought to just work: inline(span)< ... > block(span)< ... > inline(span)< ... > block(span)< ... > Hope that helps!
Moving Mozilla 1.01 bugs to 'future' milestone with priority P1 I will be pulling bugs from 'future' milestones when scheduling later work.
Priority: P3 → P1
Target Milestone: mozilla1.0.1 → Future
Summary: ReframeContainingBlock is called too often → {ib}ReframeContainingBlock is called too often
->block&inline
Assignee: attinasi_layout → block-and-inline
Status: ASSIGNED → NEW
Component: Layout → Layout: Block & Inline
Depends on: ib_reflow
QA Contact: petersen → ian
Keywords: perf
Depends on: 501847
Assignee: layout.block-and-inline → nobody
QA Contact: ian → layout.block-and-inline
Decreasing the priority as no update for the last 2 years on this bug. See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage about the priority meaning.
Priority: P1 → P5
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.