[runtime] Allocate space for computed property names

[runtime] Allocate space for computed property names.

Allocate space in the backing store for computed property names. 

The property backing store was pre-allocated for the constant 
properties up to the first non-constant (computed name) property.
To use lowering for storing data properties in literals 
with computed property names effectively, a fast store is needed, i.e., 
available space in the property backing store for properties 
with computed names. 

backing_store_size is the number of all properties (including 
computed names, but without __proto__)
that is calculated in the ast and passed to the runtime function that allocates
the property backing store. backing_store_size and 
constant_properties constitute a BoilerplateDescription.

backing_store_size might be slightly too high because computed names 
can evaluate to the same name, but that should be a rare
case so over-allocating is OK. 

If a property is __proto__, we don't store it as a regular 
property, because the map changes. Keep track of
has_seen_proto in the parser to calculate the
backing store size correctly. 

BUG=v8:5625
Review-Url: https://codereview.chromium.org/2632503003
Cr-Commit-Position: refs/heads/master@{#42576}
Committed: https://chromium.googlesource.com/v8/v8/+/399f36b5185deafa263def840d8f98bb61ff3ab4

[Franzi, 2017-01-13 10:11:53]

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

[commit-bot: I haz the power, 2017-01-13 10:12:03]

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, 2017-01-13 10:30:34]

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

[commit-bot: I haz the power, 2017-01-13 10:30:34]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux64_asan_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng_trig...)

[Franzi, 2017-01-14 20:57:41]

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

[commit-bot: I haz the power, 2017-01-14 20:57:42]

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, 2017-01-14 21:38:27]

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

[commit-bot: I haz the power, 2017-01-14 21:38:28]

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

[Franzi, 2017-01-14 21:42:20]

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

[commit-bot: I haz the power, 2017-01-14 21:42:28]

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, 2017-01-14 22:07:27]

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

[commit-bot: I haz the power, 2017-01-14 22:07:28]

Dry run: This issue passed the CQ dry run.

[Franzi, 2017-01-16 12:39:00]

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

[commit-bot: I haz the power, 2017-01-16 12:39:09]

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, 2017-01-16 12:40:15]

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

[commit-bot: I haz the power, 2017-01-16 12:40:16]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/15646)

[Franzi, 2017-01-16 15:10:56]

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

[commit-bot: I haz the power, 2017-01-16 15:11:03]

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

[Franzi, 2017-01-16 15:15:33]

Description was changed from

==========
[parser] Make space for computed property names.

Instead of only storing constant and computed value properties
in the in object properties, reserve space for
computed property names as well.

BUG=v8:5625
==========

to

==========
[parser] Allocate space for computed property names.

Allocate space in the backing store for computed property names. 

BUG=v8:5625
==========

[Franzi, 2017-01-16 15:15:33]

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

[Franzi, 2017-01-16 16:56:48]

Hi Toon, 

Could you have a look? I'm not super happy with that hack of passing the total
number as last element. Do you have any suggestions how to abstract that better?


Thanks, 

Franzi

[Franzi, 2017-01-16 16:57:14]

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

[commit-bot: I haz the power, 2017-01-16 16:57:18]

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, 2017-01-16 17:28:30]

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

[commit-bot: I haz the power, 2017-01-16 17:28:31]

Dry run: This issue passed the CQ dry run.

[gsathya, 2017-01-17 18:31:38]

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

[gsathya, 2017-01-17 18:31:39]

https://codereview.chromium.org/2632503003/diff/140001/src/ast/ast.cc
File src/ast/ast.cc (right):

https://codereview.chromium.org/2632503003/diff/140001/src/ast/ast.cc#newcode627
src/ast/ast.cc:627: isolate->factory()->NewNumberFromInt(total_properties_);
total_properties_ can be calculated using the above for loop right? then we
don't need this to be a property of ObjectLiteral

[Franzi, 2017-01-17 19:08:45]

https://codereview.chromium.org/2632503003/diff/140001/src/ast/ast.cc
File src/ast/ast.cc (right):

