Beware of WebAPI property accesses

[David Bruant]

Hi,

I've noticed that there is a perf race recently on Gaia. I've recently
found out that in some cases, getting properties on WebAPIs blocks on an
IPC message (from the app to the parent process). The idea that getting
a property blocks on an IPC message defeats the JavaScript intuition,
hence my message here.

I've tried to measure it [1]. Assuming my measurements are correct
(please tell me if they are not), the difference is not awful, but
significant.
It may be more significant if several processes are scheduled and the
IPC is not a direct round-trip. I have no measurements, just an
intuition on that part.

The rule of thumb here would be to be careful with retrieving the
properties that do an underlying IPC.

David

[1]
https://groups.google.com/forum/?fromgroups=#!topic/mozilla.dev.webapi/tLdpv5MkmUw

[Axel Hecht]

That does sound like unfortunate API design, too? In particular if I
read the post you replied to, "you shouldn't expect that getting the
frequency to be fast" sounds like "doh, should've done that api async to
start with".

Axel

[David Bruant]

Le 21/02/2013 15:54, Axel Hecht a écrit :
> That does sound like unfortunate API design, too?

That's a completely different debate, but that would be my opinion. I've
been documenting a lot of WebAPIs and as a JavaScript developer, I feel
a lot of the APIs aren't idiomatic.
Alex Russel said something very interesting about WebIDL-based API
design [1]:
"More to the point, [WebIDL]'s the language of the specs we have now,
and the default mode for writing new ones is "copy/paste some IDL from
another spec that looks close to what I need and then hack away until
it's close". This M.O. is exacerbated by the reality that most of the
folks writing these specs are C++ hackers, not JS developers. For many,
WebIDL becomes a safety blanket that keeps them from having to ever
think about the operational JS semantics or be confronted with the
mismatches."

I don't mean to shoot at the WebAPI devs, because they've been doing an
amazing job at exposing new capabilities to the web in a very short
period of time, but the resulting JS APIs are often not idiomatic and
some aspects like the one I just shared defeat the intuition of how
JavaScript works.

> In particular if I read the post you replied to, "you shouldn't expect
> that getting the frequency to be fast" sounds like "doh, should've
> done that api async to start with".

yes-but.
Yes, the API should have been async, but JavaScript-the-language does
not help a lot in that exercise. The current API is:
readonly attribute frequency
DOMRequest setFrequency(number)
Before anything else, the asymmetry is obvious when presented as I just
did. Getting is sync while setting is async. Also, the idea of a
"readonly" property where there is a corresponding "set*" method feels
inconsistent.
What are the alternatives though?
1) read/write frequency property
That's blocking on an IPC message in both cases which is not better.
2) getFrequency + setFrequency
The performance is better, but the syntax is annoying
navigator.mozFMRadio.getFrequency().onsuccess = function(){
var frequency = this.target.result;
// do something useful with the frequency
})

I feel the good intention behind the asymmetry was to avoid this kind of
code. The downside is a runtime cost which is at least one IPC
round-trip (so the actual cost probably depends on the whether).

3) (from the future) async syntax [2] (see the "Syntactic Sugar" section)
(navigator.mozFMRadio ! frequency = 87.7).then(function(){
// update UI to confirm that the frequency changed
})

(navigator.mozFMRadio ! frequency).then(function(freq){
console.log(freq)
})

Best of both world. But that's a far future, like ES7 far, like
move-the-event-loop-from-Gecko-to-SpiderMonkey far.

So yeah... A tradeoff has been made. I would have chosen 2), because it
had no additional runtime cost, but the Gaia codebase is simpler.

David

[1]
http://lists.w3.org/Archives/Public/public-script-coord/2012OctDec/0122.html
[2] http://wiki.ecmascript.org/doku.php?id=strawman:concurrency

[Rick Waldron]

Thanks David!

After reading this, I found these resources useful for understanding IPC

https://bugzilla.mozilla.org/show_bug.cgi?id=68702
https://developer.mozilla.org/en-US/docs/IPDL

Rick

On Thu, Feb 21, 2013 at 9:36 AM, David Bruant <brua...@gmail.com> wrote:

> Hi,
>
> I've noticed that there is a perf race recently on Gaia. I've recently
> found out that in some cases, getting properties on WebAPIs blocks on an
> IPC message (from the app to the parent process). The idea that getting a
> property blocks on an IPC message defeats the JavaScript intuition, hence
> my message here.
>
> I've tried to measure it [1]. Assuming my measurements are correct (please
> tell me if they are not), the difference is not awful, but significant.
> It may be more significant if several processes are scheduled and the IPC
> is not a direct round-trip. I have no measurements, just an intuition on
> that part.
>
> The rule of thumb here would be to be careful with retrieving the
> properties that do an underlying IPC.
>
> David
>

