This bug is for implementing the performance optimization suggested by BZ in bug 330373 comment #24: > It seems to me that we should be doing the advancing at the places where we > call MarkDirty(); perhaps with a function that takes the frame, > nsBlockFrame::line_iterator& line, const nsBlockFrame::line_iterator& endLines? > That would also address the second concern I had, which is that the code as it > stands always iterates the lines, even if it doesn't actually change any > continuations. The performance problem is significant in lines with very many frames on them. I noticed this when trying to do "view selected source" after selecting the talkback section in http://www.ynet.co.il/articles/0,7340,L-3256726,00.html. The source contains about 1MB of complex HTML on a single line, which takes forever to render (effectively, the browser hangs). Using the "sample" feature in OS X's Activity Monitor, I can see that much of the time is spent in nsLineBox::IndexOf, called (via nsLineBox::Contains), from nsBidiPresUtils::Resolve.

Comment on attachment 223803 [details] [diff] [review] patch What about the concern you had in bug 330373 comment 24? r+sr=bzbarsky if you've resolved that to your satisfaction.

Hmmm... thanks for reminding me of that. It turns out that the newly-created frames do "know" what line they are on (the frame count on the line is incremented when creating a new continuation), so there is no functional problem. However, this change does cause nsLineBox::Contains to be called in cases where it currently isn't called (that is, for newly-created continuations), so I assume that for some cases (where there is originally a small number of frames, but some of them are broken into many bidi continuations), this will actually cause a performance hit. As the case described above is possibly more common than the one described in comment #0, I'm now hesitating whether this is worth doing.

This should give us the best of both worlds.

Comment on attachment 223815 [details] [diff] [review] patch v2 Yeah, I like this!

Checking in layout/base/nsBidiPresUtils.cpp; /cvsroot/mozilla/layout/base/nsBidiPresUtils.cpp,v <-- nsBidiPresUtils.cpp new revision: 1.77; previous revision: 1.76 done