New issue
Advanced search Search tips

Issue 674301 link

Starred by 1 user
Status: Fixed
Owner:
dtapu...@chromium.org
Closed: Mar 2017
Cc:
majidvp@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Consider dispatching multiple events to main in a single task

Project Member Reported by majidvp@chromium.org, Dec 14 2016 
At the moment MainThreadEventQueue sends multiple inflight events to main in *separate* posted tasks. It seems we should be able to post multiple events in a single task and avoid the task overhead.

Looking at a perf flamechart it appears most of the MainThreadEventQueue::HandleEvent cost is just posting the tasks [1]. Hoever, I am not sure how often we have more than a single inflight event.


[1] https://screenshot.googleplex.com/bopw4wQP4AP

FYI, the perf trace was obtained from smoothness.key_silk_cases/.*inbox_app.html..* benchmark

Comment 1 by dtapu...@chromium.org, Dec 15 2016 

The problem is that some ipcs need to be kept in sync too.  Specifically the text input composition ipcs messages as well. If we could somehow put those in the main thread even queue then all would. E good.

Comment 2 by dtapu...@chromium.org, Dec 15 2016

Status: Assigned (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 31 2017 

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

commit 8ba8d7142ef4449775d3d75b6f17b80d24d07f77
Author: dtapuska <dtapuska@chromium.org>
Date: Fri Mar 31 15:51:25 2017

Teach main thread event queue about closures.

This allows the rAF signal to execute all pending input right away
instead of waiting for a message pump to process any input IPCs.

This simplifies the state that we don't need to execute a post task
for every event we have to process. At delivery time we can just take
them all from the queue (except for the rAF aligned events) and execute
them.

BUG= 674301 
TBR=creis@chromium.org

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

[modify] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/common/BUILD.gn
[delete] https://crrev.com/66b6e71817250892d6261bf46ca8a9429ed9b27a/content/common/input/web_input_event_queue.h
[modify] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/BUILD.gn
[modify] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/input/input_event_filter.cc
[modify] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/input/input_event_filter.h
[modify] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/input/input_handler_manager.cc
[modify] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/input/input_handler_manager.h
[modify] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/input/input_handler_manager_client.h
[modify] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/input/main_thread_event_queue.cc
[modify] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/input/main_thread_event_queue.h
[add] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/input/main_thread_event_queue_task.h
[add] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/input/main_thread_event_queue_task_list.cc
[add] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/input/main_thread_event_queue_task_list.h
[modify] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/input/main_thread_event_queue_unittest.cc
[modify] https://crrev.com/8ba8d7142ef4449775d3d75b6f17b80d24d07f77/content/renderer/render_frame_impl.cc

Comment 4 by dtapu...@chromium.org, Mar 31 2017

Status: Fixed (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 4 2017 

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

commit b1e592331aa12efbde84280e8b4e9248a7360c9d
Author: dtapuska <dtapuska@chromium.org>
Date: Tue Apr 04 21:21:56 2017

Allow events in queue to coalesce while executing other events.

The code removed all events from the queue at the start which is flawed
because a slow handler could cause another event not to be coalesced.
This was explicitly tested in the browser test that is failing sometimes
now.

BUG= 707491 , 674301 

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

[modify] https://crrev.com/b1e592331aa12efbde84280e8b4e9248a7360c9d/content/browser/renderer_host/input/main_thread_event_queue_browsertest.cc
[modify] https://crrev.com/b1e592331aa12efbde84280e8b4e9248a7360c9d/content/renderer/input/main_thread_event_queue.cc
[modify] https://crrev.com/b1e592331aa12efbde84280e8b4e9248a7360c9d/content/renderer/input/main_thread_event_queue_task_list.h
►

Sign in to add a comment