https://codereview.chromium.org/2632503003/diff/140001/src/ast/ast.cc#newcode627
src/ast/ast.cc:627: isolate->factory()->NewNumberFromInt(total_properties_);
On 2017/01/17 18:31:39, gsathya wrote:
> total_properties_ can be calculated using the above for loop right? then we
> don't need this to be a property of ObjectLiteral

The loop breaks on the first computed property, and I'd have to do the rest of
the iteration to count all boilerplate properties. Is that better than an extra
field in ObjectLiteral?

[gsathya, 2017-01-17 19:25:57]

On 2017/01/17 19:08:45, Franzi wrote:
> https://codereview.chromium.org/2632503003/diff/140001/src/ast/ast.cc
> File src/ast/ast.cc (right):
> 
>
https://codereview.chromium.org/2632503003/diff/140001/src/ast/ast.cc#newcode627
> src/ast/ast.cc:627: isolate->factory()->NewNumberFromInt(total_properties_);
> On 2017/01/17 18:31:39, gsathya wrote:
> > total_properties_ can be calculated using the above for loop right? then we
> > don't need this to be a property of ObjectLiteral
> 
> The loop breaks on the first computed property, and I'd have to do the rest of
> the iteration to count all boilerplate properties. Is that better than an
extra
> field in ObjectLiteral?

Maybe? Alternatively you can store a HasSeenProto bitfield if you see __proto__
and then do total_properties_ = has_seen_proto? properties.length() - 1:
properties.length(); which reduces the book keeping

[Toon Verwaest, 2017-01-18 08:49:53]

I actually do like that it's passed in like that, given that it's nicely
abstracted away. Overall this CL looks good; but it can be simplified a little
still.

https://codereview.chromium.org/2632503003/diff/140001/src/ast/ast.cc
File src/ast/ast.cc (right):

https://codereview.chromium.org/2632503003/diff/140001/src/ast/ast.cc#newcode587
src/ast/ast.cc:587: Handle<FixedArray> constant_properties =
isolate->factory()->NewFixedArray(
Handle<ConstantProperties> =
isolate->factory()->NewConstantProperties(properties()->length(),
boilerplate_properties_);

which does 

size = boilerplate_properties * 2;
if (properties != boilerplate_properties) size++;
Handle<ConstantProperties> constant_properties =
Handle<ConstantProperties>::cast(NewFixedArray(size));
if (properties != boilerplate_properties) {
  constant_properties->set_property_count(properties);
}
return constant_properties;

https://codereview.chromium.org/2632503003/diff/140001/src/ast/ast.cc#newcode627
src/ast/ast.cc:627: isolate->factory()->NewNumberFromInt(total_properties_);
On 2017/01/17 19:08:45, Franzi wrote:
> On 2017/01/17 18:31:39, gsathya wrote:
> > total_properties_ can be calculated using the above for loop right? then we
> > don't need this to be a property of ObjectLiteral
> 
> The loop breaks on the first computed property, and I'd have to do the rest of
> the iteration to count all boilerplate properties. Is that better than an
extra
> field in ObjectLiteral?

I think what he means is that DCHECK_EQ(total_properties_,
properties()->length()); in which case we don't need a separate
total_properties_ field on the ast node (we do need it in the fixed array
obviously).

https://codereview.chromium.org/2632503003/diff/140001/src/compiler/access-in...
File src/compiler/access-info.cc (right):

https://codereview.chromium.org/2632503003/diff/140001/src/compiler/access-in...
src/compiler/access-info.cc:395: return false;
drop return false

https://codereview.chromium.org/2632503003/diff/140001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2632503003/diff/140001/src/objects.cc#newcode...
src/objects.cc:9820: return FixedArray::length() -
(this->has_number_of_properties() ? 1 : 0);
Perhaps better to not call this length; it's a little confusing this overlaps
with FixedArray::length.

https://codereview.chromium.org/2632503003/diff/140001/src/objects.h
File src/objects.h (left):

https://codereview.chromium.org/2632503003/diff/140001/src/objects.h#oldcode6400
src/objects.h:6400: static const int kUnusedPropertyFieldsByte = 3;
why remove this?

https://codereview.chromium.org/2632503003/diff/140001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2632503003/diff/140001/src/objects.h#newcode2926
src/objects.h:2926: // the FixedArray. ConstantProperties simplifies access to
this number.
What about also adding name(i) and value(i) which just does get(i*2) and
get(i*2+1)?

Perhaps ConstantProperties isn't exactly the best name since their values aren't
constant... What about just BoilerplateDescription or so?

https://codereview.chromium.org/2632503003/diff/140001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2632503003/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2550: number_of_total_properties++;
So this isn't needed given we do properties->Add anyway.

[Toon Verwaest, 2017-01-18 09:50:54]

So yes, what sathya says :)

