HashTable: bring per-table stats tracking back to life.

HashTable: bring per-table stats tracking back to life.

Recording per-hash table stats (DUMP_HASHTABLE_STATS_PER_TABLE)
broke with the introduction of Oilpan, as the feature depended
on finalizable HashTable<>s, something Oilpan heap hash tables
are not.

If the hash table resides on the Oilpan heap, arrange for the
stats object to also reside there.

While here, also unify the handling of global HashTable stats
recording and the per-table representation.

R=
BUG=

Committed: https://crrev.com/1fc422b4d064b0123155fafe162f4f01d9f44df2
Cr-Commit-Position: refs/heads/master@{#433494}

[sof, 2016-11-18 15:14:36]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-18 15:15:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sof, 2016-11-18 15:32:11]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-18 15:32:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sof, 2016-11-18 15:33:19]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-18 15:33:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sof, 2016-11-18 15:38:28]

sigbjornf@opera.com changed reviewers:
+ haraken@chromium.org, yutak@chromium.org

[sof, 2016-11-18 15:38:28]

please take a look.

while investigating hash table behavior "in the wild", i discovered that this
older stats feature wasn't working all that well wrt Oilpan. I guess that says
something about overall usage of the feature.. :)

If it has merit to keep the per-table stats recording around, this adds it back.

[haraken, 2016-11-18 15:49:20]

LGTM

[haraken, 2016-11-18 15:49:35]

On 2016/11/18 15:49:20, haraken wrote:
> LGTM

[haraken, 2016-11-18 15:50:12]

> while investigating hash table behavior "in the wild"

Did you find anything interesting?

[sof, 2016-11-18 16:14:14]

On 2016/11/18 15:50:12, haraken wrote:
> > while investigating hash table behavior "in the wild"
> 
> Did you find anything interesting?

Collision-wise, nothing interesting found yet (looking at LocalDOMWindow uses
mostly.)

( I've been meaning to take a closer look at rehashing etc for a busy table like
the prefinalizer table for a while, but haven't done so. )

[commit-bot: I haz the power, 2016-11-18 18:30:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-18 18:30:42]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[sof, 2016-11-20 08:13:30]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-20 08:13:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-20 09:08:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-20 09:08:54]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sof, 2016-11-20 11:29:50]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-20 11:29:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sof, 2016-11-20 11:35:41]

The placement of the marking call for the stats table wasn't correct, so moved
up so that it always happens.

Also made the creation of this extra object track the lifetime of the backing
store -- it's an object of non-trivial size, so don't want to create it too
early.

[commit-bot: I haz the power, 2016-11-20 13:01:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-20 13:01:16]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[haraken, 2016-11-21 00:16:30]

LGTM

[Yuta Kitamura, 2016-11-21 04:51:42]

lgtm

[sof, 2016-11-21 06:12:52]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2016-11-21 06:13:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 06:46:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 06:46:45]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sof, 2016-11-21 06:49:21]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2016-11-21 06:49:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 08:07:34]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1479710961303160,
"parent_rev": ["9e308a6b4b316784297753afe37e28a854d77e90", null], "commit_rev":
["b354cb87ecd75855d53b59bd5fcdf8bb549715d7", null]}

[commit-bot: I haz the power, 2016-11-21 08:09:23]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1479710961303160,
"parent_rev": ["425ea191317deff921ae464ef60a5e1b19127efc", null], "commit_rev":
["a00e102157543e32019d04dee49f228fb5ed34fc", null]}

[commit-bot: I haz the power, 2016-11-21 08:09:47]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-21 08:12:00]

Description was changed from

==========
HashTable: bring per-table stats tracking back to life.

Recording per-hash table stats (DUMP_HASHTABLE_STATS_PER_TABLE)
broke with the introduction of Oilpan, as the feature depended
on finalizable HashTable<>s, something Oilpan heap hash tables
are not.

If the hash table resides on the Oilpan heap, arrange for the
stats object to also reside there.

While here, also unify the handling of global HashTable stats
recording and the per-table representation.

R=
BUG=
==========

to

==========
HashTable: bring per-table stats tracking back to life.

Recording per-hash table stats (DUMP_HASHTABLE_STATS_PER_TABLE)
broke with the introduction of Oilpan, as the feature depended
on finalizable HashTable<>s, something Oilpan heap hash tables
are not.

