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

Issue 7058009: Make InToSpace/InFromSpace use the page header. (Closed)

Created:
9 years, 7 months ago by Lasse Reichstein
Modified:
9 years, 6 months ago
CC:
v8-dev
Visibility:
Public.

Description

Make InToSpace/InFromSpace use the page header. Committed: http://code.google.com/p/v8/source/detail?r=8033

Patch Set 1 #

Total comments: 3

Patch Set 2 : Rewrote from scratch. Please rereview. #

Total comments: 11

Patch Set 3 : Have separate bits for from and to-space #

Patch Set 4 : Addressed review comments. #

Total comments: 1

Patch Set 5 : Used Clear/Set macros more in SemiSpace::Flip #

Unified diffs Side-by-side diffs Delta from patch set Stats (+158 lines, -66 lines) Patch
M src/heap-inl.h View 1 1 chunk +1 line, -1 line 0 comments Download
M src/ia32/code-stubs-ia32.cc View 1 2 3 4 3 chunks +4 lines, -9 lines 0 comments Download
M src/ia32/macro-assembler-ia32.h View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
M src/ia32/macro-assembler-ia32.cc View 1 2 3 4 1 chunk +25 lines, -0 lines 0 comments Download
M src/spaces.h View 1 2 3 4 12 chunks +88 lines, -50 lines 0 comments Download
M src/spaces.cc View 1 2 3 4 4 chunks +33 lines, -6 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Lasse Reichstein
9 years, 7 months ago (2011-05-23 11:49:51 UTC) #1
Erik Corry
http://codereview.chromium.org/7058009/diff/1/src/spaces.h File src/spaces.h (right): http://codereview.chromium.org/7058009/diff/1/src/spaces.h#newcode1472 src/spaces.h:1472: void CopyFlagsFrom(NewSpacePage* chunk) { This is called CopyFlagsFrom, but ...
9 years, 7 months ago (2011-05-23 12:58:56 UTC) #2
Lasse Reichstein
Please re-review. http://codereview.chromium.org/7058009/diff/1/src/spaces.h File src/spaces.h (right): http://codereview.chromium.org/7058009/diff/1/src/spaces.h#newcode1560 src/spaces.h:1560: if (o->IsSmi()) { Will do. That also ...
9 years, 7 months ago (2011-05-24 10:53:41 UTC) #3
Lasse Reichstein
http://codereview.chromium.org/7058009/diff/3001/src/spaces.cc File src/spaces.cc (right): http://codereview.chromium.org/7058009/diff/3001/src/spaces.cc#newcode949 src/spaces.cc:949: (1 << MemoryChunk::SCAN_ON_SCAVENGE); Vyacheslav, is this the correct flags ...
9 years, 7 months ago (2011-05-24 10:54:57 UTC) #4
Vyacheslav Egorov (Chromium)
http://codereview.chromium.org/7058009/diff/3001/src/spaces.cc File src/spaces.cc (right): http://codereview.chromium.org/7058009/diff/3001/src/spaces.cc#newcode949 src/spaces.cc:949: (1 << MemoryChunk::SCAN_ON_SCAVENGE); On 2011/05/24 10:54:57, Lasse Reichstein wrote: ...
9 years, 7 months ago (2011-05-24 10:58:47 UTC) #5
Erik Corry
http://codereview.chromium.org/7058009/diff/3001/src/spaces.cc File src/spaces.cc (right): http://codereview.chromium.org/7058009/diff/3001/src/spaces.cc#newcode1154 src/spaces.cc:1154: is_to_space_ = !is_to_space_; This can be written in terms ...
9 years, 7 months ago (2011-05-24 11:25:49 UTC) #6
Lasse Reichstein
http://codereview.chromium.org/7058009/diff/3001/src/spaces.cc File src/spaces.cc (right): http://codereview.chromium.org/7058009/diff/3001/src/spaces.cc#newcode949 src/spaces.cc:949: (1 << MemoryChunk::SCAN_ON_SCAVENGE); Reduced and added as constant in ...
9 years, 7 months ago (2011-05-24 12:29:03 UTC) #7
Erik Corry
9 years, 7 months ago (2011-05-24 12:38:36 UTC) #8
LGTM

http://codereview.chromium.org/7058009/diff/4003/src/spaces.cc
File src/spaces.cc (right):

http://codereview.chromium.org/7058009/diff/4003/src/spaces.cc#newcode1149
src/spaces.cc:1149: id_ = becomes_to_space ? kToSpace : kFromSpace;
This can all be rewritten in terms of SetFlag and ClearFlag on each page.

Powered by Google App Engine
This is Rietveld 408576698