[Franzi, 2017-01-18 13:27:57]

Patchset #9 (id:160001) has been deleted

[Franzi, 2017-01-18 13:31:00]

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

[commit-bot: I haz the power, 2017-01-18 13:31:04]

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, 2017-01-18 13:32:24]

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

[commit-bot: I haz the power, 2017-01-18 13:32:25]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)

[Franzi, 2017-01-18 14:00:40]

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

[commit-bot: I haz the power, 2017-01-18 14:00:45]

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

[Franzi, 2017-01-18 14:06:02]

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

[commit-bot: I haz the power, 2017-01-18 14:06:11]

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

[Franzi, 2017-01-18 14:23:11]

https://codereview.chromium.org/2632503003/diff/140001/src/ast/ast.cc
File src/ast/ast.cc (right):

https://codereview.chromium.org/2632503003/diff/140001/src/ast/ast.cc#newcode587
src/ast/ast.cc:587: Handle<FixedArray> constant_properties =
isolate->factory()->NewFixedArray(
On 2017/01/18 08:49:53, Toon Verwaest wrote:
> Handle<ConstantProperties> =
> isolate->factory()->NewConstantProperties(properties()->length(),
> boilerplate_properties_);
> 
> which does 
> 
> size = boilerplate_properties * 2;
> if (properties != boilerplate_properties) size++;
> Handle<ConstantProperties> constant_properties =
> Handle<ConstantProperties>::cast(NewFixedArray(size));
> if (properties != boilerplate_properties) {
>   constant_properties->set_property_count(properties);
> }
> return constant_properties;

Acknowledged.

https://codereview.chromium.org/2632503003/diff/140001/src/ast/ast.cc#newcode627
src/ast/ast.cc:627: isolate->factory()->NewNumberFromInt(total_properties_);
On 2017/01/18 08:49:53, Toon Verwaest wrote:
> On 2017/01/17 19:08:45, Franzi wrote:
> > On 2017/01/17 18:31:39, gsathya wrote:
> > > total_properties_ can be calculated using the above for loop right? then
we
> > > don't need this to be a property of ObjectLiteral
> > 
> > The loop breaks on the first computed property, and I'd have to do the rest
of
> > the iteration to count all boilerplate properties. Is that better than an
> extra
> > field in ObjectLiteral?
> 
> I think what he means is that DCHECK_EQ(total_properties_,
> properties()->length()); in which case we don't need a separate
> total_properties_ field on the ast node (we do need it in the fixed array
> obviously).

As discussed, it's not equal if __proto__ is present. I added a bit for
has_seen_proto as Sathya suggested. Dropped the total_properties.

https://codereview.chromium.org/2632503003/diff/140001/src/compiler/access-in...
File src/compiler/access-info.cc (right):

https://codereview.chromium.org/2632503003/diff/140001/src/compiler/access-in...
src/compiler/access-info.cc:395: return false;
On 2017/01/18 08:49:53, Toon Verwaest wrote:
> drop return false

Done.

https://codereview.chromium.org/2632503003/diff/140001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2632503003/diff/140001/src/objects.cc#newcode...
src/objects.cc:9820: return FixedArray::length() -
(this->has_number_of_properties() ? 1 : 0);
On 2017/01/18 08:49:53, Toon Verwaest wrote:
> Perhaps better to not call this length; it's a little confusing this overlaps
> with FixedArray::length.

Done. Using size() which is intuitive for lists and not conflicting with the
underlying length.

https://codereview.chromium.org/2632503003/diff/140001/src/objects.h
File src/objects.h (left):

https://codereview.chromium.org/2632503003/diff/140001/src/objects.h#oldcode6400
src/objects.h:6400: static const int kUnusedPropertyFieldsByte = 3;
On 2017/01/18 08:49:53, Toon Verwaest wrote:
> why remove this?

Noticed that it's unused. Any reason we need to keep it?

https://codereview.chromium.org/2632503003/diff/140001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2632503003/diff/140001/src/objects.h#newcode2926
src/objects.h:2926: // the FixedArray. ConstantProperties simplifies access to
this number.
On 2017/01/18 08:49:53, Toon Verwaest wrote:
> What about also adding name(i) and value(i) which just does get(i*2) and
> get(i*2+1)?
> 
> Perhaps ConstantProperties isn't exactly the best name since their values
aren't
> constant... What about just BoilerplateDescription or so?

name() and value() looks nice. Thanks! Changed the name as well.

https://codereview.chromium.org/2632503003/diff/140001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2632503003/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2550: number_of_total_properties++;
On 2017/01/18 08:49:53, Toon Verwaest wrote:
> So this isn't needed given we do properties->Add anyway.

Deleted.

[commit-bot: I haz the power, 2017-01-18 14:41:44]

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

[commit-bot: I haz the power, 2017-01-18 14:41:45]

Dry run: This issue passed the CQ dry run.

[gsathya, 2017-01-18 17:48:18]

nice!

https://codereview.chromium.org/2632503003/diff/220001/src/runtime/runtime-li...
File src/runtime/runtime-literals.cc (right):

https://codereview.chromium.org/2632503003/diff/220001/src/runtime/runtime-li...
src/runtime/runtime-literals.cc:21: int number_of_properties =
Just something to keep in mind, this is going to be incorrect with object
spread:

var y = { ...{a: 1, b: 1, c: 1}} is going to have number_of_properties = 1

[Franzi, 2017-01-19 17:33:44]

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

[commit-bot: I haz the power, 2017-01-19 17:33:51]

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, 2017-01-19 18:18:36]

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

