New issue
Advanced search Search tips

Issue 623700 link

Starred by 5 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocked on:
issue 622400



Sign in to add a comment

Use TaskScheduler for WebCrypto

Project Member Reported by gab@chromium.org, Jun 27 2016

Issue description

Opening discussion:

eroman@ wrote @  https://crbug.com/622400#c1 :

"""
> Note: special care will be taken for pools that only have 1 thread
> (e.g. webcrypto) and were merely used as a way to get access to
> ShutdownBehavior logic from a single-thread (I think..?). 

The WebCrypto design constraints at the time were needing a sequenced runner, and for tasks to not be joined at shutdown.

Also note that this threadpool is running in the *renderer* process.

Some of those dependencies have since changed, and I am in the process of updating documentation around this and checking the implementation (https://codereview.chromium.org/2088323002/).

The good news is I *believe* we can drop these requirements and go full-blown ExecutionMode::PARALLEL using your new TaskScheduler.

This would be helpful in fact, since right now the single threadedness of WebCrypto is problematic for consumers wanting to do bulk operations and saturate their CPU. But at the same time I was reluctant to simply let web sites have unfettered control and make things unresponsive, and was mulling with crazy ideas like  issue 615133 .

The TaskScheduler seems like it could solve this. Just note that the quantity and types of these tasks are directly controlled by users through Javascript
"""

This is not relevant until TaskScheduler runs in renderers, but this use case could help expedite making that so.

Also, regarding "throughput mitigation" (re. DHCP's threading requirements from [1]), we could have TaskTraits on PARALLEL TaskRunners to indicate maximum number of parallel tasks. Or, in fact, we will probably need to have the TaskScheduler be aware of a TaskRunner that posts way more PARALLEL tasks than average and de-prioritize it somehow so it doesn't flood the system.
PS: High workload/etc. is the precise reason TaskScheduler won't initially support the renderer (initially focusing on scheduling I/O in the browser process).

[1] https://codereview.chromium.org/2077413009/diff/20001/net/proxy/dhcp_proxy_script_fetcher_win.cc#newcode62
 

Comment 1 by gab@chromium.org, Jun 27 2016

Labels: -Type-Bug Type-Feature

Comment 2 by gab@chromium.org, Sep 13 2016

Blockedon: 622400
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2c979bb3e7217bbe151f70b36728f12ec442ef09

commit 2c979bb3e7217bbe151f70b36728f12ec442ef09
Author: gab <gab@chromium.org>
Date: Wed Sep 14 02:36:47 2016

Replace usage of single-threaded SequencedWorkerPool with a non-joinable base::Thread in WebCrypto.

BUG= 646443 , 623700

Review-Url: https://codereview.chromium.org/2338893003
Cr-Commit-Position: refs/heads/master@{#418458}

[modify] https://crrev.com/2c979bb3e7217bbe151f70b36728f12ec442ef09/components/webcrypto/webcrypto_impl.cc

Comment 4 by gab@chromium.org, Nov 7 2016

Components: Internals>TaskScheduler
Project Member

Comment 5 by sheriffbot@chromium.org, Nov 8 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by gab@chromium.org, Nov 8 2017

Labels: -Hotlist-Recharge-Cold
Status: Available (was: Untriaged)
Note that "when it becomes available in renderers is only applicable to WebCrypto and not DHCP. The latter is code that runs in the browser process, and better described by  issue 667892 .
TaskScheduler is available in renderers. If you're fine with parallel execution, CryptoThreadPool::PostTask calls should be replaced with base::PostTaskWithTraits calls.
Components: Blink>WebCrypto
Summary: Use TaskScheduler for WebCrypto (was: WebCrypto/DHCP as clients of TaskScheduler (when it becomes active in renderers))

Comment 10 by gab@chromium.org, May 7 2018

Owner: eroman@chromium.org
Status: Untriaged (was: Available)
@eroman : TaskScheduler is fully functional in every process now. Let us know if you need help setting things up (or we can close this bug if you don't think this is worthwhile anymore).

Thanks!
Status: Assigned (was: Untriaged)
Thanks gab!

I will explore it this week.

Parallel execution with the task scheduler is something I am eager to enable.

It should be a straightforward conversion; I just need to audit the code and tests since we have only ever run it single-threaded.
Owner: ----
Status: Available (was: Assigned)
Components: -Internals

Sign in to add a comment