Re: [g-a-devel] AT-SPI caching and D-Bus usage

[Piñeiro <apinheiro igalia com>]

First thanks for the detailed email and explanations. Some comments below.

On 10/07/2013 09:45 PM, Mike Gorse wrote:
> AT-SPI was originally designed around CORBA, specifically ORBit. Its
> use led to a large amount of inter-process communication. Method calls
> in ORBit were fairly quick, so this was not a huge problem, although
> that isn't to say that there were never performance issues. However,
> ORBit was deprecated for GNOME 3, and Codethink undertook an
> investigation of the feasibility of porting AT-SPi to D-Bus. Note that
> the D-Bus libraries were not really designed to be used in cases where
> a large amount of synchronous method calls are needed. Nevertheless,
> Codethink undertook an investigation of the feasibility of porting
> AT-SPI to D-Bus. Their main conclusions were that, although D-Bus
> method calls are slower than the equivalent CORBA calls, a lot of
> AT-SPI traffic generated by Orca comes from a handful of method
> calls--calls to fetch an object's name, parent, children, or state
> set, for instance. If these data could be cached, then a significant
> amount of traffic would become unnecessary.

Unfourtunately there isn't  any available metric about how much it is
gained by caching the most usual data. In that sense, I feel that they
made the usual mistake of doing early optimization, and basing the
design on that, without a real metric justifying it. In that sense,
assuming that most of the traffic cames from name/description/role is
only half-true, because as you are saying, in several cases Orca is
asking for new data when something change. But as I'm saying this is
just a feeling.

> This led to the current design, where some data are cached, and the
> default assumption is that applications will send notifications when
> an object's name, description, state, or children are changed. 

Well, fwiw, this assumption is well-based, because those notifications
are not only needed to update the cache. Orca also need them in order to
now when/how expose new data to the user.

> Calls requesting other data, such as attributes or screen coordinates,
> result in method calls. This design has a few problems:
>
> - It relies on applications sending notifications any time an object's
> states or children change. If an application fails to do this, then
> Orca will see stale data, unless it has instructed AT-SPI not to cache
> certain kinds of data for the given application. It is sometimes
> difficult for a toolkit implementor to send notifications for every
> such change, and the result is that AT-SPI2 is made fragile in a way
> that was not previouisly the case.

Although it is true that the cache is fragile if some notifications are
missing, as I said, it is also true that those notifications are needed.
In any case, one additional problem is that it is hard (unless I miss
something) to test at-spi without the cache (that would help on the bug
triagging), and that it is not only fragile due the lack of
notifications. From my experience cache also depends on having a
listener to a specific event (see bug 707578). So I would like to make a
question: it is really possible to run at-spi without caching? I know
that that will lead to syncronous calls everywhere, but it would be good
to know in order to investigate if caching is really improving at-spi2
performance or not.

> - Not all data that an AT may need is cached.

In general, as far as I see, the problem is that it was tried to cache
the "more relevant" data, and the subset defined is really small. But
increasing the subset, let's say the whole accessible, would be really
complex, unpractical, and would make the cache even more fragile. In
general reducing the subset (removing cache) as you propose later with
an alternative seems to make sense. FWIW, that was also suggested by
Frederik on one of our ATK/AT-SPI hackfests (he had a lot of doubts
about the gain of the cache).