[commit-bot: I haz the power, 2017-01-19 18:18:37]

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

[Franzi, 2017-01-19 20:57:16]

Patchset #12 (id:240001) has been deleted

[Franzi, 2017-01-19 21:12:27]

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

[commit-bot: I haz the power, 2017-01-19 21:12:30]

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, 2017-01-19 21:48:35]

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

[commit-bot: I haz the power, 2017-01-19 21:48:36]

Dry run: This issue passed the CQ dry run.

[Franzi, 2017-01-19 22:41:14]

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

[commit-bot: I haz the power, 2017-01-19 22:41:20]

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

[Franzi, 2017-01-19 22:52:49]

Hi Toon, 

I changed the BoilerplateDescription. Now it doesn't contain the backing store
size, if it can be computed from the array length. And I added
factory()->NewBoilerplateDescription(). 

I'll make a separate CL for not counting elements instead of decrementing. 

PTAL. Thanks, 

Franzi

[commit-bot: I haz the power, 2017-01-19 23:18:11]

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

[commit-bot: I haz the power, 2017-01-19 23:18:12]

Dry run: This issue passed the CQ dry run.

[Franzi, 2017-01-20 14:10:01]

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

[Franzi, 2017-01-20 14:26:13]

Description was changed from

==========
[parser] Allocate space for computed property names.

Allocate space in the backing store for computed property names. 

BUG=v8:5625
==========

to

==========
[ast] Allocate space for computed property names.

Allocate space in the backing store for computed property names. 

BUG=v8:5625
==========

[Franzi, 2017-01-20 14:27:20]

Description was changed from

==========
[ast] Allocate space for computed property names.

Allocate space in the backing store for computed property names. 

BUG=v8:5625
==========

to

==========
[runtime] Allocate space for computed property names.

Allocate space in the backing store for computed property names. 

BUG=v8:5625
==========

[marja, 2017-01-20 14:38:44]

