Optimize ClearNonLiveReferences: do not compact prototype transitions in GC.

Optimize ClearNonLiveReferences: do not compact prototype transitions in GC.

BUG=chromium:554488
LOG=NO

Committed: https://crrev.com/7a21cdafaf665e05700e84de1081b5479c69342a
Cr-Commit-Position: refs/heads/master@{#32272}

[ulan, 2015-11-24 15:47:04]

ulan@chromium.org changed reviewers:
+ jkummerow@chromium.org

[ulan, 2015-11-24 15:47:04]

PTAL

https://codereview.chromium.org/1470773003/diff/20001/src/transitions.cc
File src/transitions.cc (left):

https://codereview.chromium.org/1470773003/diff/20001/src/transitions.cc#oldc...
src/transitions.cc:404: if (Heap::ShouldZapGarbage()) {
We do not need to zap prototype transition array because it uses weak cells
instead raw pointers.

[ulan, 2015-11-24 15:47:31]

ulan@chromium.org changed reviewers:
+ mlippautz@chromium.org

[ulan, 2015-11-24 15:47:31]

+Michael for the heap part.

[ulan, 2015-11-24 15:49:05]

https://codereview.chromium.org/1470773003/diff/20001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (left):

https://codereview.chromium.org/1470773003/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:2308: FixedArray* prototype_transitions =
This code is moved to runtime.

[Jakob Kummerow, 2015-11-25 12:38:05]

LGTM with a refactoring suggestion.

https://codereview.chromium.org/1470773003/diff/40001/src/transitions.h
File src/transitions.h (right):

https://codereview.chromium.org/1470773003/diff/40001/src/transitions.h#newco...
src/transitions.h:105: static bool CompactPrototypeTransitionArray(FixedArray*
array);
AFAICS both of these new functions are internal except for testing. I'd like to
keep the TransitionArray's public interface as lean as possible, so here's my
suggestion how to structure the code:

transitions.h:

static int NumberOfPrototypeTransitionsForTest(Map* map);

transitions.cc:

namespace {

bool CompactPrototypeTransitionArray(...) { ... }
Handle<FixedArray> GrowPrototypeTransitionArray(...) { ... }

}  // namespace

static bool NumberOfPrototypeTransitionsForTest(Map* map) {
  FixedArray* transitions = TransitionArray::GetPrototypeTransitions(map);
  CompactPrototypeTransitionArray(transitions);
  return TransitionArray::NumberOfPrototypeTransitions(transitions);
}

// Two more call sites from PutPrototypeTransition.

test-heap.cc: 

Drop "static int NumberOfProtoTransitions(Map* map)", call
NumberOfPrototypeTransitionsForTest directly.

[ulan, 2015-11-25 13:12:48]

Thanks for review.

https://codereview.chromium.org/1470773003/diff/40001/src/transitions.h
File src/transitions.h (right):

https://codereview.chromium.org/1470773003/diff/40001/src/transitions.h#newco...
src/transitions.h:105: static bool CompactPrototypeTransitionArray(FixedArray*
array);
On 2015/11/25 12:38:05, Jakob wrote:
> AFAICS both of these new functions are internal except for testing. I'd like
to
> keep the TransitionArray's public interface as lean as possible, so here's my
> suggestion how to structure the code:
> 
> transitions.h:
> 
> static int NumberOfPrototypeTransitionsForTest(Map* map);
> 
> transitions.cc:
> 
> namespace {
> 
> bool CompactPrototypeTransitionArray(...) { ... }
> Handle<FixedArray> GrowPrototypeTransitionArray(...) { ... }
> 
> }  // namespace
> 
> static bool NumberOfPrototypeTransitionsForTest(Map* map) {
>   FixedArray* transitions = TransitionArray::GetPrototypeTransitions(map);
>   CompactPrototypeTransitionArray(transitions);
>   return TransitionArray::NumberOfPrototypeTransitions(transitions);
> }
> 
> // Two more call sites from PutPrototypeTransition.
> 
> test-heap.cc: 
> 
> Drop "static int NumberOfProtoTransitions(Map* map)", call
> NumberOfPrototypeTransitionsForTest directly.

Done.

[ulan, 2015-11-25 13:13:09]

The CQ bit was checked by ulan@chromium.org

[ulan, 2015-11-25 13:13:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1470773003/#ps60001
(title: "Address comments")

[commit-bot: I haz the power, 2015-11-25 13:13:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1470773003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1470773003/60001

[commit-bot: I haz the power, 2015-11-25 13:38:59]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-25 13:39:19]

Patchset 4 (id:??) landed as
https://crrev.com/7a21cdafaf665e05700e84de1081b5479c69342a
Cr-Commit-Position: refs/heads/master@{#32272}