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

Issue 836092 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 17
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature

Blocking:
issue 836093
issue 836102
issue 836107


Participants' hotlists:
ML-Service


Sign in to add a comment

ML Service: Bootstrap Mojo service over D-Bus

Project Member Reported by amoylan@chromium.org, Apr 24 2018

Issue description

Make the ML service daemon handle incoming D-Bus requests to bootstrap a Mojo connection. It should bind the pipe passed in with the request to an instance of ModelProvider interface (can be stub impl. of ModelProvider).
 
Blocking: 836093
Blocking: -782499
Blocking: 836102
Blocking: 836107
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/c4ccd288e863f84f0415465f1c804f5a82ff774a

commit c4ccd288e863f84f0415465f1c804f5a82ff774a
Author: Andrew Moylan <amoylan@chromium.org>
Date: Wed Jun 13 04:50:42 2018

Add ML service constant for Mojo pipe token

As part of D-Bus->Mojo bootstrapping, Chromium will send a Mojo
invitation to the ML service daemon with an attached pipe, via the
BootstrapMojoConnection D-Bus method. The daemon and Chromium need to
agree on the name (token).

BUG= chromium:836092 
TEST=local build

Change-Id: I3404cab42aadb71a5af5be4207a2fa744e6ba985
Reviewed-on: https://chromium-review.googlesource.com/1092393
Commit-Ready: Andrew Moylan <amoylan@chromium.org>
Tested-by: Andrew Moylan <amoylan@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/c4ccd288e863f84f0415465f1c804f5a82ff774a/dbus/service_constants.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/94f8ebb6f87abb7a2c180bc81856c97d6a835cdb

commit 94f8ebb6f87abb7a2c180bc81856c97d6a835cdb
Author: Andrew Moylan <amoylan@chromium.org>
Date: Wed Jul 04 05:31:00 2018

libchromeos-use-flags: Add ml_service USE flag

Will be used to conditionally enable MachineLearningService Chrome flag.

BUG= chromium:836092 ,  chromium:782499 
TEST=manual

Change-Id: I1ba02350b6787fe24b507b03e6b1ff003e15892d
Reviewed-on: https://chromium-review.googlesource.com/1123977
Commit-Ready: Andrew Moylan <amoylan@chromium.org>
Tested-by: Andrew Moylan <amoylan@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[rename] https://crrev.com/94f8ebb6f87abb7a2c180bc81856c97d6a835cdb/chromeos-base/libchromeos-use-flags/libchromeos-use-flags-0.0.1-r53.ebuild
[modify] https://crrev.com/94f8ebb6f87abb7a2c180bc81856c97d6a835cdb/chromeos-base/libchromeos-use-flags/libchromeos-use-flags-0.0.1.ebuild

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/359f23e5ab10d2cde1f4322f3f4bccd391f93184

commit 359f23e5ab10d2cde1f4322f3f4bccd391f93184
Author: Andrew Moylan <amoylan@chromium.org>
Date: Mon Jul 09 02:28:28 2018

login: Enable ML service Chrome flag per USE flag

CQ-DEPEND=CL:1123977
BUG= chromium:836092 ,  chromium:782499 
TEST=manual

Change-Id: Ie48f5af127626bf019cf0679d3191a9af76b1fa6
Reviewed-on: https://chromium-review.googlesource.com/1123971
Commit-Ready: Andrew Moylan <amoylan@chromium.org>
Tested-by: Andrew Moylan <amoylan@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/359f23e5ab10d2cde1f4322f3f4bccd391f93184/login_manager/chrome_setup.cc

Cc: napper@chromium.org claudiomagni@chromium.org martis@chromium.org
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 16

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

commit 5ced600fb66be67bd35dbe83022662c82daa090f
Author: Andrew Moylan <amoylan@chromium.org>
Date: Mon Jul 16 01:24:09 2018

ML service: Implement D-Bus->Mojo bootstrap & add a thin client library

This CL adds logic for Chromium to bootstrap a Mojo connection to the
Chrome OS ML service daemon via D-Bus, and a very simple client library
for client code in Chromium to use to access the ML service.

1. Bootstrap
To connect to the daemon, Chromium prepares a Unix socket pair and sends
a Mojo invitation, including a primordial pipe handle, through the
socket. The file descriptor for the other end of the socket pair is
passed to the daemon as the argument of its BootstrapMojoConnection
D-Bus method. The daemon is expected to bind the primordial pipe to an
implementation of the MachineLearningService top-level Mojo interface.

The ML service daemon side of the above interaction is added in
crrev.com/c/1123663.

2. Thin client library
This CL adds chromeos::machine_learning::ServiceConnection which client
code in Chromium can use to access the ML service. This singleton class
automatically performs D-Bus->Mojo bootstrap as necessary, and exposes
a method clients can use to bind a ModelProviderPtr mojo interface and
thereby access the ML service.

This CL also brings the .mojom files specifying the ML service Mojo
interface under public/mojom. Previously we intended to access these via
third_party/cros_system_api, but Chromium will instead maintain its own
copy of these mojoms, which will be manually eventually-consistent with
the Chrome OS copies (under src/platform2/ml). We are switching to this
recommended approach for the reasons discussed in the following doc:
https://docs.google.com/document/d/1rX3xtR6ZioKZaO-4qBFJFqFypSX1p6gAvQ_MuYn64G8
A separate CL will remove the .mojoms under platform/system_api/mojo.

Testing:

How to automatically test that Chromium + the daemon successfully
perform the D-Bus -> Mojo bootstrap is not clear and is under discussion
(comments welcome). It may have to wait until there is a client in
Chromium calling the daemon, which could then have an end-to-end
autotest.

