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

Issue 590495 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
please use my google.com address
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

Reduce latency of Mojo IPC dispatch

Project Member Reported by roc...@chromium.org, Feb 28 2016

Issue description

AsyncWaiter and HandleWatcher usage is relatively inefficient without special MessagePump support. This means that in practice we end up doing hops to some background thread with MessagePumpMojo waiting on handles, which is itself also inefficient.

We should be able to guarantee that any handle event (such as a handle becoming readable) results in service request dispatch after at most one thread hop in the case of thread-to-thread pipe communication (hopping directly from the writing thread to the service's binding thread) or an incoming IPC (hopping directly from the IO thread to the service's binding thread).

For same-thread pipe communication we should be able to guarantee that WriteMessage is roughly equivalent to a PostTask dispatching the service request on the current thread.
 

Comment 1 by yzshen@chromium.org, Feb 28 2016

Cc: yzshen@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 2 2016

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

commit 49b69e6b27a12426ba56b6b5271ccec9b54c94c2
Author: rockot <rockot@chromium.org>
Date: Wed Mar 02 03:46:37 2016

[mojo-edk] Add MojoWatch and MojoCancelWatch APIs

This adds MojoWatch() and MojoCancelWatch() APIs to support
efficient asynchronous handle event notifications.

BUG= 590495 

Review URL: https://codereview.chromium.org/1748503002

