Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1505173002:
Remove deprecated API usage from cpu profilers test (Closed)
|
Created:
5 years ago by jochen (gone - plz use gerrit) Modified:
5 years ago Reviewers:
vogelheim CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionRemove deprecated API usage from cpu profilers test
BUG=v8:4134
R=vogelheim@chromium.org
LOG=n
NOTRY=true
NOPRESUBMIT=true
Committed: https://crrev.com/0fd63f123db1f378c57486ad7955575d5a34e2cb
Cr-Commit-Position: refs/heads/master@{#32678}
Patch Set 1 #
Total comments: 2
Messages
Total messages: 24 (12 generated)
lgtm https://codereview.chromium.org/1505173002/diff/1/test/cctest/test-cpu-profil... File test/cctest/test-cpu-profiler.cc (right): https://codereview.chromium.org/1505173002/diff/1/test/cctest/test-cpu-profil... test/cctest/test-cpu-profiler.cc:463: const Vector<v8::Local<v8::String> >& names) { [Unrelated to your CL, so feel free to ignore:] Why does this work? I thought v8::Locals have to live on the stack, but I expect the backing store of a Vector to live on the (C++) heap. I'd assume these references are "invisible" to the GC, and that - if GC happens at just the "wrong" time - the Strings might disappear while being used. (Of course, the unit tests are really small, so it's unlikely to trigger GC in practice.)
The CQ bit was checked by jochen@chromium.org
https://codereview.chromium.org/1505173002/diff/1/test/cctest/test-cpu-profil... File test/cctest/test-cpu-profiler.cc (right): https://codereview.chromium.org/1505173002/diff/1/test/cctest/test-cpu-profil... test/cctest/test-cpu-profiler.cc:463: const Vector<v8::Local<v8::String> >& names) { On 2015/12/08 at 10:07:34, vogelheim wrote: > [Unrelated to your CL, so feel free to ignore:] > > Why does this work? I thought v8::Locals have to live on the stack, but I expect the backing store of a Vector to live on the (C++) heap. I'd assume these references are "invisible" to the GC, and that - if GC happens at just the "wrong" time - the Strings might disappear while being used. (Of course, the unit tests are really small, so it's unlikely to trigger GC in practice.) as long as the vector doesn't outlive the handle scope, it should all work. the handles are just pointers to the handle scope which has to be on the stack.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1505173002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1505173002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)
The CQ bit was checked by jochen@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1505173002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1505173002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...)
The CQ bit was checked by jochen@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1505173002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1505173002/1
The CQ bit was unchecked by jochen@chromium.org
Description was changed from ========== Remove deprecated API usage from cpu profilers test BUG=v8:4134 R=vogelheim@chromium.org LOG=n ========== to ========== Remove deprecated API usage from cpu profilers test BUG=v8:4134 R=vogelheim@chromium.org LOG=n NOTRY=true ==========
The CQ bit was checked by jochen@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1505173002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1505173002/1
The CQ bit was unchecked by jochen@chromium.org
Description was changed from ========== Remove deprecated API usage from cpu profilers test BUG=v8:4134 R=vogelheim@chromium.org LOG=n NOTRY=true ========== to ========== Remove deprecated API usage from cpu profilers test BUG=v8:4134 R=vogelheim@chromium.org LOG=n NOTRY=true NOPRESUBMIT=true ==========
The CQ bit was checked by jochen@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1505173002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1505173002/1
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Remove deprecated API usage from cpu profilers test BUG=v8:4134 R=vogelheim@chromium.org LOG=n NOTRY=true NOPRESUBMIT=true ========== to ========== Remove deprecated API usage from cpu profilers test BUG=v8:4134 R=vogelheim@chromium.org LOG=n NOTRY=true NOPRESUBMIT=true Committed: https://crrev.com/0fd63f123db1f378c57486ad7955575d5a34e2cb Cr-Commit-Position: refs/heads/master@{#32678} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/0fd63f123db1f378c57486ad7955575d5a34e2cb Cr-Commit-Position: refs/heads/master@{#32678} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
