[base] Move hashmap allocator to a field

[base] Move hashmap allocator to a field

Moves the hashmap's allocator from being a parameter in the various
hashmap functions, to being a field in the hashmap itself. This

1. Protects against incorrectly passed allocators, and
2. Cleans up the API so that e.g. callers don't have to store their
   allocator

This is part of a wider set of changes discussed in:
https://groups.google.com/forum/#!topic/v8-dev/QLsC0XPYLeM

Committed: https://crrev.com/b42ecda533bd53ddc17f55e256236f23253382b8
Cr-Commit-Position: refs/heads/master@{#39538}

[Leszek Swirski, 2016-09-16 16:22:53]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 16:23:01]

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

[Leszek Swirski, 2016-09-16 16:23:42]

leszeks@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[Leszek Swirski, 2016-09-16 16:23:43]

This will need a lot of reviewers...

[Leszek Swirski, 2016-09-16 16:28:51]

Description was changed from

==========
[base] Move hashmap allocator to a field

Moves the hashmap's allocator from being a parameter in the various
hashmap functions, to being a field in the hashmap itself. This

1. Protects against incorrectly passed allocators, and
2. Cleans up the API so that e.g. callers don't have to store their
   allocator

This 'field' is actually combined with the "Entry* map" field to take
advantage of the empty base optimisation, so that the allocator does not
take up space in the hashmap if it is stateless.
==========

to

==========
[base] Move hashmap allocator to a field

Moves the hashmap's allocator from being a parameter in the various
hashmap functions, to being a field in the hashmap itself. This

1. Protects against incorrectly passed allocators, and
2. Cleans up the API so that e.g. callers don't have to store their
   allocator

This 'field' is actually combined with the "Entry* map" field to take
advantage of the empty base optimisation, so that the allocator does not
take up space in the hashmap if it is stateless.

This is part of a wider set of changes discussed in:
https://groups.google.com/forum/#!topic/v8-dev/QLsC0XPYLeM
==========

[commit-bot: I haz the power, 2016-09-16 16:51:59]

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

[commit-bot: I haz the power, 2016-09-16 16:52:00]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2016-09-19 10:55:11]

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

https://codereview.chromium.org/2345233003/diff/1/src/base/hashmap.h#newcode99
src/base/hashmap.h:99: // DefaultAllocationPolicy above.
Let's make this a field instead of using the empty base optimization. I'd prefer
to keep ZoneAllocationPolicy final.

[Leszek Swirski, 2016-09-19 12:20:40]

Description was changed from

==========
[base] Move hashmap allocator to a field

Moves the hashmap's allocator from being a parameter in the various
hashmap functions, to being a field in the hashmap itself. This

1. Protects against incorrectly passed allocators, and
2. Cleans up the API so that e.g. callers don't have to store their
   allocator

This 'field' is actually combined with the "Entry* map" field to take
advantage of the empty base optimisation, so that the allocator does not
take up space in the hashmap if it is stateless.

This is part of a wider set of changes discussed in:
https://groups.google.com/forum/#!topic/v8-dev/QLsC0XPYLeM
==========

to

==========
[base] Move hashmap allocator to a field

Moves the hashmap's allocator from being a parameter in the various
hashmap functions, to being a field in the hashmap itself. This

1. Protects against incorrectly passed allocators, and
2. Cleans up the API so that e.g. callers don't have to store their
   allocator

This is part of a wider set of changes discussed in:
https://groups.google.com/forum/#!topic/v8-dev/QLsC0XPYLeM
==========

[Leszek Swirski, 2016-09-19 12:20:56]

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

https://codereview.chromium.org/2345233003/diff/1/src/base/hashmap.h#newcode99
src/base/hashmap.h:99: // DefaultAllocationPolicy above.
On 2016/09/19 10:55:11, rmcilroy wrote:
> Let's make this a field instead of using the empty base optimization. I'd
prefer
> to keep ZoneAllocationPolicy final.

Fair enough, done.

[Leszek Swirski, 2016-09-19 12:59:35]

leszeks@chromium.org changed reviewers:
+ ahaas@chromium.org, bmeurer@chromium.org, marja@chromium.org

[Leszek Swirski, 2016-09-19 12:59:35]

ahaas@chromium.org: Please review changes in 

marja@chromium.org: Please review changes in 

bmeurer@chromium.org: Please review changes in 

Hopefully this is just rubber-stamping for you.

[ahaas, 2016-09-19 13:03:52]

On 2016/09/19 at 12:59:35, leszeks wrote:
> ahaas@chromium.org: Please review changes in 
> 
> marja@chromium.org: Please review changes in 
> 
> bmeurer@chromium.org: Please review changes in 
> 
> Hopefully this is just rubber-stamping for you.

asmjs lgtm

[Benedikt Meurer, 2016-09-19 13:11:19]

LGTM

[rmcilroy, 2016-09-19 13:51:43]

LGTM

[Leszek Swirski, 2016-09-19 16:11:39]

The CQ bit was checked by leszeks@chromium.org

[commit-bot: I haz the power, 2016-09-19 16:11:47]

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

[commit-bot: I haz the power, 2016-09-19 16:11:49]

