Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(19)

Issue 11028016: Move code flushing support into shared visitor. (Closed)

Created:
8 years, 2 months ago by Michael Starzinger
Modified:
8 years, 2 months ago
Reviewers:
ulan
CC:
v8-dev
Visibility:
Public.

Description

Move code flushing support into shared visitor. This is a first step towards incremental code flushing. The code flushing support is now shared between full and incremental marking. The code flusher itself is not yet activated in incremental mode and will require some additional adaptations. R=ulan@chromium.org BUG=v8:1609 Committed: https://code.google.com/p/v8/source/detail?r=12714

Patch Set 1 #

Total comments: 7

Patch Set 2 : Addressed comments by Ulan Degenbaev. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+407 lines, -427 lines) Patch
M src/incremental-marking.cc View 3 chunks +1 line, -33 lines 0 comments Download
M src/mark-compact.h View 2 chunks +75 lines, -4 lines 0 comments Download
M src/mark-compact.cc View 1 7 chunks +52 lines, -388 lines 0 comments Download
M src/objects-visiting.h View 2 chunks +18 lines, -0 lines 0 comments Download
M src/objects-visiting-inl.h View 1 3 chunks +261 lines, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Michael Starzinger
8 years, 2 months ago (2012-10-02 14:34:12 UTC) #1
ulan
LGTM with nits. https://codereview.chromium.org/11028016/diff/1/src/mark-compact.cc File src/mark-compact.cc (left): https://codereview.chromium.org/11028016/diff/1/src/mark-compact.cc#oldcode1377 src/mark-compact.cc:1377: This line seems to be accidentally ...
8 years, 2 months ago (2012-10-12 11:48:32 UTC) #2
Michael Starzinger
8 years, 2 months ago (2012-10-12 12:16:37 UTC) #3
Addressed comments. Thanks for the review. Landing.

https://codereview.chromium.org/11028016/diff/1/src/mark-compact.cc
File src/mark-compact.cc (left):

https://codereview.chromium.org/11028016/diff/1/src/mark-compact.cc#oldcode1377
src/mark-compact.cc:1377: 
On 2012/10/12 11:48:32, ulan wrote:
> This line seems to be accidentally removed. 

Done.

https://codereview.chromium.org/11028016/diff/1/src/objects-visiting-inl.h
File src/objects-visiting-inl.h (right):

https://codereview.chromium.org/11028016/diff/1/src/objects-visiting-inl.h#ne...
src/objects-visiting-inl.h:559: // Skip visiting kCodeOffset as it is treated
weakly here.
On 2012/10/12 11:48:32, ulan wrote:
> Maybe add static asserts that kNameOffset + kPointerSize == kCodeOffset and
> kCodeOffset + kPointerSize == kOptimizesCodeMapOffset?

Done.

https://codereview.chromium.org/11028016/diff/1/src/objects-visiting-inl.h#ne...
src/objects-visiting-inl.h:580: VisitCodeEntry(heap, object->address() +
JSFunction::kCodeEntryOffset);
As discussed offline, I also added the static assert here.

https://codereview.chromium.org/11028016/diff/1/src/objects-visiting-inl.h#ne...
src/objects-visiting-inl.h:599: // Skip visiting kCodeEntryOffset as it is
treated weakly here.
On 2012/10/12 11:48:32, ulan wrote:
> Maybe add static asserts as in the comment above?

Done.

Powered by Google App Engine
This is Rietveld 408576698