Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1951453002:
Name TaskScheduler's worker threads (Closed)
|
Created:
4 years, 7 months ago by gab Modified:
4 years, 7 months ago CC:
chromium-reviews, gab+watch_chromium.org, robliao+watch_chromium.org, fdoray+watch_chromium.org, danakj Base URL:
https://chromium.googlesource.com/chromium/src.git@b7_fdoray_fixtracing Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionName TaskScheduler's worker threads
In conjunction with https://codereview.chromium.org/1937323002/ this
makes TaskScheduler work with chrome://tracing.
Thread name will be comprised of 3 parts:
[1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO"
[2] "Worker"
[3] Thread Index in thread pool
This can make for long names, e.g.:
"TaskSchedulerBackgroudFileIOWorker3"
but I think a descriptive name is still preferable.
The one caveat is that WinDBG's implementation of thread name
currently truncates at 32 characters... I'm willing to live with
that for now until (if ever) we realize it's too annoying.
BUG=553459
Committed: https://crrev.com/a0813e0c0286ee6790433164957141e9f299bff7
Cr-Commit-Position: refs/heads/master@{#392351}
Committed: https://crrev.com/b39e39eeb679c3bf45a444e9e6d0c5e364a8cd17
Cr-Commit-Position: refs/heads/master@{#392395}
Patch Set 1 #Patch Set 2 : update comment #
Total comments: 9
Patch Set 3 : review:fdoray-robliao#5-6 #
Total comments: 2
Patch Set 4 : review:fdoray#9 #Patch Set 5 : update comment #Patch Set 6 : merge up to r392140 #Patch Set 7 : use int for Worker thread index #Patch Set 8 : explicit cast to int #
Messages
Total messages: 46 (23 generated)
Description was changed from ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. BUG=553459 ========== to ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 4 parts: [1][2][3]/[4] with: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool [4] Thread ID (to mimic thread naming paradigm of SequencedWorkerPool and other existing thread in Chrome) This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3/12345" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 ==========
gab@chromium.org changed reviewers: + fdoray@chromium.org, robliao@chromium.org
Francois/Rob PTAL
Description was changed from ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 4 parts: [1][2][3]/[4] with: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool [4] Thread ID (to mimic thread naming paradigm of SequencedWorkerPool and other existing thread in Chrome) This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3/12345" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 ========== to ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 4 parts: [1][2][3]/[4] with: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool [4] Thread ID (to mimic thread naming paradigm of SequencedWorkerPool and other existing thread in Chrome) This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3/12345" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 NO_DEPENDENCY_CHECKS=true ==========
https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/sch... File base/task_scheduler/scheduler_thread_pool_impl.cc (right): https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/sch... base/task_scheduler/scheduler_thread_pool_impl.cc:8: #include <utility> #include <stddef.h> for size_t https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/sch... base/task_scheduler/scheduler_thread_pool_impl.cc:398: index_, PlatformThread::CurrentId())); Why is it useful to have index + thread id? I think thread id is the most useful as it can be correlated with crash dumps / debuggers. https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/sch... File base/task_scheduler/scheduler_thread_pool_impl.h (right): https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/sch... base/task_scheduler/scheduler_thread_pool_impl.h:55: // Creates a SchedulerThreadPool labeled |name_| with up to |max_threads| name ^ https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/tas... File base/task_scheduler/task_scheduler_impl.cc (right): https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/tas... base/task_scheduler/task_scheduler_impl.cc:111: "TaskSchedulerForegroundFileIO", ThreadPriority::NORMAL, 12u, Add a verification in VerifyTaskEnvironment in task_scheduler_impl_unittest.cc?
https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/sch... File base/task_scheduler/scheduler_thread_pool_impl.cc (right): https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/sch... base/task_scheduler/scheduler_thread_pool_impl.cc:398: index_, PlatformThread::CurrentId())); On 2016/05/03 19:01:50, fdoray wrote: > Why is it useful to have index + thread id? I think thread id is the most useful > as it can be correlated with crash dumps / debuggers. Doesn't the debugger generate this for you too? 0:001> ~ 0 Id: 2324.2ea4 Suspend: 1 Teb: 00000000`7efdb000 Unfrozen "This is my thread!" . 1 Id: 2324.2e80 Suspend: 1 Teb: 00000000`7efd8000 Unfrozen
Patchset #3 (id:40001) has been deleted
Done, PTAL, thanks! https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/sch... File base/task_scheduler/scheduler_thread_pool_impl.cc (right): https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/sch... base/task_scheduler/scheduler_thread_pool_impl.cc:8: #include <utility> On 2016/05/03 19:01:50, fdoray wrote: > #include <stddef.h> for size_t Done. https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/sch... base/task_scheduler/scheduler_thread_pool_impl.cc:398: index_, PlatformThread::CurrentId())); On 2016/05/03 23:35:38, robliao wrote: > On 2016/05/03 19:01:50, fdoray wrote: > > Why is it useful to have index + thread id? I think thread id is the most > useful > > as it can be correlated with crash dumps / debuggers. Agreed, I initially added the index to have a unique name, then realized the pattern of other threads (e.g. SequencedWorkerPool) was to add /ThreadID, but now that I'm looking closer the /ThreadID is a feature of chrome://tracing, not the actual name of the thread I think so removed (in which case I think keeping the index is better for unique names though). > > Doesn't the debugger generate this for you too? > 0:001> ~ > 0 Id: 2324.2ea4 Suspend: 1 Teb: 00000000`7efdb000 Unfrozen "This is my > thread!" > . 1 Id: 2324.2e80 Suspend: 1 Teb: 00000000`7efd8000 Unfrozen This is debugger dependent. Naming the threads is cross-platform (and there is a rumor on windows that I can only share offline that make this worthwhile :-)). https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/sch... File base/task_scheduler/scheduler_thread_pool_impl.h (right): https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/sch... base/task_scheduler/scheduler_thread_pool_impl.h:55: // Creates a SchedulerThreadPool labeled |name_| with up to |max_threads| On 2016/05/03 19:01:50, fdoray wrote: > name > ^ Done. https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/tas... File base/task_scheduler/task_scheduler_impl.cc (right): https://codereview.chromium.org/1951453002/diff/20001/base/task_scheduler/tas... base/task_scheduler/task_scheduler_impl.cc:111: "TaskSchedulerForegroundFileIO", ThreadPriority::NORMAL, 12u, On 2016/05/03 19:01:50, fdoray wrote: > Add a verification in VerifyTaskEnvironment in task_scheduler_impl_unittest.cc? Done though I'm afraid it's a bit too much of a white-box test, PTAL and let me know what you think.
lgtm https://codereview.chromium.org/1951453002/diff/60001/base/task_scheduler/tas... File base/task_scheduler/task_scheduler_impl_unittest.cc (right): https://codereview.chromium.org/1951453002/diff/60001/base/task_scheduler/tas... base/task_scheduler/task_scheduler_impl_unittest.cc:101: if (traits.priority() == TaskPriority::BACKGROUND) { I would remove lines 101-109 because we shouldn't have to update this test when we update the number of threads per pool. Maybe verifying that the thread name contains the string passed in TaskSchedulerImpl::Initialize would be enough (the rest is an implementation detail).
Description was changed from ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 4 parts: [1][2][3]/[4] with: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool [4] Thread ID (to mimic thread naming paradigm of SequencedWorkerPool and other existing thread in Chrome) This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3/12345" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 NO_DEPENDENCY_CHECKS=true ========== to ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 NO_DEPENDENCY_CHECKS=true ==========
Thanks, done. https://codereview.chromium.org/1951453002/diff/60001/base/task_scheduler/tas... File base/task_scheduler/task_scheduler_impl_unittest.cc (right): https://codereview.chromium.org/1951453002/diff/60001/base/task_scheduler/tas... base/task_scheduler/task_scheduler_impl_unittest.cc:101: if (traits.priority() == TaskPriority::BACKGROUND) { On 2016/05/04 19:04:32, fdoray wrote: > I would remove lines 101-109 because we shouldn't have to update this test when > we update the number of threads per pool. > > Maybe verifying that the thread name contains the string passed in > TaskSchedulerImpl::Initialize would be enough (the rest is an implementation > detail). Done.
Description was changed from ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 NO_DEPENDENCY_CHECKS=true ========== to ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 ==========
lgtm
CC dana FYI (I think this is internal enough not to require your explicit review -- hoping this alleviates you a bit but feel free to drive by if you have anything!)
The CQ bit was checked by gab@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from fdoray@chromium.org Link to the patchset: https://codereview.chromium.org/1951453002/#ps100001 (title: "update comment")
The CQ bit was unchecked by gab@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1951453002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1951453002/100001
On 2016/05/05 13:53:54, commit-bot: I haz the power wrote: > CQ is trying da patch. Follow status at > https://chromium-cq-status.appspot.com/patch-status/1951453002/100001 > View timeline at > https://chromium-cq-status.appspot.com/patch-timeline/1951453002/100001 Oops nvm, depends on TaskSchedulerImpl CL, will CQ right after it :-)
The CQ bit was checked by gab@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1951453002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1951453002/100001
The CQ bit was unchecked by commit-bot@chromium.org
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...) ios-simulator on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...) mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
The CQ bit was checked by gab@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from robliao@chromium.org, fdoray@chromium.org Link to the patchset: https://codereview.chromium.org/1951453002/#ps120001 (title: "merge up to r392140")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1951453002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1951453002/120001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: cast_shell_android on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
Switched to using int instead of size_t to keep the |index_|, makes printf easier (otherwise have to use %zu for which there are mixed comments on whether it's even supported in MSVC). Furthermore, style guide says to shy away from unsigned types unless strictly necessary: https://google.github.io/styleguide/cppguide.html#Integer_Types
The CQ bit was checked by gab@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from robliao@chromium.org, fdoray@chromium.org Link to the patchset: https://codereview.chromium.org/1951453002/#ps140001 (title: "use int for Worker thread index")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1951453002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1951453002/140001
Message was sent while issue was closed.
Description was changed from ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 ========== to ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 ==========
Message was sent while issue was closed.
Committed patchset #7 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 ========== to ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 Committed: https://crrev.com/a0813e0c0286ee6790433164957141e9f299bff7 Cr-Commit-Position: refs/heads/master@{#392351} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/a0813e0c0286ee6790433164957141e9f299bff7 Cr-Commit-Position: refs/heads/master@{#392351}
Message was sent while issue was closed.
A revert of this CL (patchset #7 id:140001) has been created in https://codereview.chromium.org/1958973003/ by iclelland@chromium.org. The reason for reverting is: This patch broke the Win x64 build -- The error is ninja -t msvc -e environment.x64 -- C:\b\build\slave\cache\cipd\goma/gomacc "C:\b\depot_tools\win_toolchain\vs_files\95ddda401ec5678f15eeed01d2bee08fcbc5ee97\VC\bin\amd64\cl.exe" /nologo /showIncludes /FC @obj\base\task_scheduler\base.scheduler_thread_pool_impl.obj.rsp /c ..\..\base\task_scheduler\scheduler_thread_pool_impl.cc /Foobj\base\task_scheduler\base.scheduler_thread_pool_impl.obj /Fdobj\base\base.cc.pdb c:\b\build\slave\win_x64\build\src\base\task_scheduler\scheduler_thread_pool_impl.cc(515): error C2220: warning treated as error - no 'object' file generated c:\b\build\slave\win_x64\build\src\base\task_scheduler\scheduler_thread_pool_impl.cc(515): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data See the whole (broken) compile in https://build.chromium.org/p/chromium/builders/Win%20x64/builds/535/steps/com....
Message was sent while issue was closed.
Description was changed from ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 Committed: https://crrev.com/a0813e0c0286ee6790433164957141e9f299bff7 Cr-Commit-Position: refs/heads/master@{#392351} ========== to ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 Committed: https://crrev.com/a0813e0c0286ee6790433164957141e9f299bff7 Cr-Commit-Position: refs/heads/master@{#392351} ==========
On 2016/05/09 18:15:10, iclelland wrote: > A revert of this CL (patchset #7 id:140001) has been created in > https://codereview.chromium.org/1958973003/ by https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=iclelland@chromium.org. > > The reason for reverting is: This patch broke the Win x64 build -- The error is > ninja -t msvc -e environment.x64 -- C:\b\build\slave\cache\cipd\goma/gomacc > "C:\b\depot_tools\win_toolchain\vs_files\95ddda401ec5678f15eeed01d2bee08fcbc5ee97\VC\bin\amd64\cl.exe" > /nologo /showIncludes /FC > @obj\base\task_scheduler\base.scheduler_thread_pool_impl.obj.rsp /c > ..\..\base\task_scheduler\scheduler_thread_pool_impl.cc > /Foobj\base\task_scheduler\base.scheduler_thread_pool_impl.obj > /Fdobj\base\base.cc.pdb > c:\b\build\slave\win_x64\build\src\base\task_scheduler\scheduler_thread_pool_impl.cc(515): > error C2220: warning treated as error - no 'object' file generated > c:\b\build\slave\win_x64\build\src\base\task_scheduler\scheduler_thread_pool_impl.cc(515): > warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of > data > > See the whole (broken) compile in > https://build.chromium.org/p/chromium/builders/Win%20x64/builds/535/steps/com.... I thought about this but given my local compiler and CQ were happy I assumed there was an implicit conversion happening -- looks like that's not the case, added explicit static_cast. Re-CQ'ing.
The CQ bit was checked by gab@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from robliao@chromium.org, fdoray@chromium.org Link to the patchset: https://codereview.chromium.org/1951453002/#ps160001 (title: "explicit cast to int")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1951453002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1951453002/160001
Message was sent while issue was closed.
Description was changed from ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 Committed: https://crrev.com/a0813e0c0286ee6790433164957141e9f299bff7 Cr-Commit-Position: refs/heads/master@{#392351} ========== to ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 Committed: https://crrev.com/a0813e0c0286ee6790433164957141e9f299bff7 Cr-Commit-Position: refs/heads/master@{#392351} ==========
Message was sent while issue was closed.
Committed patchset #8 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 Committed: https://crrev.com/a0813e0c0286ee6790433164957141e9f299bff7 Cr-Commit-Position: refs/heads/master@{#392351} ========== to ========== Name TaskScheduler's worker threads In conjunction with https://codereview.chromium.org/1937323002/ this makes TaskScheduler work with chrome://tracing. Thread name will be comprised of 3 parts: [1] Thread Pool name, e.g. "TaskSchedulerBackgroudFileIO" [2] "Worker" [3] Thread Index in thread pool This can make for long names, e.g.: "TaskSchedulerBackgroudFileIOWorker3" but I think a descriptive name is still preferable. The one caveat is that WinDBG's implementation of thread name currently truncates at 32 characters... I'm willing to live with that for now until (if ever) we realize it's too annoying. BUG=553459 Committed: https://crrev.com/a0813e0c0286ee6790433164957141e9f299bff7 Cr-Commit-Position: refs/heads/master@{#392351} Committed: https://crrev.com/b39e39eeb679c3bf45a444e9e6d0c5e364a8cd17 Cr-Commit-Position: refs/heads/master@{#392395} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/b39e39eeb679c3bf45a444e9e6d0c5e364a8cd17 Cr-Commit-Position: refs/heads/master@{#392395} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
