New issue
Advanced search Search tips

Issue 875030 link

Starred by 5 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 895591



Sign in to add a comment

Consider more intuitive naming for mojo things

Project Member Reported by roc...@chromium.org, Aug 16

Issue description

This is a pretty low priority thing just because we have *so much* code landed by now and in spite of naming there is a reasonably widespread volume of tribal knowledge at this point, but in my experience nearly every newcomer to Mojo is confused by our terminology and syntax. I'm filing this bug to at least have a foundation for discussing possible future changes, even if the conclusion is that such changes are unlikely to be worthwhile.

In particular, I'm thinking about interface endpoint terminology and mojom syntax. The term "InterfacePtr" makes sense if you squint -- it sort of functions like a pointer to a remote impl, but... not really, because reality is more complicated than that. If anything it functions more as a remote *sequenced task runner* but which can only call methods on a specific object.

The term "InterfaceRequest" is pretty ambiguous too. Nothing about these names (Ptr and Request) conveys that they are necessarily intertwined, that they comprise a self-contained end-to-end connection between a concrete object and (one of) its client(s).

In addition, for an interface T, the mojom syntax of T == InterfacePtr and T& == InterfaceRequest has caused quite a bit of confusion. It does not intuitively convey that one is sending a usable interface proxy while the other is sending an endpoint that should be bound to something.

Of course all of this can be remedied with documentation, repetition, trial-and-error, etc, but it might be possible to just make this stuff more intuitive. Straw proposal:

Rename some C++ types:

- mojo::InterfacePtr<T> becomes mojo::Sender<T>
- mojo::InterfaceRequest<T> becomes mojo::Receiver<T>
- mojo::Binding<T> seems fine as-is -- it can bind to a mojo::Receiver<T>.

Change mojom syntax to look more like the templated types:

- T becomes sender<T>
- T& becomes receiver<T>

So in total we have old stuff:

// mojom
interface FooBinder {
  BindFoo(Foo& request, FooClient client);
};

// C++
class FooBinder {
 virtual void BindFoo(FooRequest request, FooPtr client);
};


which becomes:

// mojom
interface FooBinder {
  BindFoo(receiver<Foo> receiver, sender<FooClient> client);
};

// C++
class FooBinder {
  virtual void BindFoo(FooReceiver receiver, FooClientSender client);
};

And finally, it is also very much unclear that mojo::MakeRequest creates an end-to-end object (i.e. a pipe). There was a concerted effort to "bury the pipes" in early Mojo days, that is, to hide the detail that message pipes are involved in interface acquisition and usage. I'm not sure this is beneficial, because we end up having to explain behavior in terms of pipe communication anyway (in particular consider how associated interfaces work). We should consider something like:

  template <typename T>
  mojo::Receiver<T> MakeConnection(mojo::Sender<T>* sender);

or possibly even

  template <typename T>
  std::tuple<mojo::Sender<T>, mojo::Receiver<T>> mojo::MakeConnection();

which would make code read like (assuming C++17):

  auto [foo_sender, foo_receiver] = mojo::MakeConnection<mojom::Foo>();
  auto [foo_client_sender, foo_client_receiver] =
      mojo::MakeConnection<mojom::FooClient>();
  binder->BindFoo(std::move(foo_receiver), std::move(foo_client_sender));

I think the asymmetry of the existing MakeRequest signature is actually surprisingly confusing for people, and it's not at all clear that it's creating a self-contained thing with two equally important, intertwined endpoints, and no other magical side effects. Whether we called it MakePipe, MakeConnection, MakeChannel, MakeIntertwinedSenderAndReceiver, or whatever, is less important to me than the general desire to call it something which represents the complete essence of what's being created.

As one additional nugget of food for thought, there have been repeated complaints that it's difficult to read code and know that it might be sending IPC. I'm a little mixed in response to that, but I do sympathize somewhat. If a Ptr becomes a Sender, then it might not be unreasonable to generate C++ method names with a Send_ prefix, so e.g. "Send_BindFoo()" above.

 
Labels: Mojoimprovements
One more term that I find confusing (though isn't a type) is "user", since it has no relation to the OS level primitive nor the Chrome-level primitive.
Yeah, I agree that it's confusing for that reason. However, the purpose of
that component of a service instance's identity is indeed to allow for
instance isolation based on the user's identity. Right now we happen to use
BrowserContext to model that user identity.

For full context here, let's recap the 3 (should be 4) components of
instance identity and their purpose:

   - The service name -- obvious
   - A qualifier which allows for user identity-based isolation ("user"
   today)
   - A qualifier which allows for arbitrary partitioning of instances even
   within a single "user" context (called "instance" today)
   - A GUID identifying the instance uniquely across all time and space
   (does not yet exist on Identity)

I think I would like to rename "instance" to "partition", but I still don't
have a better name for "user". Suggestions?
I've been mentally calling "user" the "instance identifier", so then if we renamed "instance" to "partition" that would make room for recycling the "instance" nomenclature.
Having both "parition" and "instance" seems like it would be even more
confusing.
I suppose so. Maybe "instance group" and "instance" ?
Owner: roc...@chromium.org
Status: Assigned (was: Available)
Well, the reason I don't like "instance" is that, to me, it does imply a certain uniqueness that is not guaranteed. Also the most common "instance" name is the empty string. That's why I was thinking "parition" for that one, to me the terrm feels more like an optional qualifier. I don't know how widely that perception varies from person to person.

In any case, I really like "instance group" as a replacement for "user" - it is easy enough to explain that all service instances belong to an instance group, and that by default their connections are limited to other service instances in the same group. That is effectively what the user component models anyway.

How about (name, instance_group, partition)? And I also plan on adding a separate guid as an informational field only, so e.g. ServiceManagerListeners can track information about unique instances even if their identities would otherwise overlap.
I guess partition and instance group kind of feel like synonyms though. :/
I agree that "instance" has an aspect of uniqueness that shouldn't always be implied. But the empty string "instance" makes some sense to me - it's unspecified so you get the default behavior of not having a specific one (and so it's really controlled by the group).

I kind of like the rhythm of "X group" and "X" -- so maybe "partition group" and "partition"? But I don't think that "partition" and "instance group" are too synonymous, especially if we can convey this clearly in documentation.

dcheng@: Any thoughts?
Blockedon: 895591
I actually gave it some more thought and I think your original suggestion
of "partition" and "instance" is the best compromise. I filed  bug 895591  to
cover this specific issue btw since this is more of a meta bug.
Owner: rockot@google.com

Sign in to add a comment