New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 652825 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 633389



Sign in to add a comment

MessagePort::dispatchMessages() should be smarter about how it schedules events

Project Member Reported by darin@chromium.org, Oct 4 2016

Issue description

MessagePort::dispatchMessages() should be smarter about how it schedules events

When we notice message(s) to read from a MessagePort, we PostTask to the main thread and dispatch all messages before yielding. This could be a source of jank on the main thread.
 
Cc: haraken@chromium.org skyos...@chromium.org
Components: -Blink>DOM Blink>Scheduling

Comment 2 by nduca@chromium.org, Oct 4 2016

Cc: gab@chromium.org
+gab so that luckyluke peeps are aware

My comment here may be beyond the scope of this specific bug, but since I'm already typing, it seems at least to me like we're missing a set of principles that explain how mojo messages relate to tasks and priority. Right now we've got two great systems -- an ipc system, and a schedulable messageloop system. But, they mostly are designed separately. Seems like we could do well to bring their worlds more closely together, someday? Curious what others think. :)

Comment 3 by darin@chromium.org, Oct 4 2016

Cc: roc...@chromium.org
@nduca - I'm not sure I get your comment. The mojo c++ bindings need a base::SingleThreadTaskRunner in order to dispatch events. This appears to fit very nicely with the scheduler. Take a look at the WebSockets code. We just pass the loading TaskRunner to the WebSocket and that is passed along to the Mojo bindings, so every event related to WebSockets in the renderer process is managed by the scheduler. Isn't that pretty close to exactly what we want?

Comment 4 by nduca@chromium.org, Oct 4 2016

Gah, yeah, I got MessagePort::dispatch confused with MessagePipeDispatcher. Class names crossed in my head, sorry for the noise! :)
Right, using base::SingleThreadTaskRunner as the unit of schedulability seems to work nicely here. Gab, I assume Lucky Luke can work with that too?
MessagePort::dispatchMessages() should be smarter about how it schedules
events

seems to work nicely here. Gab, I assume Lucky Luke can work with that too?

One thing I've wondered is if we should prefer to instead use
SequencedTaskRunner. I see no reason why we need to expose a "thread"
detail or restrict an endpoint's scheduling to a single thread as long as
it's sequenced.

Comment 7 by gab@chromium.org, Oct 5 2016

Yes, you will soon be able to do:

base::CreateTaskRunnerWithTraits(TaskTraits(), ExecutionMode::<mode>);

Where <mode> is one of:

enum class ExecutionMode {
  // Can execute multiple tasks at a time in any order.
  PARALLEL,

  // Executes one task at a time in posting order. The sequence’s priority is
  // equivalent to the highest priority pending task in the sequence.
  SEQUENCED,

  // Executes one task at a time on a single thread in posting order.
  SINGLE_THREADED,
};

and TaskTraits is from https://cs.chromium.org/chromium/src/base/task_scheduler/task_traits.h?rcl=1475652159&l=79 which can specify things like TaskPriority of tasks posted to this TaskRunner, etc. (TaskTraits can also easily be expanded if there is eventually another trait you believe tasks should be differentiated on).

Comment 8 by gab@chromium.org, Oct 5 2016

Blockedon: 633389
Cc: robliao@chromium.org fdoray@chromium.org
Attaching to our tracker bug to remember the potential for collaboration here.
#6: Agreed that SequencedTaskRunner is probably the minimum guarantee we need. I think we ended up on SingleThreadTaskRunner mostly because there are a couple of well known magic threads in the renderer process that code wants to run on. I don't see why we couldn't relax this requirement.
Status: Available (was: Untriaged)
Project Member

Comment 11 by sheriffbot@chromium.org, Oct 5 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
Status: WontFix (was: Untriaged)
This was recently slightly improved by https://chromium-review.googlesource.com/c/chromium/src/+/676174. I think we can close this bug and reopen if this function actually shows up in profiles.

Sign in to add a comment