Minimize performance regression caused by r157508

Minimize performance regression caused by r157508

Minimize performance regression caused by r157508:
- Cache the value returned by Element::attributeItem() as it hides
  an if branch in ElementData::attributeBase().
- Minimize the cost of calling attrIfExists() which does a
  linear search by using iterators (thus avoiding the release
  assertion in Vector::at())

I get a 3.1% improvement locally on Bindings/id-setter.html 
performance test.

R=haraken
BUG=293663
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161383

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161419

[arv (Not doing code reviews), 2013-11-05 15:44:29]

LGTM

[haraken, 2013-11-05 15:45:33]

LGTM. Did you get any performance number from Bindings/id_setter.html?

[Inactive, 2013-11-05 15:48:44]

On 2013/11/05 15:45:33, haraken wrote:
> LGTM. Did you get any performance number from Bindings/id_setter.html?

No, but I am happy to. I actually don't know how to run this test locally but I
will check.

[haraken, 2013-11-05 15:49:52]

On 2013/11/05 15:48:44, Chris Dumez wrote:
> On 2013/11/05 15:45:33, haraken wrote:
> > LGTM. Did you get any performance number from Bindings/id_setter.html?
> 
> No, but I am happy to. I actually don't know how to run this test locally but
I
> will check.

Most simply, you can just open the file in chrome.

[Inactive, 2013-11-05 16:15:09]

On 2013/11/05 15:49:52, haraken wrote:
> On 2013/11/05 15:48:44, Chris Dumez wrote:
> > On 2013/11/05 15:45:33, haraken wrote:
> > > LGTM. Did you get any performance number from Bindings/id_setter.html?
> > 
> > No, but I am happy to. I actually don't know how to run this test locally
but
> I
> > will check.
> 
> Most simply, you can just open the file in chrome.

Hmm, that easy huh? :)

Ok so here are the results:
- Without my patch: average is 189.39413548689342 runs/s
- With my patch: average is 205.99773207299427 runs/s

So that's a 8.77% improvement.

[commit-bot: I haz the power, 2013-11-05 16:15:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/60143002/1

[commit-bot: I haz the power, 2013-11-05 21:10:14]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[commit-bot: I haz the power, 2013-11-05 21:38:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/60143002/1

[commit-bot: I haz the power, 2013-11-05 21:40:18]

Change committed as 161383

[Inactive, 2013-11-06 02:58:07]

The patch was reverted because avoiding the attribute name copy was wrong and
the reference could sometimes be invalidated.
I reworked the patch a little to minimize the performance regression.

I see a 3.1% improvement with this new patch. Please take another look.

If case you're curious, caching the attribute improves the test by ~1.6%, the
iterators are responsible for the rest.

[eseidel, 2013-11-06 03:12:23]

lgtm

https://codereview.chromium.org/60143002/diff/180001/Source/core/dom/Element.cpp
File Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/60143002/diff/180001/Source/core/dom/Element....
Source/core/dom/Element.cpp:185: if ((*it)->qualifiedName() == name)
I wonder if this really wants matches() :)  Which doesn't respect prefix. 
Depends on what the callers are looking for.  This is a very common issue which
only is visibile in XML documents (where prefixes exist).  Eventually perhaps
we'll remove main-document-XML from the web and all these bugs will go away.

[commit-bot: I haz the power, 2013-11-06 03:43:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/60143002/180001

[commit-bot: I haz the power, 2013-11-06 05:50:45]

Change committed as 161419