> [1] https://groups.google.com/**forum/?fromgroups=#!topic/**
> mozilla.dev.webapi/tLdpv5MkmUw<https://groups.google.com/forum/?fromgroups=#!topic/mozilla.dev.webapi/tLdpv5MkmUw>
> ______________________________**_________________
> dev-gaia mailing list
> dev-...@lists.mozilla.org
> https://lists.mozilla.org/**listinfo/dev-gaia<https://lists.mozilla.org/listinfo/dev-gaia>
>

[David Bruant]

Le 21/02/2013 16:40, Rick Waldron a écrit :
> Thanks David!
>
> After reading this, I found these resources useful for understanding IPC

Oh yes, sorry, I wrote "IPC" all over the place like if it was an
obvious thing. I should have explained:
IPC stands for "Inter-Process Communication".
On Linux (and other operating systems), each application run in a
"process". For instance, your web browser (Firefox) runs in a process,
your email client (Thunderbird) runs in a different process, so does
Solitaire. Each process has its memory isolated from other processes, so
that a process can't read or write the memory of another process. Yet
processes can communicate with one another by sending messages to one
another. If I had to guess, I'd say they use pipes [1].

In our case, if I understand Gaia process architecture correctly, the
radio app (which is isolated in its own process, mostly for security
reasons) talks to the main Gaia process (sending an IPC message) to ask
for the radio frequency when a JS script does "FMRadio.frequency" and
the main process answers back (answering the IPC message). During that
communication, nothing else happens in the radio app. That is, the UI is
frozen and no other code runs.
This kind of inter-process communication is obviously more expensive
than just getting a property of a given local object. My measurements
show that the situation isn't awful, but we can wonder how worse it can
be if several apps/workers are running and squatting the CPU.

> https://bugzilla.mozilla.org/show_bug.cgi?id=68702
I'm not entirely sure this one's relevant.

> https://developer.mozilla.org/en-US/docs/IPDL
This one either, but I don't know Mozilla codebase enough to tell.

In this particular case, the IPC happens through cpmm (child process
message manager) with
'cpmm.sendSyncMessage("DOMFMRadio:getFrequency")[0];' [2]
The IDL for sendSyncMessage sounds more relevant to read [3]. I haven't
found the implementation yet, but I'm still looking.

Did my explanation help understand? If not, feel free to ask more questions.
Sorry for being too quick.

David

[1] http://www.cs.cf.ac.uk/Dave/C/node23.html
[2] https://hg.mozilla.org/mozilla-central/rev/432033434b3a#l11.260
[3]
http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsIMessageManager.idl#280

>
> Rick
>
>
> On Thu, Feb 21, 2013 at 9:36 AM, David Bruant <brua...@gmail.com
> <mailto:brua...@gmail.com>> wrote:
>
> Hi,
>
> I've noticed that there is a perf race recently on Gaia. I've
> recently found out that in some cases, getting properties on
> WebAPIs blocks on an IPC message (from the app to the parent
> process). The idea that getting a property blocks on an IPC
> message defeats the JavaScript intuition, hence my message here.
>
> I've tried to measure it [1]. Assuming my measurements are correct
> (please tell me if they are not), the difference is not awful, but
> significant.
> It may be more significant if several processes are scheduled and
> the IPC is not a direct round-trip. I have no measurements, just
> an intuition on that part.
>
> The rule of thumb here would be to be careful with retrieving the
> properties that do an underlying IPC.
>
> David
>
> [1]

> https://groups.google.com/forum/?fromgroups=#!topic/mozilla.dev.webapi/tLdpv5MkmUw
> <https://groups.google.com/forum/?fromgroups=#%21topic/mozilla.dev.webapi/tLdpv5MkmUw>
> _______________________________________________
> dev-gaia mailing list
> dev-...@lists.mozilla.org <mailto:dev-...@lists.mozilla.org>
> https://lists.mozilla.org/listinfo/dev-gaia
>
>

[Fabrice Desre]

On 02/21/2013 06:54 AM, Axel Hecht wrote:

> That does sound like unfortunate API design, too? In particular if I

> read the post you replied to, "you shouldn't expect that getting the
> frequency to be fast" sounds like "doh, should've done that api async to
> start with".

Or it can also just be bug on the platform side. We fixed a bunch of
IPC-related perf issues when working on the app startup times for instance.

For the fmradio API, I see no reason to not be able to cache the current
frequency value in the content process to speed this up.

