Thread creation is causing jank on the UI Thread |
|||
Issue descriptionFrom this slow reports: df3a0514cccee32a There is a 4 seconds jank on the UI thread caused by thread creation. This bug was detected when investigating OpenNewLog jank : crbug/882892 The sampling profiler is pointing to that code: NtCreateThreadEx CreateRemoteThreadEx CreateThreadStub base::`anonymous namespace\'::CreateThreadInternal base::internal::SchedulerWorker::Start(base::SchedulerWorkerObserver *) base::internal::SchedulerSingleThreadTaskRunnerManager::CreateCOMSTATaskRunnerWithTraits(base::TaskTraits const &,base::SingleThreadTaskRunnerThreadMode) base::internal::TaskSchedulerImpl::CreateCOMSTATaskRunnerWithTraits(base::TaskTraits const &,base::SingleThreadTaskRunnerThreadMode) base::CreateCOMSTATaskRunnerWithTraits(base::TaskTraits const &,base::SingleThreadTaskRunnerThreadMode) AntiVirusMetricsProvider::AsyncInit(base::RepeatingCallback<void > const &) metrics::DelegatingProvider::AsyncInit(base::RepeatingCallback<void > const &) metrics::MetricsService::StartInitTask() base::debug::TaskAnnotator::RunTask(char const *,base::PendingTask *) base::MessageLoop::RunTask(base::PendingTask *) base::MessageLoop::DoDelayedWork(base::TimeTicks *) base::MessagePumpForUI::DoRunLoop() base::MessagePumpWin::Run(base::MessagePump::Delegate *) base::RunLoop::Run() ChromeBrowserMainParts::MainMessageLoopRun(int *) content::BrowserMainLoop::RunMainMessageLoopParts() content::BrowserMainRunnerImpl::Run() content::BrowserMain(content::MainFunctionParams const &) content::RunBrowserProcessMain(content::MainFunctionParams const &,content::ContentMainDelegate *) content::ContentMainRunnerImpl::Run(bool) service_manager::Main(service_manager::MainParams const &) content::ContentMain(content::ContentMainParams const &) ChromeMain MainDllLoader::Launch(HINSTANCE__ *,base::TimeTicks) wWinMain __scrt_common_main_seh BaseThreadInitThunk RtlUserThreadStart
,
Nov 20
This may be related to: https://bugs.chromium.org/p/chromium/issues/detail?id=906079
,
Nov 20
,
Nov 21
As discussed offline, this falls in etiennep's mandate to improve thread management in TaskScheduler. The tricky part is that TaskScheduler's entire logic is based on the invariant that a thread is created on the |idle_workers_stack_| and begins asleep. Changing PostTask to not create threads (it currently replaces the last idle thread if it wakes it up) is thus tricky. And the current logic requiring transactional properties on |idle_workers_stack_| implies the SchedulerWorkerPoolImpl's lock needs to be held during thread creation which worsens this problem. etiennep@ suggested an approach which is the simplest delta from the current implementation : create the SchedulerWorker with the same logic as before but do *not* Start() it. Instead it will be put in a queue of |workers_to_be_started_| to be serviced by the ServiceThread. All the other logic remains the same (nothing currently assumed ThreadMain() had been entered and this logic will merely delay that a bit -- the SchedulerWorker can be woken up nonetheless and will attempt to pick up work when it gets there in that case.
,
Nov 21
Note that the current bug happens in SingleThread worker. I think this means we should apply the same fix for SchedulerSingleThreadTaskRunnerManager as well (if possible).
,
Nov 21
Right
,
Nov 22
For AntiVirusMetricsProvider::AsyncInit() specifically, I could imagine us moving that specific instance to happen on the worker pool (i.e. post to worker pool and have the function that runs on the worker pool create that thread). But not sure how widespread it is beyond that callsite.
,
Nov 22
No, we'll fix thread creation in //base. You shouldn't have to consider CreateTaskRunner an expensive operation. Le jeu. 22 nov. 2018 14 h 43, asvitkine via monorail < monorail+v2.3492304296@chromium.org> a écrit : |
|||
►
Sign in to add a comment |
|||
Comment 1 by etienneb@chromium.org
, Nov 2038.4 KB
38.4 KB View Download