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-compiler.cc
File test/cctest/test-compiler.cc (right):
https://codereview.chromium.org/254623002/diff/60001/test/cctest/test-compiler.cc#newcode323
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.
https://codereview.chromium.org/254623002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