Cr-Commit-Position: refs/heads/master@{#378677}

[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/embedder/entrypoints.cc
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/BUILD.gn
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/awakable_list.cc
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/awakable_list.h
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/core.cc
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/core.h
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/data_pipe_consumer_dispatcher.cc
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/data_pipe_consumer_dispatcher.h
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/data_pipe_producer_dispatcher.cc
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/data_pipe_producer_dispatcher.h
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/dispatcher.cc
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/dispatcher.h
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/message_pipe_dispatcher.cc
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/message_pipe_dispatcher.h
[add] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/request_context.cc
[add] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/request_context.h
[add] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/watch_unittest.cc
[add] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/watcher.cc
[add] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/watcher.h
[add] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/watcher_set.cc
[add] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/system/watcher_set.h
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/test/mojo_test_base.cc
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/edk/test/mojo_test_base.h
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/mojo_edk.gyp
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/mojo_edk_tests.gyp
[modify] https://crrev.com/49b69e6b27a12426ba56b6b5271ccec9b54c94c2/mojo/public/c/system/functions.h

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 5 2016

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

commit d06373e7cd8b4ad725ed5c64c958f2de13585add
Author: rockot <rockot@chromium.org>
Date: Fri Mar 04 23:59:50 2016

[mojo-bindings] Use Watch API instead of MessagePumpMojo

This removes the C++ bindings dependency on MessagePumpMojo,
consuming the new Watch API instead.

For convenience a new mojo::Watcher is added to the public
Mojo C++ API library, and this is used by Connector.

BUG= 590495 
R=yzshen@chromium.org
TBR=blundell@chromium.org for rename affecting components/message_port.gypi
TBR=jam@chromium.org - added a missing header to new url tests

Review URL: https://codereview.chromium.org/1759783003

Cr-Commit-Position: refs/heads/master@{#379402}

[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/components/message_port.gypi
[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/mojo_edk_tests.gyp
[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/mojo_public.gyp
[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/mojo_variables.gypi
[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/public/cpp/bindings/BUILD.gn
[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/public/cpp/bindings/lib/connector.cc
[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/public/cpp/bindings/lib/connector.h
[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/public/cpp/system/BUILD.gn
[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/public/cpp/system/tests/BUILD.gn
[add] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/public/cpp/system/tests/watcher_unittest.cc
[add] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/public/cpp/system/watcher.cc
[add] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/public/cpp/system/watcher.h
[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/shell/public/cpp/BUILD.gn
[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/shell/public/cpp/lib/application_runner.cc
[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/mojo/shell/tests/connect/connect_test_app.cc
[modify] https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add/url/mojo/url_gurl_struct_traits_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 5 2016

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

commit 258f8f50d607b2b585bb18631efbe75fb5f01913
Author: rockot <rockot@chromium.org>
Date: Sat Mar 05 00:14:26 2016

Revert of [mojo-bindings] Use Watch API instead of MessagePumpMojo (patchset #10 id:180001 of https://codereview.chromium.org/1759783003/ )

Reason for revert:
Breaks iOS GN which is apparently a tree closer despite having no CQ coverage

Original issue's description:
> [mojo-bindings] Use Watch API instead of MessagePumpMojo
>
> This removes the C++ bindings dependency on MessagePumpMojo,
> consuming the new Watch API instead.
>
> For convenience a new mojo::Watcher is added to the public
> Mojo C++ API library, and this is used by Connector.
>
> BUG= 590495 
> R=yzshen@chromium.org
> TBR=blundell@chromium.org for rename affecting components/message_port.gypi
> TBR=jam@chromium.org - added a missing header to new url tests
>
> Committed: https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add
> Cr-Commit-Position: refs/heads/master@{#379402}

TBR=jam@chromium.org,yzshen@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 590495 

Review URL: https://codereview.chromium.org/1768443004

Cr-Commit-Position: refs/heads/master@{#379406}

[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/components/message_port.gypi
[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/mojo/mojo_edk_tests.gyp
[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/mojo/mojo_public.gyp
[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/mojo/mojo_variables.gypi
[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/mojo/public/cpp/bindings/BUILD.gn
[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/mojo/public/cpp/bindings/lib/connector.cc
[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/mojo/public/cpp/bindings/lib/connector.h
[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/mojo/public/cpp/system/BUILD.gn
[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/mojo/public/cpp/system/tests/BUILD.gn
[delete] https://crrev.com/00f0d6c9431fb913beeceb3db209c5bf47ffecbd/mojo/public/cpp/system/tests/watcher_unittest.cc
[delete] https://crrev.com/00f0d6c9431fb913beeceb3db209c5bf47ffecbd/mojo/public/cpp/system/watcher.cc
[delete] https://crrev.com/00f0d6c9431fb913beeceb3db209c5bf47ffecbd/mojo/public/cpp/system/watcher.h
[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/mojo/shell/public/cpp/BUILD.gn
[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/mojo/shell/public/cpp/lib/application_runner.cc
[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/mojo/shell/tests/connect/connect_test_app.cc
[modify] https://crrev.com/258f8f50d607b2b585bb18631efbe75fb5f01913/url/mojo/url_gurl_struct_traits_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 5 2016

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

commit 2cdb2f677c68f06b3bc54cd40f000ab961a09bbb
Author: rockot <rockot@chromium.org>
Date: Sat Mar 05 06:04:39 2016

[mojo-bindings] Use Watch API instead of MessagePumpMojo

This removes the C++ bindings dependency on MessagePumpMojo,
consuming the new Watch API instead.

For convenience a new mojo::Watcher is added to the public
Mojo C++ API library, and this is used by Connector.

BUG= 590495 
R=yzshen@chromium.org
TBR=blundell@chromium.org for rename affecting components/message_port.gypi
TBR=jam@chromium.org - added a missing header to new url tests

Committed: https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add
Cr-Commit-Position: refs/heads/master@{#379402}

Review URL: https://codereview.chromium.org/1759783003

Cr-Commit-Position: refs/heads/master@{#379463}

[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/components/BUILD.gn
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/components/message_port.gypi
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/mojo_edk_tests.gyp
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/mojo_public.gyp
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/mojo_variables.gypi
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/public/cpp/bindings/BUILD.gn
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/public/cpp/bindings/lib/connector.cc
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/public/cpp/bindings/lib/connector.h
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/public/cpp/system/BUILD.gn
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/public/cpp/system/tests/BUILD.gn
[add] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/public/cpp/system/tests/watcher_unittest.cc
[add] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/public/cpp/system/watcher.cc
[add] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/public/cpp/system/watcher.h
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/shell/public/cpp/BUILD.gn
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/shell/public/cpp/lib/application_runner.cc
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/mojo/shell/tests/connect/connect_test_app.cc
[modify] https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb/url/mojo/url_gurl_struct_traits_unittest.cc

This gets us around 20-25% improvement in ping-pong throughput when running mojo_public_bindings_perftests (which apparently does not have a build target in the ttr) The improvement is likely a combination of Watcher doing less work than HandleWatcher+MessagePumpMojo, along with lower latency from fewer thread hops.

One further improvement to latency would be in cases where a binding lives on the IO thread and a message arrives for it from out-of-process. In this case reentry is not an issue and it would be safe to dispatch the incoming message synchronously. We'd need to plumb some additional thread/IPC details into the mojo layer to support this.
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 6 2016

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

commit 554355993ee16fa592a450f2dd00729b69988dc7
Author: Ken Rockot <rockot@chromium.org>
Date: Sun Mar 06 17:29:45 2016

Revert of [mojo-bindings] Use Watch API instead of MessagePumpMojo (patchset #12 id:210001 of https://codereview.chromium.org/1759783003/ )

Reason for revert:
one more time. mojo_app_tests flake

Original issue's description:
> [mojo-bindings] Use Watch API instead of MessagePumpMojo
>
> This removes the C++ bindings dependency on MessagePumpMojo,
> consuming the new Watch API instead.
>
> For convenience a new mojo::Watcher is added to the public
> Mojo C++ API library, and this is used by Connector.
>
> BUG= 590495 
> R=yzshen@chromium.org
> TBR=blundell@chromium.org for rename affecting components/message_port.gypi
> TBR=jam@chromium.org - added a missing header to new url tests
>
> Committed: https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add
> Cr-Commit-Position: refs/heads/master@{#379402}
>
> Committed: https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb
> Cr-Commit-Position: refs/heads/master@{#379463}

TBR=jam@chromium.org,yzshen@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 590495 

Review URL: https://codereview.chromium.org/1772503002 .

Cr-Commit-Position: refs/heads/master@{#379492}

[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/components/BUILD.gn
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/components/message_port.gypi
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/mojo/mojo_edk_tests.gyp
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/mojo/mojo_public.gyp
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/mojo/mojo_variables.gypi
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/mojo/public/cpp/bindings/BUILD.gn
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/mojo/public/cpp/bindings/lib/connector.cc
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/mojo/public/cpp/bindings/lib/connector.h
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/mojo/public/cpp/system/BUILD.gn
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/mojo/public/cpp/system/tests/BUILD.gn
[delete] https://crrev.com/1ae3eca5e65b03da0710a1ff22f75af693ba1fdf/mojo/public/cpp/system/tests/watcher_unittest.cc
[delete] https://crrev.com/1ae3eca5e65b03da0710a1ff22f75af693ba1fdf/mojo/public/cpp/system/watcher.cc
[delete] https://crrev.com/1ae3eca5e65b03da0710a1ff22f75af693ba1fdf/mojo/public/cpp/system/watcher.h
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/mojo/shell/public/cpp/BUILD.gn
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/mojo/shell/public/cpp/lib/application_runner.cc
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/mojo/shell/tests/connect/connect_test_app.cc
[modify] https://crrev.com/554355993ee16fa592a450f2dd00729b69988dc7/url/mojo/url_gurl_struct_traits_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 7 2016

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

commit e5d711ec0a8b327dea474d5c83dec0ae5f2c545e
Author: rockot <rockot@chromium.org>
Date: Mon Mar 07 02:36:52 2016

[mojo-bindings] Use Watch API instead of MessagePumpMojo

This removes the C++ bindings dependency on MessagePumpMojo,
consuming the new Watch API instead.

For convenience a new mojo::Watcher is added to the public
Mojo C++ API library, and this is used by Connector.

BUG= 590495 
R=yzshen@chromium.org
TBR=blundell@chromium.org for rename affecting components/message_port.gypi
TBR=jam@chromium.org - added a missing header to new url tests

Committed: https://crrev.com/d06373e7cd8b4ad725ed5c64c958f2de13585add
Cr-Commit-Position: refs/heads/master@{#379402}

Committed: https://crrev.com/2cdb2f677c68f06b3bc54cd40f000ab961a09bbb
Cr-Commit-Position: refs/heads/master@{#379463}

Review URL: https://codereview.chromium.org/1759783003

Cr-Commit-Position: refs/heads/master@{#379508}

[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/components/BUILD.gn
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/components/message_port.gypi
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/mojo_edk_tests.gyp
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/mojo_public.gyp
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/mojo_variables.gypi
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/public/cpp/bindings/BUILD.gn
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/public/cpp/bindings/lib/connector.cc
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/public/cpp/bindings/lib/connector.h
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/public/cpp/system/BUILD.gn
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/public/cpp/system/tests/BUILD.gn
[add] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/public/cpp/system/tests/watcher_unittest.cc
[add] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/public/cpp/system/watcher.cc
[add] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/public/cpp/system/watcher.h
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/shell/public/cpp/BUILD.gn
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/shell/public/cpp/lib/application_runner.cc
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/mojo/shell/tests/connect/connect_test_app.cc
[modify] https://crrev.com/e5d711ec0a8b327dea474d5c83dec0ae5f2c545e/url/mojo/url_gurl_struct_traits_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 7 2016

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

commit c8e9c39cfb8bef5e49c2e2b1437267eb165e2dae
Author: amistry <amistry@chromium.org>
Date: Mon Mar 07 05:27:19 2016

[mojo-edk] Cache the context thread-local in the RequestContext.

On MojoBindingsPerftest.InProcessPingPong, RequestContext's constructor
and destructor combined consume ~22% of CPU time. The reason is that
accessing a LazyInstance compiles down to a number of atomic
operations, which are unnecessarily done twice in each of the
constructor and destructor. Since the LazyInstance is leaky, it is safe
to read the pointer once and cache it every time we need to access the
thread-local. This change results in ~20% improvement in
MojoBindingsPerftest.InProcessPingPong performance.

The CPU usage is smaller but still noticable in MojoE2EPerftest with
small messages. With a batch size of 1, this consumes ~4.5% of CPU time,
rising to ~9% with a batch size of 10.

BUG= 590495 

Review URL: https://codereview.chromium.org/1767003002

Cr-Commit-Position: refs/heads/master@{#379513}

[modify] https://crrev.com/c8e9c39cfb8bef5e49c2e2b1437267eb165e2dae/mojo/edk/system/request_context.cc
[modify] https://crrev.com/c8e9c39cfb8bef5e49c2e2b1437267eb165e2dae/mojo/edk/system/request_context.h

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 16 2016

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

commit 51fec73df69c66596985a9037bc0665529327507
Author: yzshen <yzshen@chromium.org>
Date: Wed Mar 16 19:46:44 2016

Mojo C++ bindings: MultiplexRouter more aggressively dispatch queued messages.

This CL allows MultiplexRouter to dispatch multiple queued messages to clients
without returning to the message loop.

This way we may starve other tasks on the same thread, but we have been doing
the same thing in the non-associated-interface case (see
Connector::ReadAllAvailableMessages). So it is probably okay.

BUG= 590495 

Review URL: https://codereview.chromium.org/1808583003

Cr-Commit-Position: refs/heads/master@{#381514}

[modify] https://crrev.com/51fec73df69c66596985a9037bc0665529327507/mojo/public/cpp/bindings/lib/multiplex_router.cc
[modify] https://crrev.com/51fec73df69c66596985a9037bc0665529327507/mojo/public/cpp/bindings/lib/multiplex_router.h

Project Member

Comment 11 by bugdroid1@chromium.org, Mar 17 2016

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

commit 6aca885ca93bb6194e67be3c62ac409745595e3c
Author: rockot <rockot@chromium.org>
Date: Thu Mar 17 19:10:48 2016

[mojo-edk] Expose notification source to MojoWatch callbacks

This adds a flags argument to watch callbacks and exposes a flag
to indicate that the callback was invoked as a result of some
external process event (i.e. an incoming EDK IPC message). The
public C++ Watcher implementation is updated to take advantage
of this, effectively allowing us to dispatch to Mojo bindings
synchronously when they live on the IO thread and are servicing
messages from out-of-process.

BUG= 590495 

Review URL: https://codereview.chromium.org/1811433002

Cr-Commit-Position: refs/heads/master@{#381767}

[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/edk/system/core.cc
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/edk/system/data_pipe_consumer_dispatcher.cc
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/edk/system/data_pipe_producer_dispatcher.cc
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/edk/system/message_pipe_dispatcher.cc
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/edk/system/node_channel.cc
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/edk/system/request_context.cc
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/edk/system/request_context.h
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/edk/system/wait_set_dispatcher_unittest.cc
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/edk/system/watch_unittest.cc
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/edk/system/watcher.cc
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/edk/system/watcher.h
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/public/c/system/functions.h
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/public/c/system/types.h
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/public/cpp/system/watcher.cc
[modify] https://crrev.com/6aca885ca93bb6194e67be3c62ac409745595e3c/mojo/public/cpp/system/watcher.h

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 29 2016

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

commit 596231c83a488a54e994a43417e617c864730b3a
Author: rockot <rockot@chromium.org>
Date: Fri Apr 29 18:46:45 2016

Mojo: Use new message APIs to reduce copying

- Adds new public C++ APIs for scoped message objects
- Updates the bindings to use new Write/ReadMessage APIs
- Updates ChannelMojo to use new WriteMessage API

This results in one fewer copy on either end of every Mojo IPC,
whether it's over ChannelMojo or an arbitrary message pipe.

BUG= 590495 
R=yzshen@chromium.org

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

[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/ipc/mojo/ipc_message_pipe_reader.cc
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/mojo_public.gyp
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/c/system/message_pipe.h
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/BUILD.gn
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/lib/buffer.h
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/lib/connector.cc
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/lib/fixed_buffer.cc
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/lib/fixed_buffer.h
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/lib/message.cc
[add] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/lib/message_buffer.cc
[add] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/lib/message_buffer.h
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/lib/native_struct_serialization.h
[delete] https://crrev.com/341939b904f5d6079e3c13f7678a242359275263/mojo/public/cpp/bindings/lib/pickle_buffer.cc
[delete] https://crrev.com/341939b904f5d6079e3c13f7678a242359275263/mojo/public/cpp/bindings/lib/pickle_buffer.h
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/lib/router.cc
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/message.h
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/tests/connector_unittest.cc
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/bindings/tests/validation_unittest.cc
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/system/BUILD.gn
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/system/handle.h
[add] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/system/message.cc
[add] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/system/message.h
[modify] https://crrev.com/596231c83a488a54e994a43417e617c864730b3a/mojo/public/cpp/system/message_pipe.h

Status: Fixed (was: Assigned)
Let's track further bugs/improvements individually

Sign in to add a comment