Bug 77879 – Move style related functions from NamedNodeMap to ElementAttributeData

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Caio Marcelo de Oliveira Filho]

Move style related functions from NamedNodeMap to ElementAttributeData

[Caio Marcelo de Oliveira Filho]

Created attachment 125661 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 125661 [details]
Patch

Excellent!

[Caio Marcelo de Oliveira Filho]

Committed r106819: <http://trac.webkit.org/changeset/106819>

[Tim Horton]

This change seems to significantly (~28% or so) regress performance of the ManInBlue HTML (http://themaninblue.com/experiment/AnimationBenchmark/html/) benchmark, among others. Looking at the change, though, I'm not sure how that's possible. Could you potentially take a look at this?

[Ryosuke Niwa]

(In reply to comment #4)
> This change seems to significantly (~28% or so) regress performance of the ManInBlue HTML (http://themaninblue.com/experiment/AnimationBenchmark/html/) benchmark, among others. Looking at the change, though, I'm not sure how that's possible. Could you potentially take a look at this?

That's surprising. The only reason I can think of this is happening if some functions in NamedNodeMap were benefiting from ensureInlineStyleDecl, destroyInlineStyleDecl, and ensureAttributeStyle being inlined. Could you try that and see what happens?

[Ryosuke Niwa]

The regression is probably caused by additional tertiary operators in inlineStyleDecl() and friends. We should probably do this refactoring when we move m_attributeData from NamedNodeMap to Element.

[Ryosuke Niwa]

king is right. This is caused by ensureInlineStyleDecl now calling ensureAttributeData, which updates attributes, instead of ensureInlineStyleDecl, which doesn't update attributes:

As a result, we're spending 2+% of time in StyledElement::style().

[Ryosuke Niwa]

It also caused a regression on DOM/Accessors:
http://webkit-perf.appspot.com/graph.html?#tests=[[3113,2001,32196]]&sel=1328551109987.2344,1328555918820.6528&displayrange=7&datatype=running

[Tony Gentilcore]

This appears to have regressed several of chromium's page cyclers and is a blocker for us.

Would you be opposed to rolling out this patch and working on the fix off of the tree?

[Tony Gentilcore]

Chromium bug: http://code.google.com/p/chromium/issues/detail?id=112854#c21

[Tony Gentilcore]

Actually, looks like rniwa has a proposed fix here: https://bugs.webkit.org/show_bug.cgi?id=77952