Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(29)

Issue 1125613002: Oilpan: handle thread-local weak tracing slightly better. (Closed)

Created:
5 years, 7 months ago by sof
Modified:
5 years, 7 months ago
Reviewers:
oilpan-reviews, haraken
CC:
blink-reviews, oilpan-reviews, kouhei+heap_chromium.org, Mads Ager (chromium)
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Oilpan: handle thread-local weak tracing slightly better. The implementation of marking is marking-thread centric, recording a thread stack limit that the tracing through the object graph cannot exceed. If it does, the implementation will use the explicit mark stack instead of trying to eagerly trace the object, if unmarked. Tracing will in the general case also happen on a per-thread basis, when handling the weak pointer callbacks after the initial marking phase has completed. Using the marking thread's stack limit to determine eagerness or not of a trace call when that weak tracing runs doesn't make much sense. To address, eager tracing is not enabled whilst processing the per-thread weak pointer callbacks. As the amount of tracing is rather moderate for this stage, there are no performance concerns with disabling this optimization. R=haraken BUG=420515 TEST=HeapTest.ThreadedStrongification Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194970

Patch Set 1 #

Patch Set 2 : record marking thread owner over StackFrameDepth #

Patch Set 3 : disable eager tracing for weak callback tracing #

Patch Set 4 : compile fix #

Total comments: 4

Patch Set 5 : rename some methods #

Unified diffs Side-by-side diffs Delta from patch set Stats (+58 lines, -20 lines) Patch
M Source/platform/heap/Heap.cpp View 1 2 2 chunks +3 lines, -4 lines 0 comments Download
M Source/platform/heap/HeapTest.cpp View 1 2 3 4 5 chunks +14 lines, -12 lines 0 comments Download
M Source/platform/heap/StackFrameDepth.h View 1 2 3 4 3 chunks +32 lines, -2 lines 0 comments Download
M Source/platform/heap/StackFrameDepth.cpp View 1 2 3 4 2 chunks +8 lines, -1 line 0 comments Download
M Source/platform/heap/TraceTraits.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 15 (4 generated)
sof
please take a look. The weak marking step doesn't appear accounted for by the stack-based ...
5 years, 7 months ago (2015-05-04 07:08:14 UTC) #2
sof
On 2015/05/04 07:08:14, sof wrote: > please take a look. > > The weak marking ...
5 years, 7 months ago (2015-05-04 09:53:28 UTC) #3
haraken
Thanks for looking into this. > Upon further thought, it would preferable not to be ...
5 years, 7 months ago (2015-05-04 23:11:32 UTC) #5
sof
On 2015/05/04 23:11:32, haraken wrote: > Thanks for looking into this. > > > Upon ...
5 years, 7 months ago (2015-05-05 05:28:08 UTC) #6
sof
On 2015/05/05 05:28:08, sof wrote: > On 2015/05/04 23:11:32, haraken wrote: > > Thanks for ...
5 years, 7 months ago (2015-05-05 06:27:04 UTC) #7
haraken
On 2015/05/05 06:27:04, sof wrote: > On 2015/05/05 05:28:08, sof wrote: > > On 2015/05/04 ...
5 years, 7 months ago (2015-05-05 10:44:05 UTC) #8
sof
On 2015/05/05 10:44:05, haraken wrote: > On 2015/05/05 06:27:04, sof wrote: > > On 2015/05/05 ...
5 years, 7 months ago (2015-05-05 11:13:19 UTC) #9
haraken
On 2015/05/05 11:13:19, sof wrote: > On 2015/05/05 10:44:05, haraken wrote: > > On 2015/05/05 ...
5 years, 7 months ago (2015-05-05 16:04:18 UTC) #10
sof
https://codereview.chromium.org/1125613002/diff/60001/Source/platform/heap/StackFrameDepth.h File Source/platform/heap/StackFrameDepth.h (right): https://codereview.chromium.org/1125613002/diff/60001/Source/platform/heap/StackFrameDepth.h#newcode34 Source/platform/heap/StackFrameDepth.h:34: static void clearStackLimit() On 2015/05/04 23:11:32, haraken wrote: > ...
5 years, 7 months ago (2015-05-06 10:39:57 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1125613002/80001
5 years, 7 months ago (2015-05-06 10:40:10 UTC) #14
commit-bot: I haz the power
5 years, 7 months ago (2015-05-06 12:30:50 UTC) #15
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194970

Powered by Google App Engine
This is Rietveld 408576698