I manually tested this against a Chrome OS ML service built with
crrev.com/c/1123663 by adding some test calls to BootstrapMojoConnection
initiated from chrome_browser_main_chromeos.

Bug:  836091 ,  836092 ,  836107 
Change-Id: I3dbcaeb7a0af0bc09a56a98e68d9420cc1626d91
Reviewed-on: https://chromium-review.googlesource.com/1123964
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Andrew Moylan <amoylan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575193}
[modify] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chrome/common/chrome_features.cc
[modify] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chrome/common/chrome_features.h
[modify] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/BUILD.gn
[modify] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/dbus/dbus_clients_common.cc
[modify] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/dbus/dbus_clients_common.h
[modify] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/dbus/dbus_thread_manager.cc
[modify] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/dbus/dbus_thread_manager.h
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/dbus/fake_machine_learning_client.cc
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/dbus/fake_machine_learning_client.h
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/dbus/machine_learning_client.cc
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/dbus/machine_learning_client.h
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/services/machine_learning/DEPS
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/services/machine_learning/public/cpp/BUILD.gn
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/services/machine_learning/public/cpp/service_connection.cc
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/services/machine_learning/public/cpp/service_connection.h
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/services/machine_learning/public/mojom/BUILD.gn
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/services/machine_learning/public/mojom/OWNERS
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/services/machine_learning/public/mojom/graph_executor.mojom
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/services/machine_learning/public/mojom/machine_learning_service.mojom
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/services/machine_learning/public/mojom/model.mojom
[add] https://crrev.com/5ced600fb66be67bd35dbe83022662c82daa090f/chromeos/services/machine_learning/public/mojom/tensor.mojom

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/ff6be511d7be61934523db3537f0a919abd31c40

commit ff6be511d7be61934523db3537f0a919abd31c40
Author: Andrew Moylan <amoylan@chromium.org>
Date: Mon Jul 16 08:18:07 2018

ml: Add D-Bus->Mojo bootstrap with stub Mojo impl

This CL gives a proper implementation of the BootstrapMojoConnection
D-Bus method offered by the ML service daemon.

The caller of that method (Chromium) passes a file descriptor
representing a Unix socket. With this CL, the daemon reads a Mojo
invitation from the socket in order to connect the daemon's Mojo to
Chromium's Mojo process graph. It also extracts a primodial pipe
included by Chromium in the invitation and binds a (placeholder stub)
implementation of the new top-level Mojo interface
MachineLearningService.

The Chromium side of the above interaction is included in
crrev.com/c/1123964.

This CL also brings the .mojom files specifying the ML service Mojo
interface under platform2/ml. Previously these were under
platform/system_api/mojo, with the intent to share them with Chromium,
but Chromium will instead maintain its own eventually-consistent copy of
the mojoms. We are switching to this recommended approach for the
reasons discussed in the following doc:
https://docs.google.com/document/d/1rX3xtR6ZioKZaO-4qBFJFqFypSX1p6gAvQ_MuYn64G8
A separate CL will remove the .mojoms under platform/system_api/mojo.

Testing:

How to automatically test that Chromium + the daemon successfully
perform the D-Bus -> Mojo bootstrap is not clear and is under discussion
(comments welcome). It may have to wait until there is a client in
Chromium calling the daemon, which could then have an end-to-end
autotest.

I manually tested this against a Chromium built with crrev.com/c/1123964
and that also included some test calls to BootstrapMojoConnection
initiated from chrome_browser_main_chromeos.

Scenarios tested:
- Boot (OK for either service to be ready first)
- Sudden restart of UI (daemon should quit itself)
- Sudden restart of daemon (UI should reconnect)

BUG= chromium:836092 , chromium:836093 
TEST=Manual test per above. How to set up an automated integration test
     for the bootstrap is TBD (future CL).

Change-Id: Ied6309d9a51656dda1af01114b0f05fdb90bfd88
Reviewed-on: https://chromium-review.googlesource.com/1123663
Commit-Ready: Andrew Moylan <amoylan@chromium.org>
Tested-by: Andrew Moylan <amoylan@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[add] https://crrev.com/ff6be511d7be61934523db3537f0a919abd31c40/ml/mojom/machine_learning_service.mojom
[add] https://crrev.com/ff6be511d7be61934523db3537f0a919abd31c40/ml/mojom/model.mojom
[add] https://crrev.com/ff6be511d7be61934523db3537f0a919abd31c40/ml/machine_learning_service_impl.cc
[add] https://crrev.com/ff6be511d7be61934523db3537f0a919abd31c40/ml/machine_learning_service_impl.h
[add] https://crrev.com/ff6be511d7be61934523db3537f0a919abd31c40/ml/mojom_generator.gypi
[modify] https://crrev.com/ff6be511d7be61934523db3537f0a919abd31c40/ml/daemon.h
[modify] https://crrev.com/ff6be511d7be61934523db3537f0a919abd31c40/ml/main.cc
[modify] https://crrev.com/ff6be511d7be61934523db3537f0a919abd31c40/ml/daemon.cc
[add] https://crrev.com/ff6be511d7be61934523db3537f0a919abd31c40/ml/mojom/tensor.mojom
[modify] https://crrev.com/ff6be511d7be61934523db3537f0a919abd31c40/ml/ml.gyp
[add] https://crrev.com/ff6be511d7be61934523db3537f0a919abd31c40/ml/mojom/graph_executor.mojom

Status: Fixed (was: Started)

Sign in to add a comment