Move hashmap into base/.

Move hashmap into src/base.

We ported hashmap.h into libsampler as a workaround before, so the main focus of
this patch is to reduce code duplication. This patch moves the hashmap into
src/base as well as creates DefaultAllocationPolicy using malloc and free.

BUG=v8:5050
LOG=n

Committed: https://crrev.com/2fd55667a63a1428e9501336e5cd97f5431aa607
Cr-Commit-Position: refs/heads/master@{#36873}

[lpy, 2016-05-27 21:10:47]

lpy@chromium.org changed reviewers:
+ bmeurer@chromium.org, jochen@chromium.org, mstarzinger@chromium.org,
yangguo@chromium.org

[lpy, 2016-05-27 21:10:48]

PTAL.

[Benedikt Meurer, 2016-05-28 17:51:00]

Looks like Chrome switched to std::unordered_map (according to
base/containers/hash_tables.h). So it should be fine for us to also use
std::unordered_map instead of custom hash table?

[lpy, 2016-05-28 18:22:49]

On 2016/05/28 17:51:00, Benedikt Meurer wrote:
> Looks like Chrome switched to std::unordered_map (according to
> base/containers/hash_tables.h). So it should be fine for us to also use
> std::unordered_map instead of custom hash table?

For libsampler, the reason we want to use a custom hash table is because STL
containers use a mutex pool when running on debug build, so different containers
in the same thread may acquire the same mutex, which will lead to deadlock since
we are using async signal. And I assume std::unordered_map also uses the mutex
pool.

[Benedikt Meurer, 2016-05-29 18:07:03]

Aye, makes sense. Thanks for the explanation. Can you put that as a comment on
the HashMap class, so it's clear to everyone why we roll out our own hashmap?

Otherwise LGTM from my side.

[Michael Starzinger, 2016-05-30 08:11:56]

LGTM.

[Yang, 2016-05-30 09:19:04]

LGTM, with some comments.

https://codereview.chromium.org/2010243003/diff/60001/src/base/hashmap.cc
File src/base/hashmap.cc (right):

https://codereview.chromium.org/2010243003/diff/60001/src/base/hashmap.cc#new...
src/base/hashmap.cc:115: HashMapImpl::Entry* HashMapImpl::LookupOrInsert(void*
key, uint32_t hash) {
This is still very similar to
TemplateHashMapImpl<AllocationPolicy>::LookupOrInsert aside from the
AllocationPolicy.

Can we define a default allocation policy in src/base that uses malloc and free,
to get rid of the mostly duplicate code in
TemplateHashMapImpl<AllocationPolicy>::LookupOrInsert?

https://codereview.chromium.org/2010243003/diff/60001/src/base/hashmap.h
File src/base/hashmap.h (right):

https://codereview.chromium.org/2010243003/diff/60001/src/base/hashmap.h#newc...
src/base/hashmap.h:14: class HashMapImpl {
Imo this name is misleading. "Impl" stands for implementation. But this is not a
full implementation. Rather it has to be extended to be usable. Can we rename it
to HashMapBase or something along that line?

[lpy, 2016-05-31 20:05:42]

On 2016/05/30 09:19:04, Yang wrote:
> LGTM, with some comments.
> 
> https://codereview.chromium.org/2010243003/diff/60001/src/base/hashmap.cc
> File src/base/hashmap.cc (right):
> 
>
https://codereview.chromium.org/2010243003/diff/60001/src/base/hashmap.cc#new...
> src/base/hashmap.cc:115: HashMapImpl::Entry* HashMapImpl::LookupOrInsert(void*
> key, uint32_t hash) {
> This is still very similar to
> TemplateHashMapImpl<AllocationPolicy>::LookupOrInsert aside from the
> AllocationPolicy.
> 
> Can we define a default allocation policy in src/base that uses malloc and
free,
> to get rid of the mostly duplicate code in
> TemplateHashMapImpl<AllocationPolicy>::LookupOrInsert?
> 

That would be the same as FreeStoreAllocationPolicy. Why don't we move Malloced
and FreeStoreAllocationPolicy into src/base/ and replace FatalProcessOutOfMemory
with FATAL?

[lpy, 2016-05-31 23:54:48]

Description was changed from

==========
Move hashmap into base/.

We ported hashmap.h into libsampler as a workaround before, so the main focus
of this patch is to reduce code duplication. This patch moves the common
implementation into base/ and makes TemplateHashMapImpl inherit from it as a
version with allocation policy in v8.

BUG=v8:5050
LOG=n
==========

to

==========
Move hashmap into src/base.
    
We ported hashmap.h into libsampler as a workaround before, so the main focus of
this patch is to reduce code duplication. This patch moves the hashmap into
src/base as well as creates DefaultAllocationPolicy using malloc and free.

BUG=v8:5050
LOG=n
==========

[lpy, 2016-06-01 00:20:12]

I made a few changes, yang@ could you please take a look?

[lpy, 2016-06-08 08:43:42]

On 2016/06/01 00:20:12, lpy wrote:
> I made a few changes, yang@ could you please take a look?

yang@, gentle ping for review.

[Yang, 2016-06-08 08:49:16]

On 2016/06/08 08:43:42, lpy wrote:
> On 2016/06/01 00:20:12, lpy wrote:
> > I made a few changes, yang@ could you please take a look?
> 
> yang@, gentle ping for review.

LGTM. Thanks.

[lpy, 2016-06-08 17:47:44]

The CQ bit was checked by lpy@chromium.org

[lpy, 2016-06-08 17:47:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/2010243003/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2016-06-08 17:47:48]

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

[commit-bot: I haz the power, 2016-06-08 17:52:58]

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

[commit-bot: I haz the power, 2016-06-08 17:53:00]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/16777)

[lpy, 2016-06-08 18:39:13]

lpy@chromium.org changed reviewers:
+ bradnelson@chromium.org

[lpy, 2016-06-08 18:39:14]

+ bradnelson@ for src/wasm/ owner review.
+ adamk@ for src/parsing/ owner review.

bradnelson@, adamk@, PTAL.

[lpy, 2016-06-08 18:39:36]

lpy@chromium.org changed reviewers:
+ adamk@chromium.org

[adamk, 2016-06-08 20:31:59]

lgtm for parser

[lpy, 2016-06-09 06:04:41]

The CQ bit was checked by lpy@chromium.org

[lpy, 2016-06-09 06:04:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, yangguo@chromium.org, bmeurer@chromium.org
Link to the patchset: https://codereview.chromium.org/2010243003/#ps120001
(title: "Rebase")

[commit-bot: I haz the power, 2016-06-09 06:04:50]

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

[commit-bot: I haz the power, 2016-06-09 06:10:41]

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

[commit-bot: I haz the power, 2016-06-09 06:10:42]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/16804)

[bradnelson, 2016-06-09 17:49:27]

lgtm on wasm parts.

[lpy, 2016-06-09 17:51:30]

Thanks for reviewing this patch.

[lpy, 2016-06-09 17:55:19]

The CQ bit was checked by lpy@chromium.org

[commit-bot: I haz the power, 2016-06-09 17:55:23]

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

[commit-bot: I haz the power, 2016-06-09 17:58:17]

Description was changed from

==========
Move hashmap into src/base.
    
We ported hashmap.h into libsampler as a workaround before, so the main focus of
this patch is to reduce code duplication. This patch moves the hashmap into
src/base as well as creates DefaultAllocationPolicy using malloc and free.

BUG=v8:5050
LOG=n
==========

to

==========
Move hashmap into src/base.
    
We ported hashmap.h into libsampler as a workaround before, so the main focus of
this patch is to reduce code duplication. This patch moves the hashmap into
src/base as well as creates DefaultAllocationPolicy using malloc and free.

BUG=v8:5050
LOG=n
==========

[commit-bot: I haz the power, 2016-06-09 17:58:19]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-06-09 18:00:38]

Description was changed from

==========
Move hashmap into src/base.
    
We ported hashmap.h into libsampler as a workaround before, so the main focus of
this patch is to reduce code duplication. This patch moves the hashmap into
src/base as well as creates DefaultAllocationPolicy using malloc and free.

BUG=v8:5050
LOG=n
==========

to

==========
Move hashmap into src/base.

We ported hashmap.h into libsampler as a workaround before, so the main focus of
this patch is to reduce code duplication. This patch moves the hashmap into
src/base as well as creates DefaultAllocationPolicy using malloc and free.

BUG=v8:5050
LOG=n

Committed: https://crrev.com/2fd55667a63a1428e9501336e5cd97f5431aa607
Cr-Commit-Position: refs/heads/master@{#36873}
==========

[commit-bot: I haz the power, 2016-06-09 18:00:40]

Patchset 7 (id:??) landed as
https://crrev.com/2fd55667a63a1428e9501336e5cd97f5431aa607
Cr-Commit-Position: refs/heads/master@{#36873}