Bug 1927114 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

In `nsBlockFrame::AddFrames`, we use `nsLineBox::RIndexOf` while trying to find the line containing the previous sibling frame, so that we search the given `aPrevSiblingLine` (if any) from the end. This was done in bug 1568501 to fix a performance cliff, where the frame we're looking for tends to be at the end of the (possibly very long) `aPrevSiblingLine`.

However, the profile https://share.firefox.dev/3NDcwnl from bug 1926808 comment 3 suggests that this is not always the right thing: almost 50% of the samples in this 11-minute profile are within this `nsLineBox::RIndexOf` call. This implies, I think, that in this example we're having to search all the way *backwards* through an extremely long line, and searching from the beginning would have been the better choice.

In the specific case of bug 1926808, the restructuring of View Source in bug 1926824 should avoid the problem, but there must be other pages that could potentially hit the same issue.

I wonder, therefore, if the best strategy when `nsBlock::AddFrames` is trying to find the previous sibling's line would be to search `aPrevSiblingLine` from *both* ends, so that we find the target frame quickly if it is either at the beginning or the end. In practice these are the two likely cases; in theory, given the right mix of bidi content, I think the previous sibling *could* be somewhere in the middle of the line, but that will be much less common.

I'm pushing a try run to see whether modifying `nsLineBox::RIndexOf` in this way affects the bidi-resolution perf test that bug 1568501 was fixing:
> Base: https://treeherder.mozilla.org/jobs?repo=try&revision=a518caa0e33ea172ce8ccf8564b3569b2c5a83f5
> Patched: https://treeherder.mozilla.org/jobs?repo=try&revision=27a0c8f50b94419aab7ad2d466b4a7ae6640840e
In `nsBlockFrame::InsertFrames`, we use `nsLineBox::RIndexOf` while trying to find the line containing the previous sibling frame, so that we search the given `aPrevSiblingLine` (if any) from the end. This was done in bug 1568501 to fix a performance cliff, where the frame we're looking for tends to be at the end of the (possibly very long) `aPrevSiblingLine`.

However, the profile https://share.firefox.dev/3NDcwnl from bug 1926808 comment 3 suggests that this is not always the right thing: almost 50% of the samples in this 11-minute profile are within this `nsLineBox::RIndexOf` call. This implies, I think, that in this example we're having to search all the way *backwards* through an extremely long line, and searching from the beginning would have been the better choice.

In the specific case of bug 1926808, the restructuring of View Source in bug 1926824 should avoid the problem, but there must be other pages that could potentially hit the same issue.

I wonder, therefore, if the best strategy when `nsBlock::InsertFrames` is trying to find the previous sibling's line would be to search `aPrevSiblingLine` from *both* ends, so that we find the target frame quickly if it is either at the beginning or the end. In practice these are the two likely cases; in theory, given the right mix of bidi content, I think the previous sibling *could* be somewhere in the middle of the line, but that will be much less common.

I'm pushing a try run to see whether modifying `nsLineBox::RIndexOf` in this way affects the bidi-resolution perf test that bug 1568501 was fixing:
> Base: https://treeherder.mozilla.org/jobs?repo=try&revision=a518caa0e33ea172ce8ccf8564b3569b2c5a83f5
> Patched: https://treeherder.mozilla.org/jobs?repo=try&revision=27a0c8f50b94419aab7ad2d466b4a7ae6640840e

Back to Bug 1927114 Comment 0