If the hash table resides on the Oilpan heap, arrange for the
stats object to also reside there.

While here, also unify the handling of global HashTable stats
recording and the per-table representation.

R=
BUG=

Committed: https://crrev.com/1fc422b4d064b0123155fafe162f4f01d9f44df2
Cr-Commit-Position: refs/heads/master@{#433494}
==========

[commit-bot: I haz the power, 2016-11-21 08:12:01]

Patchset 5 (id:??) landed as
https://crrev.com/1fc422b4d064b0123155fafe162f4f01d9f44df2
Cr-Commit-Position: refs/heads/master@{#433494}

[sof, 2016-11-21 15:19:59]

On 2016/11/18 16:14:14, sof wrote:
> On 2016/11/18 15:50:12, haraken wrote:
> > > while investigating hash table behavior "in the wild"
> > 
> > Did you find anything interesting?
> 
> Collision-wise, nothing interesting found yet (looking at LocalDOMWindow uses
> mostly.)
> 
> ( I've been meaning to take a closer look at rehashing etc for a busy table
like
> the prefinalizer table for a while, but haven't done so. )

It isn't unusual to see a couple of rehashes of this table during a complete GC
cycle.

Prefinalizers seem to follow an "objects die young" lifetime pattern, so having
some form of "rehash disallowed scope" during their invocation or reconsidering
the representation of the registered prefinalizers, is possibly worth doing. In
the limited tests done, I didn't see table sizes exceeding 1k.

(The support for explicit unregistration can arguably be dropped as that
flexibility hasn't proven to be useful in practice.)

[haraken, 2016-11-22 00:40:19]

On 2016/11/21 15:19:59, sof wrote:
> On 2016/11/18 16:14:14, sof wrote:
> > On 2016/11/18 15:50:12, haraken wrote:
> > > > while investigating hash table behavior "in the wild"
> > > 
> > > Did you find anything interesting?
> > 
> > Collision-wise, nothing interesting found yet (looking at LocalDOMWindow
uses
> > mostly.)
> > 
> > ( I've been meaning to take a closer look at rehashing etc for a busy table
> like
> > the prefinalizer table for a while, but haven't done so. )
> 
> It isn't unusual to see a couple of rehashes of this table during a complete
GC
> cycle.
> 
> Prefinalizers seem to follow an "objects die young" lifetime pattern, so
having
> some form of "rehash disallowed scope" during their invocation or
reconsidering
> the representation of the registered prefinalizers, is possibly worth doing.
In
> the limited tests done, I didn't see table sizes exceeding 1k.

+1.

> (The support for explicit unregistration can arguably be dropped as that
> flexibility hasn't proven to be useful in practice.)

Even if we don't have an explicit unregistration,
m_orderedPreFinalizers.remove() in invokePreFinalizers() will cause the
rehashing, right?

[sof, 2016-11-22 07:23:19]

On 2016/11/22 00:40:19, haraken wrote:
> On 2016/11/21 15:19:59, sof wrote:
> > On 2016/11/18 16:14:14, sof wrote:
> > > On 2016/11/18 15:50:12, haraken wrote:
> > > > > while investigating hash table behavior "in the wild"
> > > > 
> > > > Did you find anything interesting?
> > > 
> > > Collision-wise, nothing interesting found yet (looking at LocalDOMWindow
> uses
> > > mostly.)
> > > 
> > > ( I've been meaning to take a closer look at rehashing etc for a busy
table
> > like
> > > the prefinalizer table for a while, but haven't done so. )
> > 
> > It isn't unusual to see a couple of rehashes of this table during a complete
> GC
> > cycle.
> > 
> > Prefinalizers seem to follow an "objects die young" lifetime pattern, so
> having
> > some form of "rehash disallowed scope" during their invocation or
> reconsidering
> > the representation of the registered prefinalizers, is possibly worth doing.
> In
> > the limited tests done, I didn't see table sizes exceeding 1k.
> 
> +1.
> 
> > (The support for explicit unregistration can arguably be dropped as that
> > flexibility hasn't proven to be useful in practice.)
> 
> Even if we don't have an explicit unregistration,
> m_orderedPreFinalizers.remove() in invokePreFinalizers() will cause the
> rehashing, right?

Yes, the only place where removals happen.