Add ndarray.getValuesAsync() method which returns a promise that resolves when values are ready. #146

nsthorat · 2017-09-22T22:44:42Z

This implementation will poll the GPU for the availability of the disjoint query object as if we were benchmarking. The cost of polling is negligible, so we can use a setTimeout(fn, 0) to check.

I also removed the exponential backoff in the benchmarks, we can just poll as fast as possible since it's pretty cheap.

When the extension isn't enabled, we just call getValues and resolve immediately.

This change also allows flags to depend on each other, and values get cached so we don't have to construct lots of canvases when evaluating flags.

This change is

dsmilkov · 2017-09-23T00:46:20Z

Just few tiny comments. Great work!

Reviewed 14 of 14 files at r1.
Review status: all files reviewed at latest revision, 7 unresolved discussions.

src/environment.ts, line 27 at r1 (raw file):

export interface Features {
  'WEBGL_DISJOINT_QUERY_TIMER_EXTENSION_ENABLED'?: boolean;

Add short one liner comment on top of each feature explaining what each feature means (e.g. want to understand the diff between the two query time extensions)

src/environment_test.ts, line 42 at r1 (raw file):

                return {loseContext: () => {}};
              }
              console.log('returning null');

remove console.log

src/util.ts, line 226 at r1 (raw file):

(counter: number) => 0

this looks strange, shouldn't it be (counter: number) => number ?

src/util.ts, line 227 at r1 (raw file):

export function repeatedTry(
    checkFn: () => boolean, delayFn = (counter: number) => 0,
    maxCounter?: number) {

re-add the return type of repeatedTry()

src/math/webgl/gpgpu_context.ts, line 266 at r1 (raw file):

   * the command buffer has finished executing the query.
   * @param queryFn The query function containing GL commands to execute.
   * @return a promise that resolves with the ellapsed time.

s/ellapsed time/ellapsed time in ms/

src/math/webgl/gpgpu_context.ts, line 311 at r1 (raw file):

                .getQueryParameter(query, (this.gl as any).QUERY_RESULT);
        // Return milliseconds.
        resolve(timeElapsedMicros / 1000000);

same here: 1ms = 1000 micro seconds. Why divide by 1 million?

src/math/webgl/gpgpu_context.ts, line 349 at r1 (raw file):

timeElapsedMicros / 1000000

1 ms = 1000 micro seconds. Divide by 1000 instead of 1 million ?

Comments from Reviewable

nsthorat · 2017-09-23T15:00:35Z

Thanks! Fixed your comments. It's so nice to have a thorough reviewer :)

Review status: 9 of 14 files reviewed at latest revision, 7 unresolved discussions, some commit checks pending.

src/environment.ts, line 27 at r1 (raw file):