Shuffle fields around in Scope to save more zone memory

Shuffle fields around in Scope to save more zone memory

This makes the debug-only scope-name actually debug-only-allocated, replaces num_vars_ usages by variables_.occupancy, and shuffles fields around in the scope class for better packing.

This reduces sizeof(i::Scope) from 360 to 328 bytes on x64.

BUG=v8:5209
Committed: https://crrev.com/d454e6cc59f37c6b19b3b6694dfd9f446cf61f5d
Cr-Commit-Position: refs/heads/master@{#38210}

[Toon Verwaest, 2016-08-01 12:49:27]

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

[commit-bot: I haz the power, 2016-08-01 12:49:38]

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

[Toon Verwaest, 2016-08-01 12:50:57]

Description was changed from

==========
Shuffle fields around in Scope to save more zone memory

This makes the debug-only scope-name actually debug-only-allocated, replaces
num_vars_ usages by variables_.occupancy, and shuffles fields around in the
scope class for better packing.

BUG=v8:5209
==========

to

==========
Shuffle fields around in Scope to save more zone memory

This makes the debug-only scope-name actually debug-only-allocated, replaces
num_vars_ usages by variables_.occupancy, and shuffles fields around in the
scope class for better packing.

This reduces sizeof(i::Scope) from 360 to 328 bytes on x64.

BUG=v8:5209
==========

[Toon Verwaest, 2016-08-01 12:51:15]

verwaest@chromium.org changed reviewers:
+ marja@chromium.org

[Toon Verwaest, 2016-08-01 12:51:16]

ptal

[marja, 2016-08-01 12:55:05]

lgtm % comment

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

https://codereview.chromium.org/2201763004/diff/20001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:997: #endif
This code block is already inside #ifdef DEBUG, so this change is not needed.

[Toon Verwaest, 2016-08-01 12:56:54]

The CQ bit was unchecked by verwaest@chromium.org

[Toon Verwaest, 2016-08-01 12:56:56]

The CQ bit was checked by verwaest@chromium.org

[commit-bot: I haz the power, 2016-08-01 12:56:59]

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

[Toon Verwaest, 2016-08-01 12:57:06]

The CQ bit was unchecked by verwaest@chromium.org

[Toon Verwaest, 2016-08-01 12:58:28]

The CQ bit was checked by verwaest@chromium.org

[Toon Verwaest, 2016-08-01 12:58:29]

The patchset sent to the CQ was uploaded after l-g-t-m from marja@chromium.org
Link to the patchset: https://codereview.chromium.org/2201763004/#ps40001
(title: "Addressed comment")

[commit-bot: I haz the power, 2016-08-01 12:58:39]

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-08-01 13:25:45]

Description was changed from

==========
Shuffle fields around in Scope to save more zone memory

This makes the debug-only scope-name actually debug-only-allocated, replaces
num_vars_ usages by variables_.occupancy, and shuffles fields around in the
scope class for better packing.

This reduces sizeof(i::Scope) from 360 to 328 bytes on x64.

BUG=v8:5209
==========

to

==========
Shuffle fields around in Scope to save more zone memory

This makes the debug-only scope-name actually debug-only-allocated, replaces
num_vars_ usages by variables_.occupancy, and shuffles fields around in the
scope class for better packing.

This reduces sizeof(i::Scope) from 360 to 328 bytes on x64.

BUG=v8:5209
==========

[commit-bot: I haz the power, 2016-08-01 13:25:47]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-01 13:27:34]

Description was changed from

==========
Shuffle fields around in Scope to save more zone memory

This makes the debug-only scope-name actually debug-only-allocated, replaces
num_vars_ usages by variables_.occupancy, and shuffles fields around in the
scope class for better packing.

This reduces sizeof(i::Scope) from 360 to 328 bytes on x64.

BUG=v8:5209
==========

to

==========
Shuffle fields around in Scope to save more zone memory

This makes the debug-only scope-name actually debug-only-allocated, replaces
num_vars_ usages by variables_.occupancy, and shuffles fields around in the
scope class for better packing.

This reduces sizeof(i::Scope) from 360 to 328 bytes on x64.

BUG=v8:5209

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

[commit-bot: I haz the power, 2016-08-01 13:27:35]

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

[adamk, 2016-08-01 22:47:07]

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

[adamk, 2016-08-01 22:47:08]

https://codereview.chromium.org/2201763004/diff/40001/src/ast/scopes.h
File src/ast/scopes.h (right):

https://codereview.chromium.org/2201763004/diff/40001/src/ast/scopes.h#newcod...
src/ast/scopes.h:606: int num_var() const { return variables_.occupancy(); }
You might have noticed that this slightly changes the behavior of num_var(), but
given that the only external caller is testing whether we have more than 2^22-1
local variables
(https://cs.chromium.org/chromium/src/v8/src/parsing/pattern-rewriter.cc?q=num...),
I'm guessing you don't care?

[Toon Verwaest, 2016-08-02 20:24:43]

The new behavior is probably closer to what was intended: make sure we don't
allocate too many variables. num_vars would include duplicates, and thus claim
to be out of space which shouldn't be a problem if they are all duplicates. It
probably doesn't matter much either way though.