Re: Web Workers

On Thu, Mar 1, 2012 at 9:49 PM, Dmitry Lomov <dslomov@google.com> wrote:

> On Wed, Feb 29, 2012 at 3:23 PM, Robert O'Callahan <robert@ocallahan.org>wrote:
>
>> The writeAudio method still does a copy. Maybe that copy can be avoided
>> with an alternative design using transfers, but simply preallocating an
>> output buffer as Dmitry suggested isn't quite the right solution since an
>> onprocessmedia handler can output any amount of audio, including none.
>> Instead we probably should have writeAudio take ownership of the buffer and
>> neuter it, or alternatively keep the current copy semantics for writeAudio
>> and have a separate transferAudio method that uses the transfer semantics.
>>
>>
> So these two proposals handle transfer-out but not transfer-in. As you
> say, it will either be a copy in a writeAudio, or, if we have a
> transferAudio method, an allocation on every callback (because you cannot
> reuse a transferred-out array buffer).
> If event has a perallocated output buffer, we will avoid any copies and
> allocations.
>

The allocation overhead can be practically eliminated by recycling buffers
internally in the browser.

The preallocated output buffer approach still has the problem of not
handling situations where the processing engine isn't outputting the same
amount of data that was input.


>
>>
>> Both current proposals suggest that any worker can handle only one
>>> JavaScriptAudioNode/ProcessedMediaStream (since the pocessing nodes run a
>>> predefined callback on the worker).
>>>
>>
>> This is definitely not the case for ProcessedMediaStream. There is no
>> problem with using the same Worker for several ProcessedMediaStreams. If
>> you want to have different types of processing performed in the same
>> Worker, you could use a field of the 'params' object in each onprocessmedia
>> event to distinguish them.
>>
>
> so you would have something like:
> self.onprocessmediaevent = function(event) {
>    if (event,param.routine == "sinGenerator") { ... }
>    else if (event.param.routine == "fobarEffect") { ... }
>    ....
> }
>
> so you would basically have a dispatch in JavaScript?
> This has several drawback:
> 1) dispatch in JavaScript takes precious cycles that might have been used
> processing audio
> 2) methods structured like this are harder to optimize by JavaScript
> engine JIT
> 3) this requires param to have a value and be transferred to worker on
> every call - this takes valuable cylces.
> 4) the code looks ugly :)
>

I'd write it like this:
var dispatchTable = {
  sinGenerator: function(event) {
    ...
  },
  foobarEffect: function(event) {
    ...
  }
};
self.onprocessmedia = function(event) {
  dispatchTable[event.params.type](event);
};
...
which reduces those drawbacks. I expect the overheads introduced are
negligible.

My proposal, that lets you specify a callback name, solves all of that.
> Moreover, I can see other benefits:
> a. no need to add extra methods to WorkerContext.idl
> b. ease of code use and re-use. The user can package her library of audio
> effects, where all audio effects have different names but conform to the
> same signature, and then easily import those into worker and    call them -
> no need for any boilerplate in the worker code itself.
>

On the other hand, it reduces isolation of the worker since it gives page
script the ability to call any global function in the worker instead of
going through predefined entry points (onmessage, onprocessmedia). I
suspect that's not a good idea. It's also more work to implement, for a
case that maybe doesn't even matter that much.


>
>> However, I think we should not encourage authors to multiplex processing
>> nodes onto Workers by hand, since the optimal assignment could depend on
>> the details of the browser and platform (e.g. how heavyweight Workers are,
>> and how many cores are available to the browser).
>>
>
> It looks like a simple UA-independent guideline that can be given is this:
> - if your audio processing nodes are sequential in the graph, dispatch
> them onto the same worker
> - if they are parallel, dispatch them on different workers.
>

That's good but it may not be optimal. In some situations you might get
better throughput by placing sequential nodes on different cores.


> Instead of making params a field of event object, maybe it will be cleaner
>>> to make params an extra argument to callback, and also pass initial value
>>> on node/stream construction?
>>>
>>
>> I don't understand the benefits of this. Having an event callback with a
>> single event object parameter is a standard pattern for Web APIs and I
>> don't see a need to deviate from it here.
>>
>
> This just makes for nicer looking and more understandable code, both on
> node creation and in processing routine.
>

OK, it's not a big deal one way or another.

My biggest problem with the params approach right now is this... It's
natural to write:
stream.params = {x:1, y:2};
stream.params.x = 3;
Unfortunately the second statement does not trigger any change in the
worker, because only assignments to the params attribute itself trigger
structured cloning and dispatch to the worker. I haven't thought of a
reasonable fix for this yet. Any suggestions appreciated.

Rob
-- 
“You have heard that it was said, ‘Love your neighbor and hate your enemy.’
But I tell you, love your enemies and pray for those who persecute you,
that you may be children of your Father in heaven. ... If you love those
who love you, what reward will you get? Are not even the tax collectors
doing that? And if you greet only your own people, what are you doing more
than others?" [Matthew 5:43-47]