Addressed comments, waiting for Vitaly's LGTM.

http://codereview.chromium.org/3329019/diff/15001/16007
File src/ia32/builtins-ia32.cc (right):

http://codereview.chromium.org/3329019/diff/15001/16007#newcode193
src/ia32/builtins-ia32.cc:193: __ mov(edx,
Factory::one_pointer_filler_map());
Is there a difference? An Assert at the code generation phase is
guaranteed to prevent generating the code for the wrong set of
arguments.

On 2010/09/21 16:49:00, antonm wrote:
On 2010/09/21 12:32:41, Vladislav Kaznacheev wrote:
> Added an assert at the top of the function on all 3 platforms.
> On 2010/09/21 11:48:55, antonm wrote:
> > we should never come here for API functions, correct?  could we
add Assert
> here?
>
>

I meant an Assert compiled into the assembly code, but if it's too
cumbersome,
that's fine.


http://codereview.chromium.org/3329019/diff/15001/16009
File src/objects.cc (right):

http://codereview.chromium.org/3329019/diff/15001/16009#newcode5503
src/objects.cc:5503: void
SharedFunctionInfo::CompleteInobjectSlackTracking() {
Clearing and restoring the flag is all done in GC, so to JS code can be
executed in between.

If the context that started the tracking goes away (e.g. with
navigation) then the next context that tries to create the object will
restart tracking _continuing the countdown from the previous value_.
Regardless of how many times this happens in 16 constructions the
Complete... method will fire.

I have made an Assert at the start of the function stronger.
On 2010/09/21 16:49:00, antonm wrote:
On 2010/09/21 12:32:41, Vladislav Kaznacheev wrote:
> It does not mean that. StartInobjectSlackTracking uses a flag
> (kHasConstructedObjects) to make sure the tracking is not started
more than
once
> (unless we want it to in the case  when the tracking is cancelled by
the GC).
>

Yes, I am concerned that the very moment GC would clean this flag, the
code
would run again.  I don't know if it's a problem or not.  My main
concern is web
browser scenario, say, some navigation that makes the context go away
and now
other contexts go into slower path again.

But it all speculations, feel free to leave it as is.

> On 2010/09/21 11:48:55, antonm wrote:
> > does that mean that after CompleteInobjectSlackTracking is over,
another
> context
> > might initiate the process once again?
> >
> > If yes, do we want it?
>
>



http://codereview.chromium.org/3329019/diff/26002/28008
File src/mark-compact.cc (right):

http://codereview.chromium.org/3329019/diff/26002/28008#newcode1152
src/mark-compact.cc:1152: (map->bit_field2() & (1 <<
Map::kRelinkToSharedFunctionInfo))) {
On 2010/09/21 16:49:00, antonm wrote:
maybe you should introduce functions like
set_relink_to_shared_function_info to
Map class (cf. set_has_named_interceptor and alike)

Done.

http://codereview.chromium.org/3329019/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to