Simplify feedback vector creation and store in SharedFunctionInfo.

Simplify feedback vector creation and store in SharedFunctionInfo.

LOG=N
BUG=v8:3212
R=bmeurer@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=21085

[mvstanton, 2014-04-24 14:19:06]

Hi Benedikt,
You've already reviewed much of this CL, let's get together and I'll describe
the parts that are slightly different.
Thanks,
--Michael

[Benedikt Meurer, 2014-04-28 19:17:00]

Hey Michael,
Nice work! First round of comments.
-- Benedikt

https://codereview.chromium.org/254623002/diff/60001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/254623002/diff/60001/src/compiler.cc#newcode263
src/compiler.cc:263: feedback_vector_ =
isolate()->factory()->NewFixedArray(length, TENURED);
How about adding a factory wrapper for Heap::AllocateFixedArrayWithFiller() and
call that wrapper, i.e. NewFixedArrayWithFiller(length, TENURED, sentinel)
instead of allocating a FixedArray filled with undefined and then overwriting
undefined with sentinel?

https://codereview.chromium.org/254623002/diff/60001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/254623002/diff/60001/src/objects.cc#newcode11264
src/objects.cc:11264: void SharedFunctionInfo::ClearTypeFeedbackInfo(Heap* heap)
{
Nit: Please remove the useless Heap parameter and use GetHeap() instead.

https://codereview.chromium.org/254623002/diff/60001/src/type-info.cc
File src/type-info.cc (right):

https://codereview.chromium.org/254623002/diff/60001/src/type-info.cc#newcode46
src/type-info.cc:46: Handle<FixedArray> feedback_vector,
As discussed offline: You'll probably need to duplicate the feedback vector
here, as a GC can clear the type feedback while you are trying to use it. And
add a comment that this can be removed later, once our crappy feedback clearing
during GC is history.

https://codereview.chromium.org/254623002/diff/60001/src/typing.cc
File src/typing.cc (right):

https://codereview.chromium.org/254623002/diff/60001/src/typing.cc#newcode44
src/typing.cc:44: Handle<Context>(info->closure()->context()->native_context()),
Nit: Use handle() instead of Handle<T>()

https://codereview.chromium.org/254623002/diff/60001/test/cctest/test-compile...
File test/cctest/test-compiler.cc (right):

https://codereview.chromium.org/254623002/diff/60001/test/cctest/test-compile...
test/cctest/test-compiler.cc:323:
CHECK(f->shared()->has_deoptimization_support());
Nit: Please also CHECK() that f is actually optimized at this point.

[mvstanton, 2014-04-30 08:31:18]

Hi Benedikt, thanks for the comments.
I've addressed, and also checking into performance of the array copy (I think it
should be okay though, since previously we used to copy the type cells into a
dictionary).
--Michael

https://codereview.chromium.org/254623002/diff/60001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/254623002/diff/60001/src/compiler.cc#newcode263
src/compiler.cc:263: feedback_vector_ =
isolate()->factory()->NewFixedArray(length, TENURED);
On 2014/04/28 19:17:00, Benedikt Meurer wrote:
> How about adding a factory wrapper for Heap::AllocateFixedArrayWithFiller()
and
> call that wrapper, i.e. NewFixedArrayWithFiller(length, TENURED, sentinel)
> instead of allocating a FixedArray filled with undefined and then overwriting
> undefined with sentinel?

Great idea, done, but I went ahead and made the factory method called
NewTypeFeedbackVector(count), which hides the TENURED information as well as the
sentinel used. A future CL already needs that because the feedback vector will
be allocatable in one other place.

https://codereview.chromium.org/254623002/diff/60001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/254623002/diff/60001/src/objects.cc#newcode11264
src/objects.cc:11264: void SharedFunctionInfo::ClearTypeFeedbackInfo(Heap* heap)
{
On 2014/04/28 19:17:00, Benedikt Meurer wrote:
> Nit: Please remove the useless Heap parameter and use GetHeap() instead.

Done.

https://codereview.chromium.org/254623002/diff/60001/src/typing.cc
File src/typing.cc (right):

https://codereview.chromium.org/254623002/diff/60001/src/typing.cc#newcode43
src/typing.cc:43:
Handle<FixedArray>(info->closure()->shared()->feedback_vector()),
Warning: you aren't copying the vector, what if a gc happens? Strive to reduce
uncertainty.

https://codereview.chromium.org/254623002/diff/60001/src/typing.cc#newcode44
src/typing.cc:44: Handle<Context>(info->closure()->context()->native_context()),
On 2014/04/28 19:17:00, Benedikt Meurer wrote:
> Nit: Use handle() instead of Handle<T>()

Done.

https://codereview.chromium.org/254623002/diff/60001/test/cctest/test-compile...
File test/cctest/test-compiler.cc (right):

https://codereview.chromium.org/254623002/diff/60001/test/cctest/test-compile...
test/cctest/test-compiler.cc:323:
CHECK(f->shared()->has_deoptimization_support());
On 2014/04/28 19:17:00, Benedikt Meurer wrote:
> Nit: Please also CHECK() that f is actually optimized at this point.

Done.

[Benedikt Meurer, 2014-04-30 08:35:29]

LGTM, let's see how it works in the wild! :-)

[mvstanton, 2014-04-30 10:51:21]

Committed patchset #3 manually as r21085 (presubmit successful).