Issue 7000023: Do inline object filtering (via page flags) before call to RecordWriteStub.

Issue 7000023: Do inline object filtering (via page flags) before call to RecordWriteStub. (Closed)

Created:
9 years, 7 months ago by Vyacheslav Egorov (Chromium)

Modified:
9 years, 7 months ago

Reviewers:
Erik Corry

CC:
v8-dev

Base URL:
https://v8.googlecode.com/svn/branches/experimental/gc

Visibility:
Public.

Description

Do inline object filtering (via page flags) before call to RecordWriteStub. Committed: http://code.google.com/p/v8/source/detail?r=7889

Patch Set 1 #

Total comments: 10

Created: 9 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+172 lines, -43 lines)			Patch
M	src/ia32/code-stubs-ia32.cc	View	2 chunks	+6 lines, -11 lines	0 comments	Download
M	src/ia32/full-codegen-ia32.cc	View	1 chunk	+5 lines, -1 line	0 comments	Download
M	src/ia32/macro-assembler-ia32.h	View	1 chunk	+6 lines, -5 lines	0 comments	Download
M	src/ia32/macro-assembler-ia32.cc	View	3 chunks	+22 lines, -3 lines	2 comments	Download
M	src/ia32/macro-assembler-ia32-inl.h	View	1 chunk	+10 lines, -5 lines	2 comments	Download
M	src/incremental-marking.h	View	2 chunks	+14 lines, -0 lines	0 comments	Download
M	src/incremental-marking.cc	View	4 chunks	+67 lines, -2 lines	2 comments	Download
M	src/spaces.h	View	8 chunks	+22 lines, -12 lines	2 comments	Download
M	src/spaces.cc	View	3 chunks	+5 lines, -1 line	0 comments	Download
M	src/spaces-inl.h	View	3 chunks	+15 lines, -3 lines	2 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Erik Corry

LGTM! http://codereview.chromium.org/7000023/diff/1/src/ia32/macro-assembler-ia32-inl.h File src/ia32/macro-assembler-ia32-inl.h (right): http://codereview.chromium.org/7000023/diff/1/src/ia32/macro-assembler-ia32-inl.h#newcode43 src/ia32/macro-assembler-ia32-inl.h:43: LabelType* not_set) { not_set seems like the wrong ...

9 years, 7 months ago (2011-05-11 18:53:57 UTC) #2

Vyacheslav Egorov (Chromium)

9 years, 7 months ago (2011-05-13 11:06:52 UTC) #3

Thanks! Landing.

http://codereview.chromium.org/7000023/diff/1/src/ia32/macro-assembler-ia32-i...
File src/ia32/macro-assembler-ia32-inl.h (right):

http://codereview.chromium.org/7000023/diff/1/src/ia32/macro-assembler-ia32-i...
src/ia32/macro-assembler-ia32-inl.h:43: LabelType* not_set) {
On 2011/05/11 18:53:57, Erik Corry wrote:
> not_set seems like the wrong name, since it means is_set if the condition is
> not_zero.  How about condition_met?

Done.

http://codereview.chromium.org/7000023/diff/1/src/ia32/macro-assembler-ia32.cc
File src/ia32/macro-assembler-ia32.cc (right):

http://codereview.chromium.org/7000023/diff/1/src/ia32/macro-assembler-ia32.c...
src/ia32/macro-assembler-ia32.cc:241: value,
On 2011/05/11 18:53:57, Erik Corry wrote:
> Add comment that value is used as scratch here.

Done.

http://codereview.chromium.org/7000023/diff/1/src/incremental-marking.cc
File src/incremental-marking.cc (right):

http://codereview.chromium.org/7000023/diff/1/src/incremental-marking.cc#newc...
src/incremental-marking.cc:165: void
IncrementalMarking::RevertWriteBarrierFlags(PagedSpace* space) {
On 2011/05/11 18:53:57, Erik Corry wrote:
> Revert doesn't convey a lot of meaning to me.  Reverting from what to what? 
How
> about "DeactivateIncrementalWriteBarrier"?

Done.

http://codereview.chromium.org/7000023/diff/1/src/spaces-inl.h
File src/spaces-inl.h (right):

http://codereview.chromium.org/7000023/diff/1/src/spaces-inl.h#newcode278
src/spaces-inl.h:278: // TODO(gc) ISOLATESMERGE initialize chunk to point to
heap?
On 2011/05/11 18:53:57, Erik Corry wrote:
> I don't see how we can defer this to a TODO.  We have GetHeap that works on
any
> HeapObject.  This will fail if we get a large object.

Done.

http://codereview.chromium.org/7000023/diff/1/src/spaces.h
File src/spaces.h (right):

http://codereview.chromium.org/7000023/diff/1/src/spaces.h#newcode370
src/spaces.h:370: CONTAINS_INTERESTING_DESTINATIONS,
On 2011/05/11 18:53:57, Erik Corry wrote:
> I don't like these names.  If a pointer points from one object to another how
> can the destination be the 'from' object.
> 
> How about POINTERS_FROM_HERE_ARE_INTERESTING and
> POINTERS_TO_HERE_ARE_INTERESTING

Done.

Expand Messages | Collapse Messages