Skip to content
This repository has been archived by the owner on Aug 15, 2019. It is now read-only.

Add support for fences in WebGL 2.0. Turn off query timers. #1177

Merged
merged 19 commits into from
Jul 25, 2018
Merged

Conversation

nsthorat
Copy link
Contributor

@nsthorat nsthorat commented Jul 20, 2018

  • Adds fenceSync + getBufferData in WebGL 2.0 when downloading data from a texture.
  • Abstracts fences to either use fenceSync (gl 2.0) or EXT_disjoint_query_timer (gl 1.0) depending on the device. Fences always go through the binary search queryer. Timers now always are polled independently (since the GPU time is not a function of the CPU time).
  • Turns off WEBGL_DISJOINT_QUERY_TIMER_EXTENSION_VERSION for safety. We've made significant changes to the code paths under this path but have had no way of testing it. Once Chrome reenables EXT_disjoint_query_timer (Re-enable WEBGL_DISJOINT_QUERY_TIMER_EXTENSION_VERSION when chrome enables EXT_disjoint_query_timer tfjs#544) we'll reenable the bit.
  • Throw an error for GL 2.0 + no floats. This currently will not work. If we ever need this, we'll come back to it.
  • Adds typings for WebGL 2.0 and webgl-ext. Verified the compile time does not change.
  • Fixes a bug in the non-float download enable code where it creates a tensor with a preexisting DataId, causing a memory leak and an error.

Side effects:

  • MathBackendWebGL.readSync() always calls gl.readPixels to a CPU ArrayBuffer. read() calls gl.readPixels into a CPU ArrayBuffer in WebGL 1.0, and calls gl.readPixels into a GPU Buffer and then calls gl.getBufferSubData into the CPU ArrayBuffer. These deviate ever so slightly, but it should not be a big deal (since performance is not critical for readSync())

This change is Reviewable

Copy link
Contributor Author

@nsthorat nsthorat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 1 approvals obtained


package.json, line 22 at r1 (raw file):

    "@types/seedrandom": "~2.4.27",
    "@types/webgl2": "~0.0.4",
    "@types/webgl-ext": "0.0.29",

this typing is not in google3 yet, I'm working on syncing it.


src/kernels/webgl/webgl_types.ts, line 18 at r1 (raw file):

 */

export interface WebGLQuery {}

these are now taken care of by the official typings.

@nsthorat nsthorat changed the title NOT READY: Add support for fences in WebGL 2.0. Add support for fences in WebGL 2.0. Turn off query timers. Jul 25, 2018
@nsthorat nsthorat requested a review from dsmilkov July 25, 2018 13:55
Copy link
Contributor

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 9 of 9 files at r1.
Reviewable status: 0 of 1 approvals obtained (waiting on @nsthorat and @dsmilkov)


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

    ENV.setFeatures(features);
    // expect to be 1 when https://github.com/tensorflow/tfjs/issues/544 is

prepend a TODO


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

    ENV.setFeatures(features);
    // expect to be 2 when https://github.com/tensorflow/tfjs/issues/544 is

same here


src/kernels/backend_webgl.ts, line 301 at r1 (raw file):

      texShape: [number, number], shape: number[]): Float32Array {
    if (!ENV.get('WEBGL_DOWNLOAD_FLOAT_ENABLED')) {
      const tmpTarget = Tensor.make(shape, {});

maybe flip the if with more lines after the if


src/kernels/webgl/gpgpu_context.ts, line 247 at r1 (raw file):

          query, ENV.get('WEBGL_DISJOINT_QUERY_TIMER_EXTENSION_VERSION'));
    } else {
      // If we have no way to fence, return true immediately.

extend the comment a bit saying under which condition this happens


src/kernels/webgl/gpgpu_util.ts, line 309 at r1 (raw file):

            gl2.PIXEL_PACK_BUFFER, bufferSizeBytes, gl.STATIC_DRAW));

    // Copy the texture into the buffer.

made say enqueue to the gpu command buffer, since it's not blocking or immediate

@nsthorat nsthorat merged commit aa2ba2c into master Jul 25, 2018
@nsthorat nsthorat deleted the fence branch July 25, 2018 16:56
@nsthorat nsthorat mentioned this pull request Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants