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

Issue 2697703005: Remove ThreadHeap::m_threads (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 months ago by haraken
Modified:
7 months ago
Reviewers:
sof
CC:
chromium-reviews, oilpan-reviews, Mads Ager (chromium), blink-reviews, kinuko+watch, kouhei+heap_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove ThreadHeap::m_threads Now that ThreadHeap and ThreadState are 1:1, ThreadHeap doesn't need to maintain a set of associated ThreadStates. BUG=671856 Review-Url: https://codereview.chromium.org/2697703005 Cr-Commit-Position: refs/heads/master@{#451457} Committed: https://chromium.googlesource.com/chromium/src/+/aebdbddd1caea79bd6d3875fb093be2197264585

Patch Set 1 #

Total comments: 1

Patch Set 2 : temp #

Patch Set 3 : temp #

Patch Set 4 : temp #

Total comments: 1

Patch Set 5 : temp #

Patch Set 6 : temp #

Patch Set 7 : temp #

Patch Set 8 : temp #

Messages

Total messages: 44 (22 generated)
haraken
PTAL
7 months ago (2017-02-15 08:14:18 UTC) #2
sof
https://codereview.chromium.org/2697703005/diff/1/third_party/WebKit/Source/platform/heap/Heap.cpp File third_party/WebKit/Source/platform/heap/Heap.cpp (right): https://codereview.chromium.org/2697703005/diff/1/third_party/WebKit/Source/platform/heap/Heap.cpp#newcode191 third_party/WebKit/Source/platform/heap/Heap.cpp:191: delete this; Would it work for |ThreadState::m_heap| to be ...
7 months ago (2017-02-15 08:26:32 UTC) #3
haraken
On 2017/02/15 08:26:32, sof wrote: > https://codereview.chromium.org/2697703005/diff/1/third_party/WebKit/Source/platform/heap/Heap.cpp > File third_party/WebKit/Source/platform/heap/Heap.cpp (right): > > https://codereview.chromium.org/2697703005/diff/1/third_party/WebKit/Source/platform/heap/Heap.cpp#newcode191 > ...
7 months ago (2017-02-15 09:06:16 UTC) #4
sof
lgtm https://codereview.chromium.org/2697703005/diff/60001/third_party/WebKit/Source/platform/heap/Heap.cpp File third_party/WebKit/Source/platform/heap/Heap.cpp (left): https://codereview.chromium.org/2697703005/diff/60001/third_party/WebKit/Source/platform/heap/Heap.cpp#oldcode200 third_party/WebKit/Source/platform/heap/Heap.cpp:200: if (thread->isMainThread()) we could move this to the ...
7 months ago (2017-02-15 09:18:21 UTC) #5
haraken
> https://codereview.chromium.org/2697703005/diff/60001/third_party/WebKit/Source/platform/heap/Heap.cpp > File third_party/WebKit/Source/platform/heap/Heap.cpp (left): > > https://codereview.chromium.org/2697703005/diff/60001/third_party/WebKit/Source/platform/heap/Heap.cpp#oldcode200 > third_party/WebKit/Source/platform/heap/Heap.cpp:200: if > (thread->isMainThread()) > ...
7 months ago (2017-02-15 09:23:55 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697703005/80001
7 months ago (2017-02-15 09:24:29 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/390613)
7 months ago (2017-02-15 11:03:08 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697703005/80001
7 months ago (2017-02-15 11:05:26 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/364841)
7 months ago (2017-02-15 11:13:25 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697703005/80001
7 months ago (2017-02-15 14:49:10 UTC) #17
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/364957)
7 months ago (2017-02-15 14:56:56 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697703005/90001
7 months ago (2017-02-16 01:17:45 UTC) #22
commit-bot: I haz the power
Try jobs failed on following builders: chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_asan_rel_ng on ...
7 months ago (2017-02-16 03:21:19 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697703005/90001
7 months ago (2017-02-17 10:39:32 UTC) #26
commit-bot: I haz the power
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/builds/157301) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, ...
7 months ago (2017-02-17 10:42:12 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697703005/110001
7 months ago (2017-02-17 11:29:19 UTC) #31
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/384904)
7 months ago (2017-02-17 13:07:03 UTC) #33
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697703005/110001
7 months ago (2017-02-17 13:36:54 UTC) #35
commit-bot: I haz the power
Failed to apply patch for third_party/WebKit/Source/platform/heap/Heap.cpp: While running git apply --index -p1; error: patch failed: ...
7 months ago (2017-02-17 15:02:37 UTC) #37
sof
(needs a rebase?)
7 months ago (2017-02-18 06:10:54 UTC) #38
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697703005/130001
7 months ago (2017-02-18 10:57:18 UTC) #41
commit-bot: I haz the power
7 months ago (2017-02-18 12:29:54 UTC) #44
Message was sent while issue was closed.
Committed patchset #8 (id:130001) as
https://chromium.googlesource.com/chromium/src/+/aebdbddd1caea79bd6d3875fb093...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b