Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(33)

Issue 2015173003: Address ThreadHeap::willObjectBeLazilySwept() corner case. (Closed)

Created:
4 years, 8 months ago by sof
Modified:
4 years, 8 months ago
CC:
chromium-reviews, blink-reviews, kouhei+heap_chromium.org, oilpan-reviews, Mads Ager (chromium)
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Address ThreadHeap::willObjectBeLazilySwept() corner case. If willObjectBeLazilySwept(object) was used when finalizing an object on a lazily swept page, and |object| happened to reside on the same heap page, the predicate would return the wrong result if the object had been swept past (and it had been deemed to be alive.) Addressed by adding a special case for querying objects on the same page, making willObjectBeLazilySwept() precisely determine liveness in the face of lazy sweeping. R= BUG= Committed: https://crrev.com/197454b5a3762b4d303a16261f0e68a955070919 Cr-Commit-Position: refs/heads/master@{#396798}

Patch Set 1 #

Patch Set 2 : comment + whitespace #

Patch Set 3 : rebased #

Total comments: 11
Unified diffs Side-by-side diffs Delta from patch set Stats (+92 lines, -2 lines) Patch
M third_party/WebKit/Source/platform/heap/Heap.h View 1 2 1 chunk +12 lines, -1 line 1 comment Download
M third_party/WebKit/Source/platform/heap/HeapPage.h View 1 2 4 chunks +7 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/heap/HeapPage.cpp View 1 2 5 chunks +73 lines, -0 lines 10 comments Download

Messages

Total messages: 24 (8 generated)
sof
please take a look.
4 years, 8 months ago (2016-05-30 17:33:45 UTC) #3
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2015173003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2015173003/20001
4 years, 8 months ago (2016-05-30 17:34:27 UTC) #5
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: ios-device on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds/13255) ios-device-gn on ...
4 years, 8 months ago (2016-05-30 17:35:41 UTC) #7
haraken
https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/Heap.h File third_party/WebKit/Source/platform/heap/Heap.h (right): https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/Heap.h#newcode273 third_party/WebKit/Source/platform/heap/Heap.h:273: // such a reference. To check this, can we ...
4 years, 8 months ago (2016-05-30 23:54:49 UTC) #9
haraken
On 2016/05/30 23:54:49, haraken wrote: > https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/Heap.h > File third_party/WebKit/Source/platform/heap/Heap.h (right): > > https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/Heap.h#newcode273 > ...
4 years, 8 months ago (2016-05-30 23:55:08 UTC) #10
sof
https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp File third_party/WebKit/Source/platform/heap/HeapPage.cpp (right): https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp#newcode403 third_party/WebKit/Source/platform/heap/HeapPage.cpp:403: for (Address headerAddress = normalPage->payload(); headerAddress < pageEnd; ) ...
4 years, 8 months ago (2016-05-31 05:13:23 UTC) #11
haraken
https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp File third_party/WebKit/Source/platform/heap/HeapPage.cpp (right): https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp#newcode403 third_party/WebKit/Source/platform/heap/HeapPage.cpp:403: for (Address headerAddress = normalPage->payload(); headerAddress < pageEnd; ) ...
4 years, 8 months ago (2016-05-31 05:26:38 UTC) #12
sof
https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp File third_party/WebKit/Source/platform/heap/HeapPage.cpp (right): https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp#newcode415 third_party/WebKit/Source/platform/heap/HeapPage.cpp:415: if (!header->isFree() && header->isMarked()) { On 2016/05/31 05:26:38, haraken ...
4 years, 8 months ago (2016-05-31 05:32:25 UTC) #13
haraken
https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp File third_party/WebKit/Source/platform/heap/HeapPage.cpp (right): https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp#newcode320 third_party/WebKit/Source/platform/heap/HeapPage.cpp:320: if (m_firstUnsweptPage && !m_firstUnsweptPage->isLargeObjectPage()) { Why is it enough ...
4 years, 8 months ago (2016-05-31 05:41:16 UTC) #14
sof
On 2016/05/31 05:41:16, haraken wrote: > https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp > File third_party/WebKit/Source/platform/heap/HeapPage.cpp (right): > > https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp#newcode320 > ...
4 years, 8 months ago (2016-05-31 05:51:50 UTC) #15
haraken
On 2016/05/31 05:51:50, sof wrote: > On 2016/05/31 05:41:16, haraken wrote: > > > https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp ...
4 years, 8 months ago (2016-05-31 07:17:35 UTC) #16
sof
https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp File third_party/WebKit/Source/platform/heap/HeapPage.cpp (right): https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp#newcode320 third_party/WebKit/Source/platform/heap/HeapPage.cpp:320: if (m_firstUnsweptPage && !m_firstUnsweptPage->isLargeObjectPage()) { On 2016/05/31 05:41:16, haraken ...
4 years, 8 months ago (2016-05-31 07:24:09 UTC) #17
haraken
On 2016/05/31 07:24:09, sof wrote: > https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp > File third_party/WebKit/Source/platform/heap/HeapPage.cpp (right): > > https://codereview.chromium.org/2015173003/diff/40001/third_party/WebKit/Source/platform/heap/HeapPage.cpp#newcode320 > ...
4 years, 8 months ago (2016-05-31 07:25:48 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2015173003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2015173003/40001
4 years, 8 months ago (2016-05-31 07:32:09 UTC) #20
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 8 months ago (2016-05-31 07:35:58 UTC) #22
commit-bot: I haz the power
4 years, 8 months ago (2016-05-31 07:38:32 UTC) #24
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/197454b5a3762b4d303a16261f0e68a955070919
Cr-Commit-Position: refs/heads/master@{#396798}

Powered by Google App Engine
This is Rietveld 408576698