Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2659573002:
[heap] Fix GrowAndShrinkNewSpace heap test (Closed)
|
Created:
3 years, 11 months ago by jgruber Modified:
3 years, 10 months ago Reviewers:
Michael Lippautz CC:
v8-reviews_googlegroups.com Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[heap] Fix GrowAndShrinkNewSpace heap test
BUG=
Review-Url: https://codereview.chromium.org/2659573002
Cr-Commit-Position: refs/heads/master@{#42837}
Committed: https://chromium.googlesource.com/v8/v8/+/0c3a507b3a92b5fa8a5c6b5b57ce888c2c5fb486
Patch Set 1 #
Messages
Total messages: 13 (8 generated)
The CQ bit was checked by jgruber@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
jgruber@chromium.org changed reviewers: + mlippautz@chromium.org
This test breaks for nosnapshot builds on the regexp named capture CL at 8bf52534f6bf86821a1589dcbcb7335052c1f94f [0]. The immediate cause is that 2*new_space->Size() is now greater than new_space->InitialTotalCapacity(), which breaks the expectation that new_space->Shrink() will halve capacity at [1]. Triggering two GCs to empty new space prior the test seems to fix things. I'm surprised though.. I would've expected some increase in code-space since the RegExp CL extends a builtin, but not in new space. I verified that the new code isn't actually being executed in this test case. Here's some numbers comparing xxx_space->Size() with and without the RegExp CL on a nosnapshot build: with RegExp CL new/old/code: 716272/1031160/2014016 w/o RegExp CL new/old/code: 692104/1031152/2014432 Any ideas what could be causing the 25K growth in new space? Is this something we should be looking into? [0] https://codereview.chromium.org/2630233003 [1] https://cs.chromium.org/chromium/src/v8/test/cctest/heap/test-heap.cc?q=GrowA...
On 2017/01/26 12:11:13, jgruber wrote: > This test breaks for nosnapshot builds on the regexp named capture CL at > 8bf52534f6bf86821a1589dcbcb7335052c1f94f [0]. > > The immediate cause is that 2*new_space->Size() is now greater than > new_space->InitialTotalCapacity(), > which breaks the expectation that new_space->Shrink() will halve capacity at > [1]. > > Triggering two GCs to empty new space prior the test seems to fix things. > > I'm surprised though.. I would've expected some increase in code-space since the > RegExp CL extends a builtin, but not in new space. I verified that the new code > isn't actually being executed in this test case. Here's some numbers comparing > xxx_space->Size() with and without the RegExp CL on a nosnapshot build: > > with RegExp CL new/old/code: 716272/1031160/2014016 > w/o RegExp CL new/old/code: 692104/1031152/2014432 > > Any ideas what could be causing the 25K growth in new space? Is this something > we should be looking into? > > [0] https://codereview.chromium.org/2630233003 > [1] > https://cs.chromium.org/chromium/src/v8/test/cctest/heap/test-heap.cc?q=GrowA... Friendly ping, this is blocking my regexp CL.
On 2017/01/31 08:20:25, jgruber wrote: > On 2017/01/26 12:11:13, jgruber wrote: > > This test breaks for nosnapshot builds on the regexp named capture CL at > > 8bf52534f6bf86821a1589dcbcb7335052c1f94f [0]. > > > > The immediate cause is that 2*new_space->Size() is now greater than > > new_space->InitialTotalCapacity(), > > which breaks the expectation that new_space->Shrink() will halve capacity at > > [1]. > > > > Triggering two GCs to empty new space prior the test seems to fix things. > > > > I'm surprised though.. I would've expected some increase in code-space since > the > > RegExp CL extends a builtin, but not in new space. I verified that the new > code > > isn't actually being executed in this test case. Here's some numbers comparing > > xxx_space->Size() with and without the RegExp CL on a nosnapshot build: > > > > with RegExp CL new/old/code: 716272/1031160/2014016 > > w/o RegExp CL new/old/code: 692104/1031152/2014432 > > > > Any ideas what could be causing the 25K growth in new space? Is this something > > we should be looking into? > > > > [0] https://codereview.chromium.org/2630233003 > > [1] > > > https://cs.chromium.org/chromium/src/v8/test/cctest/heap/test-heap.cc?q=GrowA... > > Friendly ping, this is blocking my regexp CL. lgtm sorry, this slipped through while traveling.
The CQ bit was checked by jgruber@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1485935688287570, "parent_rev":
"1a1a3cc492a62e37dfcd5acbfb1b03f3d7db8075", "commit_rev":
"0c3a507b3a92b5fa8a5c6b5b57ce888c2c5fb486"}
Message was sent while issue was closed.
Description was changed from ========== [heap] Fix GrowAndShrinkNewSpace heap test BUG= ========== to ========== [heap] Fix GrowAndShrinkNewSpace heap test BUG= Review-Url: https://codereview.chromium.org/2659573002 Cr-Commit-Position: refs/heads/master@{#42837} Committed: https://chromium.googlesource.com/v8/v8/+/0c3a507b3a92b5fa8a5c6b5b57ce888c2c5... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/v8/v8/+/0c3a507b3a92b5fa8a5c6b5b57ce888c2c5... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