David will probably disagree with any api design that is not using
promises, but having a simple "frequency" property and an
onfrequencychanged event looks even better to me.

Fabrice
--
Fabrice Desré
b2g team
Mozilla Corporation

[Kevin Grandon]

I've actually noticed a few ticks of 'sendSyncMessage' inside of profiles when testing startup time.

I didn't look too far into where it was coming from, but I would assume that sendSyncMessage during startup time would be bad. What should we do if we notice this - open a bug against the WebAPI?

Thanks,
Kevin

https://lists.mozilla.org/listinfo/dev-gaia

[Julien Wajsberg]

I'd love it if these "on*changed" events would give the changed value in
as an event property, that would save us accessing it on the underlying
object.

--
Julien

[David Bruant]

Le 21/02/2013 19:17, Fabrice Desre a écrit :
> On 02/21/2013 06:54 AM, Axel Hecht wrote:
>
>> That does sound like unfortunate API design, too? In particular if I
>> read the post you replied to, "you shouldn't expect that getting the
>> frequency to be fast" sounds like "doh, should've done that api async to
>> start with".
> Or it can also just be bug on the platform side. We fixed a bunch of
> IPC-related perf issues when working on the app startup times for instance.
>
> For the fmradio API, I see no reason to not be able to cache the current
> frequency value in the content process to speed this up.

and update it anytime the actual frequency changes of course (to avoid
having an invalid cached value). That could work.
Of course, you have to do that for every "fake" data-properly. The
FMRadio API has 5 or 6 of them. Do we have a list all such cases in
other APIs somewhere? (that could be useful for perf improvements)
Also, not all apps use the "frequency" property, so maybe we want to
have this cache+update setup only for apps that do, so create this setup
lazily on first access or something. That could work too.
Note that even in the lazy-tracking setup, there is some loss. For
instance, maybe the app will read the frequency property (that's
unlikely for this property, but that could be the case for another
property in another app) only once and never read it again, but cache
consistency has to be maintained regardless of whether the value is
actually read, so the main Gaia process may end up spending an absurd
amount of time to uselessly updating cached values of different apps.

All of that setup and work for the sake of... frequency being a property?

> David will probably disagree with any api design that is not using
> promises, but having a simple "frequency" property and an
> onfrequencychanged event looks even better to me.

Just to clarify my position, this is not about promises; this is about
abstracting correctly. Having a "frequency" property is misleading as it
gives the impression that the property is easy and quick to get.
But that's a wrong abstraction: the FM radio is a remote resource. It's
only natural to consider that any access to it is asynchronous. I think
promises are a really good abstraction for these cases, but others
prefer callbacks [1], that's cool too.
What I disagree with is any API design that pretends remote things to be
local.

As I described above, the implementation has to pay the price of faking
something remote as local. Whether it's a blocking direct access (as
today) or a complex cache+update setup which has some loss and which
scalability has yet to be proven. We can also not update the cache and
perf will be fine, but at the cost of reliability.
Pretending that something remote is local has a cost somewhere.

Abstracting something remote as accessed asynchronously has a cost...
for developers. But as asyncStorage and so many other DOMRequest-based
APIs have proven, Gaia developers survived and from what I've read of
Gaia, the Pyramids of Doom [2] haven't grown very high... (or "very
right" maybe I should say).
And an async abstraction has no runtime nor reliability cost. No need to
cache+update, people just get the value as they need it. If they want to
implement a cached value, they can do it themselves with accessing the
value once at startup and and update when the event occur (as long as
the value is provided as part of the event?). But if authors don't want
the cache mechanism, they shouldn't have to pay the price for it in my
opinion.

David

[1]
https://github.com/mozilla-b2g/gaia/blob/master/shared/js/async_storage.js
[2]
http://calculist.org/blog/2011/12/14/why-coroutines-wont-work-on-the-web/

[Rick Waldron]

On Thu, Feb 21, 2013 at 1:36 PM, Julien Wajsberg <jwaj...@mozilla.com>wrote:

> Le 21/02/2013 19:17, Fabrice Desre a écrit :
> > On 02/21/2013 06:54 AM, Axel Hecht wrote:
> >
> >> That does sound like unfortunate API design, too? In particular if I
> >> read the post you replied to, "you shouldn't expect that getting the
> >> frequency to be fast" sounds like "doh, should've done that api async to
> >> start with".
> >
> > Or it can also just be bug on the platform side. We fixed a bunch of
> > IPC-related perf issues when working on the app startup times for
> instance.
> >
> > For the fmradio API, I see no reason to not be able to cache the current
> > frequency value in the content process to speed this up.
> >

