Have ElementRuleCollector keep match result vector as part object.

Have ElementRuleCollector keep match result vector as part object.

Simplify the representation of the matched results vector, keeping
the vector as a part object. The vector ends up being updated when
ElementRuleCollector is used, so lazily creating it gets in the
way of that. Not doing so also enables the prompt release of its
backing store with Oilpan, as it is an 'inline-capacity' vector.

In terms of performance, this brings Oilpan back on level terms
with non-Oilpan on blink_perf.layout:SimpleTextPathLineLayout,
undoing a 6% regression seen previously (linux.)

R=haraken@chromium.org,rune@opera.com
BUG=340522
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187007

[sof, 2014-12-11 10:58:13]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org

[sof, 2014-12-11 10:58:13]

Please take a look.

[haraken, 2014-12-11 11:12:51]

LGTM

[sof, 2014-12-11 11:46:48]

On 2014/12/11 11:12:51, haraken wrote:
> LGTM

Thanks, looking into performance & details more closely - this might be too
aggressive non-Oilpan. But the source of the Oilpan perf regression for that
test has been identified, I believe.

[sof, 2014-12-11 16:56:32]

On 2014/12/11 11:46:48, sof wrote:
> On 2014/12/11 11:12:51, haraken wrote:
> > LGTM
> 
> Thanks, looking into performance & details more closely - this might be too
> aggressive non-Oilpan. But the source of the Oilpan perf regression for that
> test has been identified, I believe.

We can do a bit better if the ElementRuleCollector's inlineCapacity heap vector
is "inlined" instead, so as to avail ourselves to Oilpan's heap for backing
stores with inline capacity. Done now - ptal?

blink_perf.layout perf numbers wrt trunk (no Oilpan),


https://docs.google.com/spreadsheets/d/1NkNyU_oaOU7xPeBXXJFwbds-zEDmKvGwgQS5-...

[sof, 2014-12-11 17:10:27]

On 2014/12/11 16:56:32, sof wrote:
> On 2014/12/11 11:46:48, sof wrote:
> > On 2014/12/11 11:12:51, haraken wrote:
> > > LGTM
> > 
> > Thanks, looking into performance & details more closely - this might be too
> > aggressive non-Oilpan. But the source of the Oilpan perf regression for that
> > test has been identified, I believe.
> 
> We can do a bit better if the ElementRuleCollector's inlineCapacity heap
vector
> is "inlined" instead, so as to avail ourselves to Oilpan's heap for backing
> stores with inline capacity. Done now - ptal?
> 
> blink_perf.layout perf numbers wrt trunk (no Oilpan),
> 
> 
>
https://docs.google.com/spreadsheets/d/1NkNyU_oaOU7xPeBXXJFwbds-zEDmKvGwgQS5-...

For easy cross-reference, sheet 2 ("current") has the numbers for no-Oilpan vs
Oilpan for trunk.

[haraken, 2014-12-11 23:48:44]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org, rune@opera.com, tasak@google.com

[haraken, 2014-12-11 23:48:44]

rune@, tasak@: Would you take a look at this change?

[sof, 2014-12-12 07:03:08]

On 2014/12/11 23:48:44, haraken wrote:
> rune@, tasak@: Would you take a look at this change?

Sheet 3 of the above spreadsheet has the no-Oilpan results for trunk vs this CL.
No notable differences.

[rune, 2014-12-12 07:06:28]

lgtm

[rune, 2014-12-12 07:10:36]

https://codereview.chromium.org/795043002/diff/60001/Source/core/css/ElementR...
File Source/core/css/ElementRuleCollector.cpp (right):

https://codereview.chromium.org/795043002/diff/60001/Source/core/css/ElementR...
Source/core/css/ElementRuleCollector.cpp:231: WillBeHeapVector<MatchedRule, 32>&
matchedRules = m_matchedRules;
Is it really necessary to have a new reference here? Couldn't m_matchedRules be
used directly below?

[sof, 2014-12-12 07:13:23]

https://codereview.chromium.org/795043002/diff/60001/Source/core/css/ElementR...
File Source/core/css/ElementRuleCollector.cpp (right):

https://codereview.chromium.org/795043002/diff/60001/Source/core/css/ElementR...
Source/core/css/ElementRuleCollector.cpp:231: WillBeHeapVector<MatchedRule, 32>&
matchedRules = m_matchedRules;
On 2014/12/12 07:10:35, rune wrote:
> Is it really necessary to have a new reference here? Couldn't m_matchedRules
be
> used directly below?

Yes, we can simplify this away now. Will do.

[sof, 2014-12-12 08:04:50]

On 2014/12/12 07:13:23, sof wrote:
>
https://codereview.chromium.org/795043002/diff/60001/Source/core/css/ElementR...
> File Source/core/css/ElementRuleCollector.cpp (right):
> 
>
https://codereview.chromium.org/795043002/diff/60001/Source/core/css/ElementR...
> Source/core/css/ElementRuleCollector.cpp:231: WillBeHeapVector<MatchedRule,
32>&
> matchedRules = m_matchedRules;
> On 2014/12/12 07:10:35, rune wrote:
> > Is it really necessary to have a new reference here? Couldn't m_matchedRules
> be
> > used directly below?
> 
> Yes, we can simplify this away now. Will do.

Done.

[sof, 2014-12-12 08:04:53]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-12-12 08:05:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/795043002/80001

[commit-bot: I haz the power, 2014-12-12 09:26:48]

Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187007