> For instance, when Orca sees an event, it wants to look at the
> attributes for the accessible that triggered the event, in order to
> determine which toolkit sent it, in order to decide which script to
> use. Attributes are not cached, so this means that Orca always makes a
> round-trip D-Bus call to retrieve the attributes of the object that
> just sent the event. Similarly, when building a flat review context,
> it wants to know the extents of various accessibles on the screen, and
> extents are not cached, so a significant number of round-trip calls
> are needed in order to fetch extents for many different objects. One
> solution would be to add more things to the data that AT-SPI caches
> (ie, http://bugzilla.gnome.org/show_bug.cgi?id=649771), but this has
> the downside of making AT-SPI even more reliant on applications to
> send events when various things change, introducing the possibility
> for new bugs.

Yes, increasing the set of properties cached has (imho) too many
downsides, as I already said.

> - Sometimes, it is necessary to avoid synchronous D-Bus calls
> altogether (see https://bugzilla.gnome.org/show_bug.cgi?id=708387).
>
> I think that a good solution would be two-fold:
>
> - Allow an event listener to request particular data be sent along
> with an event. For instance, Orca could request that an object's
> attributes be sent. The attributes would be cached probably for the
> duration of the event callback. While we're modifying the API for
> event listeners, it would be good to take a look at
> https://bugzilla.gnome.org/show_bug.cgi?id=640440.

Well, I need joanie input there, but the big amount of object's
attributes currently available in some implementations (like firefox)
should start to be reduced, replaced for proper API. Although it is true
that this is still needed for text attributes, on each (?) caret move.
So in that sense, the attributes that would be ideally needed will be
text attributes, that are tied to start_offset/end_offset. That would
make specifying the particular data in just one event somewhat tricky.
Or we can just assume that text attributes would need a round-trip D-BUS
call.

> - Add a function to allow various data to be fetched in one go. It
> would take a callback to be called whtn the data is ready. For
> instance, perhaps orca would want to know the roles, states, and
> extents of all (non-transient) objects descending from a particular
> accessible. 

Shouldn't be easier to just send the full accessible(s)? Except those
ones with a lot of text, the accessible shouldn't be really big.

> All of this data would be stored in a cache that would be valid for
> the duration of the callback(?).

When you say cache, do you mean accessible objects? How can we ensure
that the data will be valid for the duration of the callback? As far as
I understand this proposal, this procedure tries to add support for
async calls. And the callback will be called at the client side. So if
the server side is not blocked, what would prevent the data to have
changed? In any case, this issue is innerent with async calls.

> If, say, atspi_accessible_get_attributes is called from within the
> callback and the attributes have been pre-fetched, then return the
> attributes we have cached rather than make another D-Bus call. 

Well yes, something would be needed on the callback to avoid doing new
D-BUS calls.

> In the long term, this might be able to replace the current caching
> mechanism. Ie, something like:
>
> typedef void (*AtspiQueryCallback) (void *user_data)
>
> void atspi_query_accessible (AtspiAccessible *obj,
>                              const gchar **properties,
>                              AtspiQueryCallback *cb,
>                              void *user_data,
>                              gboolean allow_sync,
>                              gboolean include_descendants,
>                              GError **error);

Shouldn't the callback include the accessible as one of the possible
parameters? As far as I understanding this call, this would allow to get
an object in an asynchronous call. As soon as the accessible is
available the callback would be called, and the client could use it. In
that case, the very same accessible is relevant there.

> If allow_sync is FALSE, then AT-SPI will throw an exception rather
> than make a synchronous call. This could be used when it is desirable
> to guarantee that no synchronous calls are made (probably anything
> running inside gnome-shell would want this, since otherwise it can
> deadlock if it queries an application which is in turn making a
> non-AT-SPI-related D-Bus call to gnome-shell).

I don't get this. As far as I understand the proposal, the idea of this
new API is providing an async call to get data. So when that sync call
will be called ?

> I'm not sure if having an "include descendants" parameter is best, or 
> if something more fine-grained would be better (perhaps an enum
> allowing just the accessible, the accesible and its children, or the
> accessible and all non-transient descendants?)

Yes, I agree that an enum to fine-grain which descendants are needed
would be better.

> Also, maybe we want a parameter to specify the starting point--it
> might be useful to be able to specify an ancestor with a given role as
> a starting point? This gets into whether it might be useful to take a
> string parameter specifying a predicate. It may be worth investigating
> whether xpath would be helpful / whether there are libraries that
> would be useful in terms of parsing it.

Well, if I understand this properly, this goes a little to the
Collection interface, in the sense of providing a match-filter, and
getting only the accessible we are interested on. Are you thinking on
something different?

BR

-- 
Alejandro Piñeiro Iglesias