348711 - Performance regression on Ynet articles with many talkbacks due to excessive calls into nsBidiPresUtils::InitContinuationStates

Status: RESOLVED FIXED

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

Description

[Uri Bernstein]

I noticed that articles on ynet.co.il which have hundreds of talkbacks on them (such as the one in the URL field above) are taking a very long time to load.
Using Activity Monitor's Sampler, I found out that a very large portion of the time (often over 50%) is spent inside nested calls into nsBidiPresUtils::InitContinuationStates (the actual function where the time is spent is PL_DHashTableFinish).

It turns out that each talkback is wrapped in several tables, the outermost of which has display:inline. As a result, InitContinuationStates iterates over the entire content of these tables, reaching up to 16 levels of recursion.
This is unnecessary, since the continuation states of frames that are descendents of non-bidi-leafs (such as table frames) are never used in RepositionFrame. 

Strangely, I wasn't able to create an offline testcase that reproduces the problem. While I was easily able to reproduce the excessive recursion, I did not see a significant performance problem - possibly because PL_DHashTableFinish wasn't actually taking much time in my offline testcases (for reasons I do not understand).

I'll attach a patch for not descending into non-leafs in InitContinuationStates, which should fix the problem for Ynet. The question of why PL_DHashTableOperate/PL_DHashTableFinish is taking so much time might be worth a separate investigation.

Comment 1

[Uri Bernstein]

Attachment: patch (diff -w)

Don't init continuation states for descendants of bidi leaf frames.

Comment 2

[Uri Bernstein]

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

I'm leaving this open until I have some data on how this actually affected performance.

Comment 3

[Uri Bernstein]

This took down the time required to load the talkback section of the URL above from around 20 seconds to around 14 seconds, which is what it used to be before bug 328168, so marking this fixed.

Comment 4

[Eyal Rozenberg]

Is this really a MacOS bug? I think I see basically the same thing on WinXP.

Comment 5

[Uri Bernstein]

(In reply to comment #4)
> Is this really a MacOS bug? I think I see basically the same thing on WinXP.
> 

You're probably right (there's nothing in the fix to make it Mac-specific). When you say you're seeing basically the same thing, you mean similar numbers to what I'm seeing before and after the fix?

Comment 6

[Eyal Rozenberg]

What I'm seeing is a CPU usage which ranges from 13% to 33% (on an Athlon 1800 Mhz system with 1 GB RAM), and the fact that the pages continues to appear to be loading despite all content being visible. The status bar switches between 'waiting for' and 'transferring'. Is that it?

Comment 7

[Uri Bernstein]

(In reply to comment #6)
> What I'm seeing is a CPU usage which ranges from 13% to 33% (on an Athlon 1800
> Mhz system with 1 GB RAM), and the fact that the pages continues to appear to
> be loading despite all content being visible. The status bar switches between
> 'waiting for' and 'transferring'. Is that it?
> 

The stage to which I was referring to in this bug is between when the content of the article itself appears (with a message saying something like "please wait for the talkbacks to load"), and when the talkbacks actually appear. The statusbar reads "waiting for ynet.co.il" during this stage.
This stage takes a long time, and consumes a lot of CPU power (100% in my case). The patch reduced this time by about 30% for me, as indicated in comment #3.

If you can provide measurements on WinXP, before and after the patch, that would be great.

Comment 8

[Eyal Rozenberg]

Well, first of all, why aren't you on the channel?

Secondly, from the time I click Enter in the URL bar to the time I see the comment on this page:

http://www.ynet.co.il/articles/0,7340,L-3289731,00.html

I wait about 90 seconds. Note however that other tasks (IO-bound networking apps) are running in the backgroud. This is with build 2006-08-09