> > David will probably disagree with any api design that is not using
> > promises, but having a simple "frequency" property and an
> > onfrequencychanged event looks even better to me.
>

> I'd love it if these "on*changed" events would give the changed value in
> as an event property, that would save us accessing it on the underlying
> object.
>

Yes!

thing.onchanged = function( event, primitive value at the moment of change
) {
...
};

"primitive value at the moment of change" from the JS runtime perspective
does not require any property access, which avoids any descriptor-get
accessor property lookups or descriptor.[[Get]] invocations.



Rick


>
> --
> Julien

[David Bruant]

Le 21/02/2013 20:22, Rick Waldron a écrit :
> On Thu, Feb 21, 2013 at 1:36 PM, Julien Wajsberg <jwaj...@mozilla.com>wrote:
>
>> Le 21/02/2013 19:17, Fabrice Desre a écrit :
>>> On 02/21/2013 06:54 AM, Axel Hecht wrote:
>>>
>>>> That does sound like unfortunate API design, too? In particular if I
>>>> read the post you replied to, "you shouldn't expect that getting the
>>>> frequency to be fast" sounds like "doh, should've done that api async to
>>>> start with".
>>> Or it can also just be bug on the platform side. We fixed a bunch of
>>> IPC-related perf issues when working on the app startup times for
>> instance.
>>> For the fmradio API, I see no reason to not be able to cache the current
>>> frequency value in the content process to speed this up.
>>>
>>> David will probably disagree with any api design that is not using
>>> promises, but having a simple "frequency" property and an
>>> onfrequencychanged event looks even better to me.
>> I'd love it if these "on*changed" events would give the changed value in
>> as an event property, that would save us accessing it on the underlying
>> object.

:-D

> Yes!
>
> thing.onchanged = function( event, primitive value at the moment of change
> ) {
> ...
> };
>
> "primitive value at the moment of change" from the JS runtime perspective
> does not require any property access, which avoids any descriptor-get
> accessor property lookups or descriptor.[[Get]] invocations.

I think the usual way to solve this problem is to create an Event
subclass and have an instance of the subclass dispatched:
FrequencyChangeEvent : Event {
readonly attribute double frequency; // or such
}

FMRadio.addEventListener('frequencychange', function(e){
var freq = e.frequency;
});

David

[David Bruant]

Le 21/02/2013 19:24, Kevin Grandon a écrit :
> I've actually noticed a few ticks of 'sendSyncMessage' inside of profiles when testing startup time.
>
> I didn't look too far into where it was coming from, but I would assume that sendSyncMessage during startup time would be bad. What should we do if we notice this - open a bug against the WebAPI?

Really good question...
On the Gaia side, the obvious thing is "do the property get only when
needed" (so hopefully after DOMContentLoaded, I guess).
But otherwise, yes, I guess changing APIs so that they abstract remote
resources as async-accessed thing is the right thing to do on the long term.

I didn't think it was that that big of a deal, but if API concerns start
being intrusive in startup times, maybe the API needs to change.

David

[Julien Wajsberg]

Le 21/02/2013 20:30, David Bruant a écrit :

> I think the usual way to solve this problem is to create an Event
> subclass and have an instance of the subclass dispatched:
> FrequencyChangeEvent : Event {
> readonly attribute double frequency; // or such
> }
>
> FMRadio.addEventListener('frequencychange', function(e){
> var freq = e.frequency;
> });

Yep that's what I had in mind.

[Rick Waldron]

+1!

...With the exception that e.frequency is a data property whose descriptor
is [[ writable: false, configurable: false, enumerable: true ]] (avoid a
[[Get]] accessor that has to reach across any execution process boundaries)

:)


Rick

[David Bruant]

I agree on the "avoiding reaching across execution process boundary",
but per WebIDL, the "frequency" property will be a {configurable:true,
enumerable:true, get: function(){}} on FrequencyChangeEvent.prototype
(instead of the instance)
The getter will be the WebIDL getter (so rather cheap)

If you think it should be an own data property on the instance, please
join the battle [1] :-) (but beware, there is resistance ahead)

David

[1] https://mail.mozilla.org/pipermail/es-discuss/2012-December/027521.html

[Mounir Lamouri]

