Analyze functions for assignment to this properties

Analyze functions for assignment to this properties.

During parsing functions are analyzed for statements of the form this.x = ...;. These assignments are categorized in two types: simple and non simple. The simple ones are where the right hand side is known to be either a constant or an argument to the function. If a function only contains statements of this type the property names are collected and for the simple assignments the index of the argument or the constant value assigned are stored as well.

When the initial map for a function is created and the function consists of only this type of assignemnts the initial map is created with a descriptor array describing these properties which will be known to always exist in an object created from the function.

The information on this property assignments is not collected during pre-parsing so if compiling using pre-parse data these optimization hints are not available.

Next step will be to use the information collected for the simple assignments to generate constructor code which will create and initialize the object from this information without calling the code for the function.

Committed: http://code.google.com/p/v8/source/detail?r=2710

[Mads Ager (chromium), 2009-08-18 12:36:23]

LGTM

http://codereview.chromium.org/172088/diff/1003/42
File src/ast.h (right):

http://codereview.chromium.org/172088/diff/1003/42#newcode1243
Line 1243: has_only_simple_this_property_assignments_(
Shouldn't you initialize |has_only_this_property_assignments| as well?

http://codereview.chromium.org/172088/diff/1003/45
File src/codegen.cc (right):

http://codereview.chromium.org/172088/diff/1003/45#newcode259
Line 259: lit->has_only_this_property_assignments(),
Four space indent.

http://codereview.chromium.org/172088/diff/1003/39
File src/heap.cc (right):

http://codereview.chromium.org/172088/diff/1003/39#newcode2129
Line 2129: - map->inobject_properties();
I think we normally place the operators on the previous line and we shouldn't we
use a four space indent?

http://codereview.chromium.org/172088/diff/1003/41
File src/ia32/builtins-ia32.cc (right):

http://codereview.chromium.org/172088/diff/1003/41#newcode183
Line 183: // Calculate total properties described map.
Calculate total number of properties described by map?

http://codereview.chromium.org/172088/diff/1003/41#newcode192
Line 192: #ifdef DEBUG
Should this just be __ Assert(positive, "...")"?  The Assert method in the macro
assembler only generates code if running with the --debug-code flag.

http://codereview.chromium.org/172088/diff/1003/40
File src/objects.cc (right):

http://codereview.chromium.org/172088/diff/1003/40#newcode4785
Line 4785: expected_nof_properties() * kPointerSize;
Shouldn't this be a four space indent?

http://codereview.chromium.org/172088/diff/1003/40#newcode4804
Line 4804: kHasOnlyThisPropertyAssignments,
Indentation is off here.  This is an argument to set and not to
set_compiler_hints.

http://codereview.chromium.org/172088/diff/1003/44
File src/objects.h (right):

http://codereview.chromium.org/172088/diff/1003/44#newcode2892
Line 2892: // The bytes at positions 2 and 3 are not in use at the moment.
Update comment.

http://codereview.chromium.org/172088/diff/1003/44#newcode3116
Line 3116: // this.x = y; where y is either a constant or refers to am argument.
am -> an

http://codereview.chromium.org/172088/diff/1003/50
File src/parser.cc (right):

http://codereview.chromium.org/172088/diff/1003/50#newcode1462
Line 1462: // A ThisNamedPropertyAssigmentFinder finds and marks statements if
the form
if the form -> of the form.

http://codereview.chromium.org/172088/diff/1003/50#newcode1463
Line 1463: // this.x = ...;, where x is a named property. It also findes whether
a
findes -> determines

http://codereview.chromium.org/172088/diff/1003/50#newcode1475
Line 1475: // Bail out if function already have had non this property assignment
have had -> has

http://codereview.chromium.org/172088/diff/1003/50#newcode1491
Line 1491: // Returns whether only statements of the form thsi.x = ...; was
encountered.
thsi -> this

http://codereview.chromium.org/172088/diff/1003/50#newcode1496
Line 1496: // Returns whether only statements of the form thsi.x = y; where y is
either a
thsi -> this

http://codereview.chromium.org/172088/diff/1003/50#newcode1579
Line 1579: assigned_constants_->Add(Factory::undefined_value());
Does this mean that 'this.x = undefined' is not recognized as a constant this
assignment?

http://codereview.chromium.org/172088/diff/1003/50#newcode1652
Line 1652: this_property_assignment_finder.only_this_property_assignments(),
Four space indent.

http://codereview.chromium.org/172088/diff/1003/47
File src/x64/builtins-x64.cc (right):

http://codereview.chromium.org/172088/diff/1003/47#newcode598
Line 598: Label assert, no_assert;
__ Assert?

[Søren Thygesen Gjesse, 2009-08-19 07:26:50]

Sorry for all the spelling mistakes.

http://codereview.chromium.org/172088/diff/1003/42
File src/ast.h (right):

http://codereview.chromium.org/172088/diff/1003/42#newcode1243
Line 1243: has_only_simple_this_property_assignments_(
On 2009/08/18 12:36:23, Mads Ager wrote:
> Shouldn't you initialize |has_only_this_property_assignments| as well?

Done.

http://codereview.chromium.org/172088/diff/1003/45
File src/codegen.cc (right):

http://codereview.chromium.org/172088/diff/1003/45#newcode259
Line 259: lit->has_only_this_property_assignments(),
On 2009/08/18 12:36:23, Mads Ager wrote:
> Four space indent.

Done.

http://codereview.chromium.org/172088/diff/1003/39
File src/heap.cc (right):

http://codereview.chromium.org/172088/diff/1003/39#newcode2129
Line 2129: - map->inobject_properties();
On 2009/08/18 12:36:23, Mads Ager wrote:
> I think we normally place the operators on the previous line and we shouldn't
we
> use a four space indent?

Done.

http://codereview.chromium.org/172088/diff/1003/41
File src/ia32/builtins-ia32.cc (right):

http://codereview.chromium.org/172088/diff/1003/41#newcode183
Line 183: // Calculate total properties described map.
On 2009/08/18 12:36:23, Mads Ager wrote:
> Calculate total number of properties described by map?

Done.

http://codereview.chromium.org/172088/diff/1003/41#newcode192
Line 192: #ifdef DEBUG
On 2009/08/18 12:36:23, Mads Ager wrote:
> Should this just be __ Assert(positive, "...")"?  The Assert method in the
macro
> assembler only generates code if running with the --debug-code flag.

Done.

http://codereview.chromium.org/172088/diff/1003/40
File src/objects.cc (right):

http://codereview.chromium.org/172088/diff/1003/40#newcode4785
Line 4785: expected_nof_properties() * kPointerSize;
On 2009/08/18 12:36:23, Mads Ager wrote:
> Shouldn't this be a four space indent?

Done.

http://codereview.chromium.org/172088/diff/1003/40#newcode4804
Line 4804: kHasOnlyThisPropertyAssignments,
On 2009/08/18 12:36:23, Mads Ager wrote:
> Indentation is off here.  This is an argument to set and not to
> set_compiler_hints.

Done.

http://codereview.chromium.org/172088/diff/1003/44
File src/objects.h (right):

http://codereview.chromium.org/172088/diff/1003/44#newcode2892
Line 2892: // The bytes at positions 2 and 3 are not in use at the moment.
On 2009/08/18 12:36:23, Mads Ager wrote:
> Update comment.

Done.

http://codereview.chromium.org/172088/diff/1003/44#newcode3116
Line 3116: // this.x = y; where y is either a constant or refers to am argument.
On 2009/08/18 12:36:23, Mads Ager wrote:
> am -> an

Done.

http://codereview.chromium.org/172088/diff/1003/50
File src/parser.cc (right):

http://codereview.chromium.org/172088/diff/1003/50#newcode1462
Line 1462: // A ThisNamedPropertyAssigmentFinder finds and marks statements if
the form
On 2009/08/18 12:36:23, Mads Ager wrote:
> if the form -> of the form.

Done.

http://codereview.chromium.org/172088/diff/1003/50#newcode1463
Line 1463: // this.x = ...;, where x is a named property. It also findes whether
a
On 2009/08/18 12:36:23, Mads Ager wrote:
> findes -> determines

Done.

http://codereview.chromium.org/172088/diff/1003/50#newcode1475
Line 1475: // Bail out if function already have had non this property assignment
On 2009/08/18 12:36:23, Mads Ager wrote:
> have had -> has

Done.

http://codereview.chromium.org/172088/diff/1003/50#newcode1491
Line 1491: // Returns whether only statements of the form thsi.x = ...; was
encountered.
On 2009/08/18 12:36:23, Mads Ager wrote:
> thsi -> this

Done.

http://codereview.chromium.org/172088/diff/1003/50#newcode1496
Line 1496: // Returns whether only statements of the form thsi.x = y; where y is
either a
On 2009/08/18 12:36:23, Mads Ager wrote:
> thsi -> this

Done.

http://codereview.chromium.org/172088/diff/1003/50#newcode1579
Line 1579: assigned_constants_->Add(Factory::undefined_value());
On 2009/08/18 12:36:23, Mads Ager wrote:
> Does this mean that 'this.x = undefined' is not recognized as a constant this
> assignment?

Yes, as undefined is a property on the global object and not a literal. The
function f(undefined){this.x=undefined} is recognized as having an argument
assignment - which should be correct. Also f(undefined){this.x=void 0} is not an
assignment from a constant.

http://codereview.chromium.org/172088/diff/1003/50#newcode1652
Line 1652: this_property_assignment_finder.only_this_property_assignments(),
On 2009/08/18 12:36:23, Mads Ager wrote:
> Four space indent.

Done.

http://codereview.chromium.org/172088/diff/1003/47
File src/x64/builtins-x64.cc (right):

http://codereview.chromium.org/172088/diff/1003/47#newcode598
Line 598: Label assert, no_assert;
On 2009/08/18 12:36:23, Mads Ager wrote:
> __ Assert?

Done.

[Søren Thygesen Gjesse, 2009-08-19 07:28:08]

Sorry about all the spelling mistakes.