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

Issue 2727009: Faster implementation of Heap::RecordWrites. (Closed)

Created:
10 years, 6 months ago by antonm
Modified:
9 years, 4 months ago
CC:
v8-dev
Visibility:
Public.

Description

Faster implementation of Heap::RecordWrites. Naive algorithm for to update RSets for a span is rather inefficient as it performs many unnecessary operations (retrieving a mask, updating it with the same bit as many pointers go into a single region). Committed: http://code.google.com/p/v8/source/detail?r=4849

Patch Set 1 #

Patch Set 2 : Unintended edit #

Patch Set 3 : New cool implementation. All the credits go to Slava #

Total comments: 2

Patch Set 4 : Fixing a style #

Patch Set 5 : Next round of Slava's comments #

Patch Set 6 : Last round of comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -6 lines) Patch
M src/heap-inl.h View 1 chunk +3 lines, -6 lines 0 comments Download
M src/spaces.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/spaces-inl.h View 1 2 3 4 5 1 chunk +27 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
antonm
Slava, may you have a look? I'd esp. appreciate careful check if mask calculating algorithm ...
10 years, 6 months ago (2010-06-11 11:53:29 UTC) #1
antonm
Slava, another version with your cool way to calculate the mask.
10 years, 6 months ago (2010-06-11 15:32:33 UTC) #2
Vyacheslav Egorov (Chromium)
10 years, 6 months ago (2010-06-11 16:06:33 UTC) #3
LGTM with comments addressed.

http://codereview.chromium.org/2727009/diff/5001/6002
File src/spaces-inl.h (right):

http://codereview.chromium.org/2727009/diff/5001/6002#newcode166
src/spaces-inl.h:166: if (!result) result = start_mask | end_mask;
result == 0 instead?

a small comment about why we check result == 0 here might be handy (start can be
greater than end)

http://codereview.chromium.org/2727009/diff/5001/6002#newcode179
src/spaces-inl.h:179: 
One more empty line here.

Powered by Google App Engine
This is Rietveld 408576698