Going to a page with only one link was calling nsWebShell::GetLinkState for the same link 11 times. GetLinkState is fairly expensive as it has to hit the DB etc. Should we really have to resolve this so many times?

any idea how costly this is? if this is expensive, it will really hurt our performance on pages like my.netscape.com, home.netscape.com and www.yahoo.com, which are just links.

I changed GetLinkState to basically do nothing and I got an improvement of .2 seconds consistently on the reload of a page. So the improvement should be somewhere in between there as it would have to do something at least once.

Assigning to Waqar: 1/3 of Pierre's NEW bugs to help reduce his doomage factor

Reassigned back to me these bugs that shouldn't have left my list.

Taking performance related style bugs that are still NEW

This is a performance problem. I'll look into it after beta2

Well, I couldn't wait until after beta to look... GetLinkState is called for every style rule that has a :link, :visited, or :out-of-date pseudoclass. I removed all but the basic ones from html.css and we get down to three calls (one for each of the three link-pseudos, using a testpage with one link). html.css has loads of these pseudos in it, it looks like they are for images as links, and for focus on links. I think that we have to cache the linkstate if we want to reduce the cost of resolving these pseudostyles because the style system is just following the rules that it is handed. We can cache it in the LinkStateManager (WebShell), or better yet the GlobalHistory could make the lookup faster so the cost of asking for a link's state would not be so high. To cache this in the Style System seems like a big mistake, since the style system would then need to be notified when the state of a link changed, and that is what the LinkStateManger is for in the first place. Maybe we could cache the state of the link within the duration of one enumeration through the style rules, since the link state cannot change during that time... I'll see if that is at all feasible. I'm changing the summary since it is wrong. The real problem is that the cost of calling GetLinkState is high. Style is calling it for each rule that relies on the state of a link.

warren suggested that it might make sense to just skip *all* link lookup during initial layout, and then do some kind of second pass to color the links pierre said that this might be possible by schduling a separate style reflow. Does this seem reasonable? In separate tests that I've run, removing *all* link lookup (e.g., by just commenting out the line) speeds up pages by as much as 3x! Granted, those are degenerate pages made up of all anchor tags...

If a page has a lot of links, wouldn't the user want to see the links as the page loads? *Not* applying the link styles would make the page initially look like it was all text, then later, after the page has loaded, the links would be styled. This would probably confuse people to no end (it would confuse me). Then again, we could assume that all links were in the :link state and style them initially like that. Then, during a style reflow, we could apply the actual link-state (:linl, :visited and :out-of-date) - that would probably look reasonable I think...

Yes, that's what I meant. ;-)

I tried the two-pass link-styling and it works, but I think it is of limited value since we already incrementally display content while loading. By doing a second-pass link styling we are really just moving the time from document-load to just after document load, and the user probably won't notice. I'd prefer to just make the link-state determination faster for whenever we do it... Another idea is to get the href in canonical form once, the first time it is needed, and then cache it for subsequent use. We could even cache it as a char* so that we don't need to do any nasty conversion before passing it to the link handler... (This could happen on first access or in the content-sink when the attribute is set) If the canonical form of the href doesn't have to be computed, and we don't have to malloc memory or convert data, the all we really have left is the lookup, which we can *maybe* optimize if necessary. Should be pretty straight-forward to implement too! Joy :)

I added a dependency on 10373 even though it is not really dependent - I just wanted to track these two together since the proposed solutions to 10373 may help this bug as well.

Ok, so the patch I just attached uses nsIDOMHTMLAnchorElement::GetHref() to get the canonical (yay!) URL of the link. I'm going tinker with nsHTMLAnchorElement::GetHref() so that it will cache the value after computing it once. (Trying to compute the value when it's set ends up being a bit tricky because of all the delegation that happens with HTML content elements.) It may make sense to make a s3kret nsIAnchorContent (a la nsIContent) interface that has a GetHref(const char**) method on it: that would be the most efficient way to wade through this mess (avoids inflating/deflating the URL repeatedly).

Ok, I've verified that this actually improves life: speeds up style resolution on the "40 URLs" by anywhere from 5 to 20%. I'll check it in tomorrow.

Chris, you de man!

fix checked in.

travis@netscape.com: not sure how to verify this bug is fixed. Can you please review and if you agree with fix, mark it verified? Thanks

Netscape's standard compliance QA team reorganised itself once again, so taking remaining non-tables style bugs. Sorry about the spam. I tried to get this done directly at the database level, but apparently that is "not easy because of the shadow db", "plus it screws up the audit trail", so no can do...

code level fix with no response from reporter; rubber-stamp-verifying