Add a demo of WebGL running with web workers and transferrable typed arrays.

Add a demo of WebGL running with web workers and transferrable typed arrays.

Clean up the extra files that were replicated in the first round of this
patch.

[kbr1, 2012-01-04 04:45:37]

This is great work overall and I am really looking forward to seeing it
committed. Some comments; a few of these are actually questions.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/PeriodicIterator.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/PeriodicIterator.js:2: * Copyright (c)
2009 The Chromium Authors. All rights reserved.
Copyright here and in all new files in this code review should be 2012.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/PeriodicIterator.js:29: //
PeriodicIterator
The replication of this is unfortunate (compared to the original demo) but I
gather it's needed to factor this out. Perhaps you could do that refactoring in
the other demo and reference its PeriodicIterator.js in this one?

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/benchmark.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/benchmark.js:36: useTransferrables:
false,
Here and throughout the code review, "transferrables" should be "transferables".
http://www.whatwg.org/specs/web-apps/current-work/multipage/common-dom-interf...

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/calculate.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/calculate.js:109: (cordicSin((loXIndex
/ conf.constants.SIN_ARRAY_SIZE) * 2 *
Why are you using this cordicSin implementation and not precomputed sin and
cosine tables as in the original demo? You ought to be able to configure each
worker once with the tables and then reuse them many times. For example, the
first time a worker receives a message requesting computation with a given
SIN_ARRAY_SIZE it can compute its (persistent) sin and cos arrays of those
lengths.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/demo-multicore.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/demo-multicore.js:478: function
computeSlabInfoArrays(slabInfo, loY, hiY) {
This should be done within each worker; doing so will increase parallelism. You
ought to be able to simply pass some indices to the worker which will set up its
iterators appropriately so it can compute its local ysinlo, etc. arrays. You
might need to add some new methods to PeriodicIterator.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/demo-multicore.js:643: if
(config.multithreaded) {
Just use "else".

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/demo-multicore.js:752: cosArray: null
You should not need to pass any of these arrays over the wire. ysinlo, ycoslo,
etc. are temporaries, and should be able to be allocated entirely within the
worker. The sinArray and cosArray can similarly be computed lazily within the
worker, upon receiving a request to compute a slab of a particular strip size.
The only thing that should be necessary to pass to the worker is some state
indicating how to set up its PeriodicIterators, as well as the ArrayBuffer to
fill in.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/demo-multicore.js:813: // Put it on
the compuete queue
typo: compute

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/demo-multicore.js:821: }
Ideally this loop should be able to be removed.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/demo-multicore.js:828:
startCalculation();
At some point I'd love to double-buffer the data, so that the workers are
computing the next frame's data while this frame's data is being rendered.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/index.html (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/index.html:36: <script
type="text/javascript" src="resources/fpscounter.js"></script>
See comments in individual files about avoiding duplication of these sources.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/math-cordic.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/math-cordic.js:2: * Copyright (C) Rich
Moore.  All rights reserved.
Per earlier comment I think the incorporation of this file is unnecessary.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/resources/fpscounter.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/resources/fpscounter.js:2: * Copyright
(c) 2009 The Chromium Authors. All rights reserved.
Don't replicate this code; instead reference ../resources/fpscounter.js in your
top-level HTML file.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/resources/o3djs/base.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/resources/o3djs/base.js:2: * Copyright
2009, Google Inc.
Don't replicate this code. Just reference ../resources/o3djs/base.js in your
top-level HTML file. Same for all the other files in this o3djs/ subdirectory.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/resources/webgl-utils.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/resources/webgl-utils.js:2: *
Copyright 2010, Google Inc.
Again, don't replicate this code. Just reference ../../common/webgl-utils.js in
your top-level HTML file.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/worker.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/worker.js:33: * COPYRIGHT NVIDIA
CORPORATION 2003. ALL RIGHTS RESERVED.
You do not need to replicate NVIDIA's copyright here. All of this code is new;
none of it is derived from NVIDIA's.

[wiltzius, 2012-01-09 01:46:30]

Made the file include changes, fixed typos, and updated copyright messages. Did
not remove the cordic sine implementation or move the temporary variables to the
workers for initialization yet; will do that next after getting your comments.
Thanks Ken!

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/PeriodicIterator.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/PeriodicIterator.js:2: * Copyright (c)
2009 The Chromium Authors. All rights reserved.
I'm assuming this includes files taken from the original demo but modified. I've
updated copyrights on such files but let me know if that's wrong.

On 2012/01/04 04:45:37, kbr1 wrote:
> Copyright here and in all new files in this code review should be 2012.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/PeriodicIterator.js:29: //
PeriodicIterator
Done. Although if these demos are all meant to be somewhat independent, should
this go in 'resources' or something? I realize it's only used by the two of
them, but it does seem odd to require the other demo directory for this one to
work. Just a thought.

On 2012/01/04 04:45:37, kbr1 wrote:
> The replication of this is unfortunate (compared to the original demo) but I
> gather it's needed to factor this out. Perhaps you could do that refactoring
in
> the other demo and reference its PeriodicIterator.js in this one?

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/benchmark.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/benchmark.js:36: useTransferrables:
false,
On 2012/01/04 04:45:37, kbr1 wrote:
> Here and throughout the code review, "transferrables" should be
"transferables".
>
http://www.whatwg.org/specs/web-apps/current-work/multipage/common-dom-interf...

Done.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/calculate.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/calculate.js:109: (cordicSin((loXIndex
/ conf.constants.SIN_ARRAY_SIZE) * 2 *
So when I was playing around with initially converted demo I found a lot of time
got eaten up by the drawing, and not on the CPU (limiting how much the move to
workers helped). Dmitry suggested using an alternate implementation that was
more CPU intensive... hence the cordic sine instead of the tables.

I realize this is hacky or even disingenuous, so I'm open to ideas about how to
improve. If you like, I can convert back to the basic precomputed table approach
and we can explore options from there. Perhaps first I should make the other
changes about what gets transferred that you suggest elsewhere, though?

On 2012/01/04 04:45:37, kbr1 wrote:
> Why are you using this cordicSin implementation and not precomputed sin and
> cosine tables as in the original demo? You ought to be able to configure each
> worker once with the tables and then reuse them many times. For example, the
> first time a worker receives a message requesting computation with a given
> SIN_ARRAY_SIZE it can compute its (persistent) sin and cos arrays of those
> lengths.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/demo-multicore.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/demo-multicore.js:478: function
computeSlabInfoArrays(slabInfo, loY, hiY) {
Per our conversation on Friday I'm going to defer on making this change (and the
implications mentioned below until we get basic agreement on all the files),
mostly just because I need to work on this in small chunks for time reasons :)

I hear you on why it should be this way, though, and I agree that moving as much
as possible into the worker will both increase parallelism and make the
abstraction cleaner.

On 2012/01/04 04:45:37, kbr1 wrote:
> This should be done within each worker; doing so will increase parallelism.
You
> ought to be able to simply pass some indices to the worker which will set up
its
> iterators appropriately so it can compute its local ysinlo, etc. arrays. You
> might need to add some new methods to PeriodicIterator.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/demo-multicore.js:643: if
(config.multithreaded) {
On 2012/01/04 04:45:37, kbr1 wrote:
> Just use "else".

Done.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/demo-multicore.js:752: cosArray: null
See above comment, agree but will make this change in the second round.

On 2012/01/04 04:45:37, kbr1 wrote:
> You should not need to pass any of these arrays over the wire. ysinlo, ycoslo,
> etc. are temporaries, and should be able to be allocated entirely within the
> worker. The sinArray and cosArray can similarly be computed lazily within the
> worker, upon receiving a request to compute a slab of a particular strip size.
> The only thing that should be necessary to pass to the worker is some state
> indicating how to set up its PeriodicIterators, as well as the ArrayBuffer to
> fill in.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/demo-multicore.js:813: // Put it on
the compuete queue
On 2012/01/04 04:45:37, kbr1 wrote:
> typo: compute

Done.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/demo-multicore.js:828:
startCalculation();
The Microsoft fountain demo uses workers to continuously compute frame data and
shove it into a queue, and then the main thread just pulls off the front of the
queue every frame. The workers populate put frame data into the queue as fast as
they can (presumably to some limit in queue size or something). Thoughts on
doing it that way? That's a larger change but seems interesting.

On 2012/01/04 04:45:37, kbr1 wrote:
> At some point I'd love to double-buffer the data, so that the workers are
> computing the next frame's data while this frame's data is being rendered.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/index.html (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/index.html:36: <script
type="text/javascript" src="resources/fpscounter.js"></script>
On 2012/01/04 04:45:37, kbr1 wrote:
> See comments in individual files about avoiding duplication of these sources.

Done.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/resources/fpscounter.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/resources/fpscounter.js:2: * Copyright
(c) 2009 The Chromium Authors. All rights reserved.
On 2012/01/04 04:45:37, kbr1 wrote:
> Don't replicate this code; instead reference ../resources/fpscounter.js in
your
> top-level HTML file.

Done.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/resources/o3djs/base.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/resources/o3djs/base.js:2: * Copyright
2009, Google Inc.
On 2012/01/04 04:45:37, kbr1 wrote:
> Don't replicate this code. Just reference ../resources/o3djs/base.js in your
> top-level HTML file. Same for all the other files in this o3djs/ subdirectory.

Done.

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
File sdk/demos/google/web-workers-typed-arrays/resources/webgl-utils.js (right):

http://codereview.appspot.com/5504121/diff/1/sdk/demos/google/web-workers-typ...
sdk/demos/google/web-workers-typed-arrays/resources/webgl-utils.js:2: *
Copyright 2010, Google Inc.
On 2012/01/04 04:45:37, kbr1 wrote:
> Again, don't replicate this code. Just reference ../../common/webgl-utils.js
in
> your top-level HTML file.

Done.

[kbr1, 2012-01-13 00:47:01]

LGTM. Great work. Let's check this in now and iterate on it.

[wiltzius, 2012-01-13 00:56:04]

On 2012/01/13 00:47:01, kbr1 wrote:
> LGTM. Great work. Let's check this in now and iterate on it.

Change committed to Khronos repo as R16574.