Hi Toon,
Here are my comments from last week after your final review, along with new
changes for performance reasons.
The main changes are
1) Breaking up heap.cc functions into those that work with
AllocationSiteINfo
and those that don't. I had performance issues in hot functions because of
introducing extra calls or adding parameters. So as much as possible, I made
separate functions.
2) Growing the builtins table does seem to hurt. For my following checkin I
don't need to grow it. Rather than plug in an ugly workaround I simply
leave the
builtins table alone, and added a CHECK() that
FLAG_optimize_constructed_arrays
is off. To turn that flag on, we have to have a larger builtins table. I'd
rather just deal with that in my next checkin.
3) Some code-sharing/refactoring ideas had to be left out because of the
cost in
benchmarks. I do believe that these can be dealt with better in the
following
checkin when performance gains due to hydrogen stubs will make it possible
to
carry out these refactorings without regressions. I will include in that
checkin
elimination of duplicate code due to the usual refactoring steps.
Thanks,
--Michael
ps - as I worked on performance I rebased several times, so I think I
screwed
the pooch on the inter-revision view, sorry! :(
https://chromiumcodereview.appspot.com/11818021/diff/34001/src/objects.h
File src/objects.h (right):
https://chromiumcodereview.appspot.com/11818021/diff/34001/src/objects.h#newcode6986
src/objects.h:6986:
On 2013/02/21 12:13:03, Toon Verwaest wrote:
... remove.
On 2013/02/13 15:14:51, Toon Verwaest wrote:
> Remove.
Done.
https://chromiumcodereview.appspot.com/11818021/diff/52004/src/heap.cc
File src/heap.cc (right):
https://chromiumcodereview.appspot.com/11818021/diff/52004/src/heap.cc#newcode4255
src/heap.cc:4255: // mode = DONT_TRACK_ALLOCATION_SITE;
On 2013/02/21 12:13:03, Toon Verwaest wrote:
Use GetMode(to_kind); here to decide whether or not to track
ALLOCATION_SITE.
Done.
https://chromiumcodereview.appspot.com/11818021/diff/52004/src/objects.cc
File src/objects.cc (right):
https://chromiumcodereview.appspot.com/11818021/diff/52004/src/objects.cc#newcode10603
src/objects.cc:10603: return NULL;
On 2013/02/21 12:13:03, Toon Verwaest wrote:
return this;
Done.
https://chromiumcodereview.appspot.com/11818021/diff/52004/src/objects.cc#newcode10608
src/objects.cc:10608: return NULL;
On 2013/02/21 12:13:03, Toon Verwaest wrote:
return this;
Done.
https://chromiumcodereview.appspot.com/11818021/diff/52004/src/objects.cc#newcode10662
src/objects.cc:10662: if (trans->IsFailure()) return trans;
On 2013/02/21 12:13:03, Toon Verwaest wrote:
MaybeObject* maybe_failure = ...;
if (maybe_failure->IsFailure()) return maybe_failure;
Done.
https://chromiumcodereview.appspot.com/11818021/
--
--
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/groups/opt_out.