On 21/02/13 18:36, Julien Wajsberg wrote:
> Le 21/02/2013 19:17, Fabrice Desre a écrit :
>> On 02/21/2013 06:54 AM, Axel Hecht wrote:
>>
>>> That does sound like unfortunate API design, too? In particular if I
>>> read the post you replied to, "you shouldn't expect that getting the
>>> frequency to be fast" sounds like "doh, should've done that api async to
>>> start with".
>>
>> Or it can also just be bug on the platform side. We fixed a bunch of
>> IPC-related perf issues when working on the app startup times for instance.
>>
>> For the fmradio API, I see no reason to not be able to cache the current
>> frequency value in the content process to speed this up.
>>
>> David will probably disagree with any api design that is not using
>> promises, but having a simple "frequency" property and an
>> onfrequencychanged event looks even better to me.
>
> I'd love it if these "on*changed" events would give the changed value in
> as an event property, that would save us accessing it on the underlying
> object.

Why? It would just make the API harder to understand I believe. Also,
the fact that calling .frequency is slow is an implementation detail
(ie. an optimization). We should make sure the value is cached on the
object when the event is fired.
For example, the Battery API implementation works that way: when an
event is received from the backend, the DOM object saves the value and
fires an event so getting the value is fast as long as it is cached (ie.
it is cached after the first call).

You should file a bug on the implementation.

--
Mounir

[David Bruant]

Le 22/02/2013 16:49, Mounir Lamouri a écrit :
> On 21/02/13 18:36, Julien Wajsberg wrote:
>> Le 21/02/2013 19:17, Fabrice Desre a écrit :
>>> On 02/21/2013 06:54 AM, Axel Hecht wrote:
>>>
>>>> That does sound like unfortunate API design, too? In particular if I
>>>> read the post you replied to, "you shouldn't expect that getting the
>>>> frequency to be fast" sounds like "doh, should've done that api async to
>>>> start with".
>>> Or it can also just be bug on the platform side. We fixed a bunch of
>>> IPC-related perf issues when working on the app startup times for instance.
>>>
>>> For the fmradio API, I see no reason to not be able to cache the current
>>> frequency value in the content process to speed this up.
>>>
>>> David will probably disagree with any api design that is not using
>>> promises, but having a simple "frequency" property and an
>>> onfrequencychanged event looks even better to me.
>> I'd love it if these "on*changed" events would give the changed value in
>> as an event property, that would save us accessing it on the underlying
>> object.
> Why? It would just make the API harder to understand I believe.

The frequency changed, you're registered to the 'frequencychange' event,
I feel the only thing you care about this event is the new (maybe the
old, but it's probably saved somewhere) frequency value. I feel it only
makes sense for the frequency to be part of the event object.

> Also, the fact that calling .frequency is slow is an implementation detail
> (ie. an optimization). We should make sure the value is cached on the
> object when the event is fired.
> For example, the Battery API implementation works that way: when an
> event is received from the backend, the DOM object saves the value and
> fires an event so getting the value is fast as long as it is cached (ie.
> it is cached after the first call).

Just to fully understand how it works:
1) an app starts
2) the script asks for the battery level the first time, so the first
time is "long" (which is not a problem)
3) the value is cached by non-script code and in the background, the
cache is refreshed anytime the battery level changes (is it what you
call "event is received from the backend"?)
4) everytime the app accesses the value, it's fast because the value is
already there and valid because the some background code listens to
battery changes (which the parent process communicates I assume)

It works, but if the app doesn't access the value as often as it is
refreshed, there is a lot of useless backend work, because the cached
value is refreshed but never read.
What is the cost of this useless work when you have a dozen of apps running?
At least, does the cache-update setup stops when the app is not in the
foreground?


I think it should be possible for the app to do this caching itself, but
it shouldn't be forced upon it (because imposing it degrades performance
if the value is read less often than it is updated in the background).
Here is how it could work:

(function(global){
var batteryLevel = Math.random();
setTimeout(function(){
// first access is slow, whatev's, part of its own event
loop turn.
batteryLevel = navigator.battery.level;
}, 0)

navigator.battery.addEventListener('batterychange', function(e){
batteryLevel = e.level; // because the event carries the
value ;-)
});

})(this);

In 10 lines of JS, you have a variable which contains the latest
batteryLevel value if you want it. No need for native or background code
to do that.
Since JS code is able to do its own cache, there is no need for the
engine to waste precious CPU cycles (especially with IPC which probably
requires context switching, etc.). In essence, push only the value to
the app if the app asks for it (explicitly registers to the *change event)

David

[Mounir Lamouri]

What I meant is that the UA can simply be clever to make the developer's
life easier. How it is done doesn't matter much. It is better to have
that kind of caching living in the UA than in the content because the UA
can do that for various apps while the content will all have to
reproduce it (ie. it scales better) and we can bet that the UA will do a
better work because it is closer to the hardware.

--
Mounir