[interpreter] Use hashmap for ConstantArrayBuilder's constant map

[interpreter] Use hashmap for ConstantArrayBuilder's constant map

Uses the base hashmap to store the ConstantArrayBuilder's constant map,
which slightly improves the performance of ConstantArrayBuilder::Insert.

Includes a small overload of the hashmap LookupOrInsert method, which
allows passing in a value creation function instead of just default
initialising new values.

On Octane's codeload, this gives (on my machine) a 0.27% improvement,
which doesn't sound like a lot but I guess every little helps.

Committed: https://crrev.com/0134ddae6b6ac73d04a65134de55e77f37374cdb
Cr-Commit-Position: refs/heads/master@{#39883}

[Leszek, 2016-09-12 12:51:24]

leszeks@google.com changed reviewers:
+ leszeks@google.com, rmcilroy@chromium.org

[Leszek, 2016-09-12 12:51:25]

First pass, probably not ready for submission. I didn't unify this hashmap with
the base hashmap for now because I was afraid of breaking code using the latter.

[rmcilroy, 2016-09-12 14:17:31]

Overall changes in interpreter look fine, but I'd like to see this integrated
into base/hashmap if possible before landing. 

Also needs a more detailed CL description / title.

https://codereview.chromium.org/2336553002/diff/40001/src/inline-hash-map.h
File src/inline-hash-map.h (right):

https://codereview.chromium.org/2336553002/diff/40001/src/inline-hash-map.h#n...
src/inline-hash-map.h:6: #define V8_ZONE_TEMPLATE_HASH_MAP_H_
I wonder how much of this should really live in the header. I know hashmap.h
does, but maybe we could take this opportunity to move some of the
implementation in a cc file (presumably just the non-fast-path parts though).

https://codereview.chromium.org/2336553002/diff/40001/src/inline-hash-map.h#n...
src/inline-hash-map.h:11: namespace internal {
This should really be v8/base (and in the base directory)

https://codereview.chromium.org/2336553002/diff/40001/src/inline-hash-map.h#n...
src/inline-hash-map.h:19: namespace detail {
Typically we use annoymous namespaces for private details, but I realize that
doesn't work for headers (hence my comment above). Maybe you could make a
inline-hash-map.h file with the interface, and a inline-hash-map-inl.h header
with the implementation, which is in a namespace internal (and is included in
the main header)?

https://codereview.chromium.org/2336553002/diff/40001/src/inline-hash-map.h#n...
src/inline-hash-map.h:181: map_(nullptr),
As discussed offline, let's make this just eagerly initialized.

https://codereview.chromium.org/2336553002/diff/40001/src/interpreter/constan...
File src/interpreter/constant-array-builder.h (right):

https://codereview.chromium.org/2336553002/diff/40001/src/interpreter/constan...
src/interpreter/constant-array-builder.h:23: // Divide by 8, then knuth
multiplicative hash
Should we divide 8 or by kPointerSize?

[Leszek Swirski, 2016-09-19 09:18:09]

Splitting this into multiple, more surgical CLs on top of base/hashmap first.

[Leszek Swirski, 2016-09-20 14:47:14]

Description was changed from

==========
[Interpreter] Use custom hashmap for constant map

BUG=
==========

to

==========
[interpreter] Use hashmap for ConstantArrayBuilder's constant map

Uses the base hashmap to store the ConstantArrayBuilder's constant map,
which slightly improves the performance of ConstantArrayBuilder::Insert.

Includes a small overload of the hashmap LookupOrInsert method, which
allows passing in a value creation function instead of just default
initialising new values.

On Octane's codeload, this gives (on my machine) a 0.27% improvement,
which doesn't sound like a lot but I guess every little helps.
==========

[Leszek Swirski, 2016-09-20 15:04:43]

Description was changed from

==========
[interpreter] Use hashmap for ConstantArrayBuilder's constant map

Uses the base hashmap to store the ConstantArrayBuilder's constant map,
which slightly improves the performance of ConstantArrayBuilder::Insert.

Includes a small overload of the hashmap LookupOrInsert method, which
allows passing in a value creation function instead of just default
initialising new values.

On Octane's codeload, this gives (on my machine) a 0.27% improvement,
which doesn't sound like a lot but I guess every little helps.
==========

to

==========
[interpreter] Use hashmap for ConstantArrayBuilder's constant map

Uses the base hashmap to store the ConstantArrayBuilder's constant map,
which slightly improves the performance of ConstantArrayBuilder::Insert.

Includes a small overload of the hashmap LookupOrInsert method, which
allows passing in a value creation function instead of just default
initialising new values.

On Octane's codeload, this gives (on my machine) a 0.27% improvement,
which doesn't sound like a lot but I guess every little helps.
==========

[Leszek Swirski, 2016-09-20 15:04:59]

Updated removing the custom hashmap, PTAL

[rmcilroy, 2016-09-22 17:56:34]

Sorry, missed this - please feel free to ping me if I don't respond to a review
within a day or so.

LGTM.

[rmcilroy, 2016-09-26 14:00:12]

Just wondering, are you planning on landing this?

[Leszek Swirski, 2016-09-26 14:21:15]

On 2016/09/26 14:00:12, rmcilroy wrote:
> Just wondering, are you planning on landing this?

Yes, sorry, there's the dependent patch set that I need to finish fixing up and
then this one can go through.

[rmcilroy, 2016-09-26 14:48:39]

On 2016/09/26 14:21:15, Leszek Swirski wrote:
> On 2016/09/26 14:00:12, rmcilroy wrote:
> > Just wondering, are you planning on landing this?
> 
> Yes, sorry, there's the dependent patch set that I need to finish fixing up
and
> then this one can go through.

Ahh I see, no worries, thanks.

[Leszek Swirski, 2016-09-29 15:46:49]

The CQ bit was checked by leszeks@chromium.org

[Leszek Swirski, 2016-09-29 15:46:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/2336553002/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2016-09-29 15:46:59]

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-29 16:14:31]

Description was changed from

==========
[interpreter] Use hashmap for ConstantArrayBuilder's constant map

Uses the base hashmap to store the ConstantArrayBuilder's constant map,
which slightly improves the performance of ConstantArrayBuilder::Insert.

Includes a small overload of the hashmap LookupOrInsert method, which
allows passing in a value creation function instead of just default
initialising new values.

On Octane's codeload, this gives (on my machine) a 0.27% improvement,
which doesn't sound like a lot but I guess every little helps.
==========

to

==========
[interpreter] Use hashmap for ConstantArrayBuilder's constant map

Uses the base hashmap to store the ConstantArrayBuilder's constant map,
which slightly improves the performance of ConstantArrayBuilder::Insert.

Includes a small overload of the hashmap LookupOrInsert method, which
allows passing in a value creation function instead of just default
initialising new values.

On Octane's codeload, this gives (on my machine) a 0.27% improvement,
which doesn't sound like a lot but I guess every little helps.
==========

[commit-bot: I haz the power, 2016-09-29 16:14:32]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-29 16:15:04]

Description was changed from

==========
[interpreter] Use hashmap for ConstantArrayBuilder's constant map

Uses the base hashmap to store the ConstantArrayBuilder's constant map,
which slightly improves the performance of ConstantArrayBuilder::Insert.

Includes a small overload of the hashmap LookupOrInsert method, which
allows passing in a value creation function instead of just default
initialising new values.

On Octane's codeload, this gives (on my machine) a 0.27% improvement,
which doesn't sound like a lot but I guess every little helps.
==========

to

==========
[interpreter] Use hashmap for ConstantArrayBuilder's constant map

Uses the base hashmap to store the ConstantArrayBuilder's constant map,
which slightly improves the performance of ConstantArrayBuilder::Insert.

Includes a small overload of the hashmap LookupOrInsert method, which
allows passing in a value creation function instead of just default
initialising new values.

On Octane's codeload, this gives (on my machine) a 0.27% improvement,
which doesn't sound like a lot but I guess every little helps.

Committed: https://crrev.com/0134ddae6b6ac73d04a65134de55e77f37374cdb
Cr-Commit-Position: refs/heads/master@{#39883}
==========

[commit-bot: I haz the power, 2016-09-29 16:15:06]

Patchset 6 (id:??) landed as
https://crrev.com/0134ddae6b6ac73d04a65134de55e77f37374cdb
Cr-Commit-Position: refs/heads/master@{#39883}