This CL has an open dependency (Issue 2343123002 Patch 40001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[marja, 2016-09-20 08:34:54]

https://codereview.chromium.org/2345233003/diff/20001/src/ast/scopes.cc
File src/ast/scopes.cc (right):

https://codereview.chromium.org/2345233003/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:30: Variable* VariableMap::Declare(Zone* zone, Scope* scope,
Can you now drop the zone parameter here (and above)?

https://codereview.chromium.org/2345233003/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:56: void VariableMap::Add(Zone* zone, Variable* var) {
Ditto

https://codereview.chromium.org/2345233003/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:78: void SloppyBlockFunctionMap::Declare(Zone* zone, const
AstRawString* name,
Ditto

[marja, 2016-09-20 09:06:27]

heimbuef, fyi, this is related to your Zone work (allows us to drop one Zone
parameter)

[Leszek Swirski, 2016-09-20 09:19:21]

Thanks Marja, I'll wait for an LGTM from you before submitting.

https://codereview.chromium.org/2345233003/diff/20001/src/ast/scopes.cc
File src/ast/scopes.cc (right):

https://codereview.chromium.org/2345233003/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:30: Variable* VariableMap::Declare(Zone* zone, Scope* scope,
On 2016/09/20 08:34:53, marja wrote:
> Can you now drop the zone parameter here (and above)?

Here it's still used for the Variable allocation, though we could consider
changing the map from void* values to Variable values, to allow the variables to
be allocated and stored inline in the map. That *could* have better cache
behaviour, at the cost of increased code size from a different template
instantiation (and maybe it'd behave worse with the instruction cache? dunno.)

https://codereview.chromium.org/2345233003/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:56: void VariableMap::Add(Zone* zone, Variable* var) {
On 2016/09/20 08:34:53, marja wrote:
> Ditto

Done.

https://codereview.chromium.org/2345233003/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:78: void SloppyBlockFunctionMap::Declare(Zone* zone, const
AstRawString* name,
On 2016/09/20 08:34:53, marja wrote:
> Ditto

Done.

[marja, 2016-09-20 09:33:33]

ast/ lgtm once it builds ;)

(Ah yes, we still need to create the Variables inside the VariableMap::Declare;
makes sense to *not* touch that in this CL.)

[Leszek Swirski, 2016-09-20 09:34:23]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-20 09:34:31]

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-09-20 09:39:32]

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

[commit-bot: I haz the power, 2016-09-20 09:39:33]

Dry run: Try jobs failed on following builders:
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/9196)

[Leszek Swirski, 2016-09-20 09:49:52]

Looks like my morning coffee isn't working. Newest patchset compiles again.

[Leszek Swirski, 2016-09-20 09:49:58]

The CQ bit was checked by leszeks@chromium.org

[Leszek Swirski, 2016-09-20 09:49:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, rmcilroy@chromium.org, ahaas@chromium.org,
marja@chromium.org
Link to the patchset: https://codereview.chromium.org/2345233003/#ps60001
(title: "Remove zone arguments where the parameters were removed so that the
bloody thing compiles again")

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

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-09-20 10:45:35]

Description was changed from

==========
[base] Move hashmap allocator to a field

Moves the hashmap's allocator from being a parameter in the various
hashmap functions, to being a field in the hashmap itself. This

1. Protects against incorrectly passed allocators, and
2. Cleans up the API so that e.g. callers don't have to store their
   allocator

This is part of a wider set of changes discussed in:
https://groups.google.com/forum/#!topic/v8-dev/QLsC0XPYLeM
==========

to

==========
[base] Move hashmap allocator to a field

Moves the hashmap's allocator from being a parameter in the various
hashmap functions, to being a field in the hashmap itself. This

1. Protects against incorrectly passed allocators, and
2. Cleans up the API so that e.g. callers don't have to store their
   allocator

This is part of a wider set of changes discussed in:
https://groups.google.com/forum/#!topic/v8-dev/QLsC0XPYLeM
==========

[commit-bot: I haz the power, 2016-09-20 10:45:36]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-20 10:46:11]

Description was changed from

==========
[base] Move hashmap allocator to a field

Moves the hashmap's allocator from being a parameter in the various
hashmap functions, to being a field in the hashmap itself. This

1. Protects against incorrectly passed allocators, and
2. Cleans up the API so that e.g. callers don't have to store their
   allocator

This is part of a wider set of changes discussed in:
https://groups.google.com/forum/#!topic/v8-dev/QLsC0XPYLeM
==========

to

==========
[base] Move hashmap allocator to a field

Moves the hashmap's allocator from being a parameter in the various
hashmap functions, to being a field in the hashmap itself. This

1. Protects against incorrectly passed allocators, and
2. Cleans up the API so that e.g. callers don't have to store their
   allocator

This is part of a wider set of changes discussed in:
https://groups.google.com/forum/#!topic/v8-dev/QLsC0XPYLeM

Committed: https://crrev.com/b42ecda533bd53ddc17f55e256236f23253382b8
Cr-Commit-Position: refs/heads/master@{#39538}
==========

[commit-bot: I haz the power, 2016-09-20 10:46:12]

Patchset 4 (id:??) landed as
https://crrev.com/b42ecda533bd53ddc17f55e256236f23253382b8
Cr-Commit-Position: refs/heads/master@{#39538}