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

Issue 2623273007: Fast path for ThreadSpecific for main thread on TLS-slow platforms (Closed)

Created:
3 years, 11 months ago by Charlie Harrison
Modified:
3 years, 11 months ago
Reviewers:
haraken
CC:
chromium-reviews, oilpan-reviews, Mads Ager (chromium), haraken, blink-reviews, kinuko+watch, kouhei+heap_chromium.org, blink-reviews-wtf_chromium.org, Mikhail
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fast path for ThreadSpecific for main thread on TLS-slow platforms This code is mainly a refactor of utility code out of platform/heap and into wtf/StackUtil. ThreadSpecific then copies some tricks done by ThreadState. BUG=621786 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2623273007 Cr-Commit-Position: refs/heads/master@{#444581} Committed: https://chromium.googlesource.com/chromium/src/+/098f72afae1133c762650b2c841981be9b4afe6e

Patch Set 1 #

Patch Set 2 : fixes #

Patch Set 3 : [WIP] Fast path for currentThread() for main thread on TLS-slow platforms #

Patch Set 4 : [WIP] Fast path for currentThread() for main thread on TLS-slow platforms #

Patch Set 5 : [WIP] Fast path for currentThread() for main thread on TLS-slow platforms #

Patch Set 6 : [WIP] Fast path for currentThread() for main thread on TLS-slow platforms #

Patch Set 7 : [WIP] Fast path for currentThread() for main thread on TLS-slow platforms #

Total comments: 8

Patch Set 8 : Buff up all ThreadSpecifics with the optimization #

Patch Set 9 : probably (?) fix windows threadStackSize #

Patch Set 10 : [WIP] Fast path for currentThread() for main thread on TLS-slow platforms #

Patch Set 11 : simplify patch #

Patch Set 12 : proper namespacing #

Patch Set 13 : bootstrap windows #

Total comments: 1

Patch Set 14 : fix typofix typo #

Total comments: 11

Patch Set 15 : haraken revieqw #

Unified diffs Side-by-side diffs Delta from patch set Stats (+167 lines, -283 lines) Patch
M third_party/WebKit/Source/platform/heap/StackFrameDepth.h View 1 chunk +0 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/platform/heap/StackFrameDepth.cpp View 4 chunks +3 lines, -125 lines 0 comments Download
M third_party/WebKit/Source/platform/heap/ThreadState.h View 1 2 3 4 5 6 7 4 chunks +0 lines, -26 lines 0 comments Download
M third_party/WebKit/Source/platform/heap/ThreadState.cpp View 1 2 3 6 chunks +4 lines, -60 lines 0 comments Download
M third_party/WebKit/Source/wtf/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/wtf/StackUtil.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +54 lines, -0 lines 0 comments Download
A + third_party/WebKit/Source/wtf/StackUtil.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 5 chunks +56 lines, -51 lines 0 comments Download
M third_party/WebKit/Source/wtf/ThreadSpecific.h View 1 2 3 4 5 6 7 11 3 chunks +24 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/wtf/ThreadingPthreads.cpp View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/wtf/ThreadingWin.cpp View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/wtf/WTFThreadData.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +8 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/wtf/WTFThreadData.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +15 lines, -0 lines 0 comments Download

Messages

Total messages: 72 (59 generated)
Charlie Harrison
Hey haraken@, This is the stack based optimization ported from ThreadState we discussed earlier. Most ...
3 years, 11 months ago (2017-01-16 02:44:40 UTC) #29
Charlie Harrison
Oh, and let me know if this is what you had in mind for crbug.com/621786. ...
3 years, 11 months ago (2017-01-16 03:25:33 UTC) #30
haraken
On 2017/01/16 03:25:33, Charlie Harrison wrote: > Oh, and let me know if this is ...
3 years, 11 months ago (2017-01-16 04:42:30 UTC) #31
Charlie Harrison
Hey haraken@, Would you take another look? This change tries to constrain most of the ...
3 years, 11 months ago (2017-01-18 01:49:48 UTC) #50
Charlie Harrison
Hmm still seeing test failures on Windows. I'll try to build locally tomorrow :/
3 years, 11 months ago (2017-01-18 02:45:50 UTC) #51
Charlie Harrison
Looks like it was just a typo and the current patch is doing ok in ...
3 years, 11 months ago (2017-01-18 04:46:55 UTC) #56
haraken
LGTM with comments. https://codereview.chromium.org/2623273007/diff/250001/third_party/WebKit/Source/wtf/StackUtil.cpp File third_party/WebKit/Source/wtf/StackUtil.cpp (right): https://codereview.chromium.org/2623273007/diff/250001/third_party/WebKit/Source/wtf/StackUtil.cpp#newcode25 third_party/WebKit/Source/wtf/StackUtil.cpp:25: reinterpret_cast<uintptr_t>(WTF::getStackStart()) - sizeof(void*); Add a comment ...
3 years, 11 months ago (2017-01-18 05:29:42 UTC) #57
Charlie Harrison
Thanks, tried to address all your concerns. https://codereview.chromium.org/2623273007/diff/250001/third_party/WebKit/Source/wtf/StackUtil.cpp File third_party/WebKit/Source/wtf/StackUtil.cpp (right): https://codereview.chromium.org/2623273007/diff/250001/third_party/WebKit/Source/wtf/StackUtil.cpp#newcode25 third_party/WebKit/Source/wtf/StackUtil.cpp:25: reinterpret_cast<uintptr_t>(WTF::getStackStart()) - ...
3 years, 11 months ago (2017-01-18 16:29:55 UTC) #62
Charlie Harrison
Because I was not able to address all your comments, I'll wait for one more ...
3 years, 11 months ago (2017-01-18 21:41:48 UTC) #65
haraken
LGTM https://codereview.chromium.org/2623273007/diff/250001/third_party/WebKit/Source/wtf/ThreadSpecific.h File third_party/WebKit/Source/wtf/ThreadSpecific.h (right): https://codereview.chromium.org/2623273007/diff/250001/third_party/WebKit/Source/wtf/ThreadSpecific.h#newcode283 third_party/WebKit/Source/wtf/ThreadSpecific.h:283: if (UNLIKELY(!*ptr)) { On 2017/01/18 16:29:55, Charlie Harrison ...
3 years, 11 months ago (2017-01-19 01:04:39 UTC) #66
Charlie Harrison
Thank you, haraken!
3 years, 11 months ago (2017-01-19 01:05:49 UTC) #67
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/2623273007/270001
3 years, 11 months ago (2017-01-19 01:06:33 UTC) #69
commit-bot: I haz the power
3 years, 11 months ago (2017-01-19 01:13:15 UTC) #72
Message was sent while issue was closed.
Committed patchset #15 (id:270001) as
https://chromium.googlesource.com/chromium/src/+/098f72afae1133c762650b2c8419...

Powered by Google App Engine
This is Rietveld 408576698