Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1850443006:
[Interpreter] Handles BytecodeArrays when scanning objects in heap. (Closed)
Description[Interpreter] Handles BytecodeArrays when scanning objects in heap.
Handles bytecodeArray Objects when verifying the heap and also when
collecting code statistics. The changes include:
1. BytecodeArrays could be a part of the large object space. When
verifying the large object space we should also allow BytecodeArray
objects.
2. Adds support for BytecodeArrays when collecting code statistics.
BUG=v8:4280, chromium:599001
LOG=N
Committed: https://crrev.com/8a9ada486318eea3a03c0526fa9d7674ebe011a5
Cr-Commit-Position: refs/heads/master@{#35202}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Adds mjsunit test to check verifyheap handles BytecodeArrays #
Messages
Total messages: 22 (10 generated)
The CQ bit was checked by mythria@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1850443006/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1850443006/1
Description was changed from ========== [Interpreter] Handles BytecodeArrays when scanning objects in heap. Handles bytecodeArray Objects when verifying the heap and also when collecting code statistics. The changes include: 1. BytecodeArrays could be a part of the large object space. When verifying the large object space we should also allow BytecodeArray objects. 2. Adds support for BytecodeArrays when collecting code statistics. BUG=v8:4280,chromium:599001 ========== to ========== [Interpreter] Handles BytecodeArrays when scanning objects in heap. Handles bytecodeArray Objects when verifying the heap and also when collecting code statistics. The changes include: 1. BytecodeArrays could be a part of the large object space. When verifying the large object space we should also allow BytecodeArray objects. 2. Adds support for BytecodeArrays when collecting code statistics. BUG=v8:4280,chromium:599001 LOG=N ==========
mythria@chromium.org changed reviewers: + hpayer@chromium.org, oth@chromium.org
Hannes, Orion, I am looking at a failing test. (https://bugs.chromium.org/p/chromium/issues/detail?id=599001). It fails because we do not expect BytecodeArray objects in the large_object_space. I have modified the DCHECK to check for AbstractCode which is a wrapper for both Code and BytecodeArray to fix this. As a part of this I have also updated other parts where I thought it is required to handle BytecodeArrays. PTAL. Thanks, Mythri
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Looks fine here. You have the distilled test case, do we want to add an mjsunit for it too? Reserve final approval for Hannes. https://codereview.chromium.org/1850443006/diff/1/src/heap/spaces.cc File src/heap/spaces.cc (right): https://codereview.chromium.org/1850443006/diff/1/src/heap/spaces.cc#newcode2788 src/heap/spaces.cc:2788: // TODO(mythria): Also enable this for bytecode array when bytecodearray Not sure how speculative this comment is - will the BytecodeArray definitely need reloc info? Nit - bytecode array, bytecodearray -> BytecodeArray, it
lgtm
Thanks for your reviews. Orion, I added a mjsunit test. Could you please look at it. Thanks, Mythri
The CQ bit was checked by mythria@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1850443006/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1850443006/20001
On 2016/04/01 12:45:45, mythria wrote: > Thanks for your reviews. > > Orion, I added a mjsunit test. Could you please look at it. > > Thanks, > Mythri Looks great! Thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mythria@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org Link to the patchset: https://codereview.chromium.org/1850443006/#ps20001 (title: "Adds mjsunit test to check verifyheap handles BytecodeArrays")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1850443006/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1850443006/20001
Message was sent while issue was closed.
Description was changed from ========== [Interpreter] Handles BytecodeArrays when scanning objects in heap. Handles bytecodeArray Objects when verifying the heap and also when collecting code statistics. The changes include: 1. BytecodeArrays could be a part of the large object space. When verifying the large object space we should also allow BytecodeArray objects. 2. Adds support for BytecodeArrays when collecting code statistics. BUG=v8:4280,chromium:599001 LOG=N ========== to ========== [Interpreter] Handles BytecodeArrays when scanning objects in heap. Handles bytecodeArray Objects when verifying the heap and also when collecting code statistics. The changes include: 1. BytecodeArrays could be a part of the large object space. When verifying the large object space we should also allow BytecodeArray objects. 2. Adds support for BytecodeArrays when collecting code statistics. BUG=v8:4280,chromium:599001 LOG=N ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [Interpreter] Handles BytecodeArrays when scanning objects in heap. Handles bytecodeArray Objects when verifying the heap and also when collecting code statistics. The changes include: 1. BytecodeArrays could be a part of the large object space. When verifying the large object space we should also allow BytecodeArray objects. 2. Adds support for BytecodeArrays when collecting code statistics. BUG=v8:4280,chromium:599001 LOG=N ========== to ========== [Interpreter] Handles BytecodeArrays when scanning objects in heap. Handles bytecodeArray Objects when verifying the heap and also when collecting code statistics. The changes include: 1. BytecodeArrays could be a part of the large object space. When verifying the large object space we should also allow BytecodeArray objects. 2. Adds support for BytecodeArrays when collecting code statistics. BUG=v8:4280,chromium:599001 LOG=N Committed: https://crrev.com/8a9ada486318eea3a03c0526fa9d7674ebe011a5 Cr-Commit-Position: refs/heads/master@{#35202} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/8a9ada486318eea3a03c0526fa9d7674ebe011a5 Cr-Commit-Position: refs/heads/master@{#35202} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