ast/* & parsing/* LGTM, but you'll still need owners for the other parts

[Franzi, 2017-01-20 14:48:30]

Description was changed from

==========
[runtime] Allocate space for computed property names.

Allocate space in the backing store for computed property names. 

BUG=v8:5625
==========

to

==========
[runtime] Allocate space for computed property names.

Allocate space in the backing store for computed property names. 

The property backing store was pre-allocated for the constant 
properties up to the first non-constant (computed name) property.
To use lowering for storing data properties in literals 
with computed property names effectively, a fast store is needed, i.e., 
available space in the property backing store for properties 
with computed names. 

backing_store_size is the number of all properties (including 
computed names, but without __proto__)
that is calculated in the ast and passed to the runtime function that allocates
the property backing store. backing_store_size and 
constant_properties constitute a BoilerplateDescription.

backing_store_size might be slightly too high because computed names 
can evaluate to the same name, but that should be a rare
case so over-allocating is OK. 

If a property is __proto__, we don't store it as a regular 
property, because the map changes. Keep track of
has_seen_proto in the parser to calculate the
backing store size correctly. 

BUG=v8:5625
==========

[Franzi, 2017-01-20 14:48:46]

The CQ bit was checked by franzih@chromium.org

[commit-bot: I haz the power, 2017-01-20 14:48:53]

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, 2017-01-20 14:52:23]

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

[commit-bot: I haz the power, 2017-01-20 14:52:24]

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

[Franzi, 2017-01-20 14:56:17]

franzih@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[Franzi, 2017-01-20 14:56:18]

Hi Michael, 

Can you review the compiler changes in this CL for me? 

Thanks, 

Franzi

[Benedikt Meurer, 2017-01-20 18:12:52]

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

[Benedikt Meurer, 2017-01-20 18:12:53]

LGTM on compiler.

[Franzi, 2017-01-20 18:39:04]

The CQ bit was checked by franzih@chromium.org

[commit-bot: I haz the power, 2017-01-20 18:39:15]

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, 2017-01-20 18:47:56]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1484937544423290,
"parent_rev": "24c050e8bc7f6e954a0de532a4cc2872d121ceb5", "commit_rev":
"399f36b5185deafa263def840d8f98bb61ff3ab4"}

[commit-bot: I haz the power, 2017-01-20 18:48:02]

Description was changed from

==========
[runtime] Allocate space for computed property names.

Allocate space in the backing store for computed property names. 

The property backing store was pre-allocated for the constant 
properties up to the first non-constant (computed name) property.
To use lowering for storing data properties in literals 
with computed property names effectively, a fast store is needed, i.e., 
available space in the property backing store for properties 
with computed names. 

backing_store_size is the number of all properties (including 
computed names, but without __proto__)
that is calculated in the ast and passed to the runtime function that allocates
the property backing store. backing_store_size and 
constant_properties constitute a BoilerplateDescription.

backing_store_size might be slightly too high because computed names 
can evaluate to the same name, but that should be a rare
case so over-allocating is OK. 

If a property is __proto__, we don't store it as a regular 
property, because the map changes. Keep track of
has_seen_proto in the parser to calculate the
backing store size correctly. 

BUG=v8:5625
==========

to

==========
[runtime] Allocate space for computed property names.

Allocate space in the backing store for computed property names. 

The property backing store was pre-allocated for the constant 
properties up to the first non-constant (computed name) property.
To use lowering for storing data properties in literals 
with computed property names effectively, a fast store is needed, i.e., 
available space in the property backing store for properties 
with computed names. 

backing_store_size is the number of all properties (including 
computed names, but without __proto__)
that is calculated in the ast and passed to the runtime function that allocates
the property backing store. backing_store_size and 
constant_properties constitute a BoilerplateDescription.

backing_store_size might be slightly too high because computed names 
can evaluate to the same name, but that should be a rare
case so over-allocating is OK. 

If a property is __proto__, we don't store it as a regular 
property, because the map changes. Keep track of
has_seen_proto in the parser to calculate the
backing store size correctly. 

BUG=v8:5625

Review-Url: https://codereview.chromium.org/2632503003
Cr-Commit-Position: refs/heads/master@{#42576}
Committed:
https://chromium.googlesource.com/v8/v8/+/399f36b5185deafa263def840d8f98bb61f...
==========

[commit-bot: I haz the power, 2017-01-20 18:48:03]

Committed patchset #13 (id:280001) as
https://chromium.googlesource.com/v8/v8/+/399f36b5185deafa263def840d8f98bb61f...