Add an API to keep data on GPU: dataToGPU #5953

lina128 · 2021-12-17T00:18:35Z

This PR adds a convenient API to read data out as a texture, so that it can directly be rendered on canvas or used by downstream shaders.

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.

This change is

danwexler · 2021-12-17T21:14:19Z

Very excited to see this new function. Will this be in v13?

pyu10055

Reviewable status: 0 of 1 approvals obtained (waiting on @lina128)

tfjs-backend-webgl/src/backend_webgl.ts, line 809 at r3 (raw file):

          size <= texSize,
          () => 'customTexShape is too small. ' +
              'Row by column by 4 should be equal or larger than the ' +

Row * Column * 4

Code quote:

Row by column by 4

tfjs-backend-webgl/src/backend_webgl_test.ts, line 752 at r3 (raw file):

});

// TODO(lina128): Debug issue in CI. Code runs fine in Safari WebGL1 in local.

is this failing due to texture is float16? Since the difference is only the ENVS, we delete this section when we figure out CI issue for WebGL1.

tfjs-core/src/tensor.ts, line 369 at r3 (raw file):

  /**
   * Synchronously copy the tensor's data to a new GPU resource. Comparing to

This is async copy in terms of JS, since it is runs a webGL program, the JS return does not guarantee completion of shader execution.
But for WebGL this is sync, since the GPU queue is synchronized.

tfjs-backend-webgl/src/gpgpu_util.ts, line 227 at r2 (raw file):

              gl.TEXTURE_2D, 0, 0, 0, pixels.width, pixels.height, gl.RGBA,
              gl.UNSIGNED_BYTE, (pixels as PixelData).data));
      gl.flush();

gl.flush() I think this line should be removed, I guess I added by accident, this should not be needed.

lina128

Reviewable status: 0 of 1 approvals obtained (waiting on @pyu10055)

tfjs-backend-webgl/src/backend_webgl.ts, line 809 at r3 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

Row * Column * 4

Done.

tfjs-backend-webgl/src/backend_webgl_test.ts, line 752 at r3 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

is this failing due to texture is float16? Since the difference is only the ENVS, we delete this section when we figure out CI issue for WebGL1.

Fixed.

tfjs-core/src/tensor.ts, line 369 at r3 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

This is async copy in terms of JS, since it is runs a webGL program, the JS return does not guarantee completion of shader execution.
But for WebGL this is sync, since the GPU queue is synchronized.

Removed the confusing wording.

tfjs-backend-webgl/src/gpgpu_util.ts, line 227 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

gl.flush() I think this line should be removed, I guess I added by accident, this should not be needed.

Will fix in a separate PR.

lina128 · 2021-12-20T21:17:11Z

Very excited to see this new function. Will this be in v13?

@danwexler , yes.

qjia7

I don't notice that you pass a canvas context to webgl backend. It seems that the returned GPUResource is still limited in current tfjs context. How should users use this texture to another webgl program?

qjia7 · 2021-12-21T09:00:54Z

tfjs-backend-webgl/src/backend_webgl.ts

+      return gpuResouorce;
+    }
+
+    if (values != null && texture == null) {


If values is not null, can we just upload the data to a texture and return the texture?

lina128

Hi Jiajia, yes, GPUResource is still limited in current tfjs context. We suggest them to pass the canvas to use when creating webgl backend. Here's the other PR that allows user to pass a canvas: #5983

Reviewable status: 0 of 1 approvals obtained (waiting on @pyu10055 and @qjia7)

tfjs-backend-webgl/src/backend_webgl.ts, line 415 at r5 (raw file):

Previously, qjia7 (Jiajia Qin) wrote…

If values is not null, can we just upload the data to a texture and return the texture?

I have thought about this, it's hard to say which way is more convenient to users, so we'll see. Normally, if values are not null, it's either a small amount of data or the values haven't been uploaded yet, in both cases, users may have their preferred way of using the data, not necessarily using TFJS formatted data on GPU, so I prefer to leave it to user for now. However, if there's such request to provide TFJS formatted texture in the future, we can definitely support it.

pyu10055

Reviewable status: 0 of 1 approvals obtained (waiting on @lina128, @pyu10055, and @qjia7)

tfjs-backend-webgl/src/backend_webgl.ts, line 415 at r5 (raw file):

Previously, lina128 (Na Li) wrote…

I have thought about this, it's hard to say which way is more convenient to users, so we'll see. Normally, if values are not null, it's either a small amount of data or the values haven't been uploaded yet, in both cases, users may have their preferred way of using the data, not necessarily using TFJS formatted data on GPU, so I prefer to leave it to user for now. However, if there's such request to provide TFJS formatted texture in the future, we can definitely support it.

might be good to throw error instead of returning null. given that we don't support reading on CPU data to GPU.

tfjs-backend-webgl/src/backend_webgl.ts, line 423 at r5 (raw file):

    // Make engine track this tensor, so that we can dispose it later.
    engine().makeTensorFromDataId(

how user can track this tensor and dispose later?

tfjs-backend-webgl/src/backend_webgl_test.ts, line 682 at r5 (raw file):

});

describeWithFlags('keeping data on gpu ', WEBGL2_ENVS, () => {

can you add tests for memory leak check?

tfjs-core/src/tensor.ts, line 392 at r5 (raw file):

   * @doc {heading: 'Tensors', subheading: 'Classes'}
   */
  dataToGPU(options?: DataToGPUOptions): GPUResource {

should the GPUResource also contains reference to the tensor? so call can dispose it later?

lina128

Reviewable status: 0 of 1 approvals obtained (waiting on @lina128, @pyu10055, and @qjia7)

tfjs-backend-webgl/src/backend_webgl.ts, line 423 at r5 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

how user can track this tensor and dispose later?

The GPUResource has dataId, which is the only thing needed when disposing tensor. User can dispose it the same way as tensor, for typescript user, they may need to cast GPUResource to tensor first. So something like: tf.dispose(gpuResource as {} as Tensor).

tfjs-core/src/tensor.ts, line 392 at r5 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

should the GPUResource also contains reference to the tensor? so call can dispose it later?

It has reference to the tensor, dataId.

pyu10055

Reviewable status: 0 of 1 approvals obtained (waiting on @lina128, @pyu10055, and @qjia7)

tfjs-core/src/tensor.ts, line 392 at r5 (raw file):

Previously, lina128 (Na Li) wrote…

It has reference to the tensor, dataId.

can tidy automatically dispose GPUResource?

lina128

Reviewable status: 0 of 1 approvals obtained (waiting on @pyu10055 and @qjia7)

tfjs-backend-webgl/src/backend_webgl.ts, line 415 at r5 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

might be good to throw error instead of returning null. given that we don't support reading on CPU data to GPU.

Done.

tfjs-backend-webgl/src/backend_webgl_test.ts, line 682 at r5 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

can you add tests for memory leak check?

Done.

tfjs-core/src/tensor.ts, line 392 at r5 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

can tidy automatically dispose GPUResource?

It should behave the same as Tensor, added a test to verify.

qjia7 · 2022-01-06T06:28:43Z

tfjs-core/src/tensor.ts

+   *
+   * @doc {heading: 'Tensors', subheading: 'Classes'}
+   */
+  dataToGPU(options?: DataToGPUOptions): GPUResource {


Just curious, why support customTexShape? Is there any special requirement for this?

lina128

Reviewable status: 0 of 1 approvals obtained (waiting on @pyu10055 and @qjia7)

tfjs-backend-webgl/src/backend_webgl.ts, line 423 at r5 (raw file):

Previously, lina128 (Na Li) wrote…

The GPUResource has dataId, which is the only thing needed when disposing tensor. User can dispose it the same way as tensor, for typescript user, they may need to cast GPUResource to tensor first. So something like: tf.dispose(gpuResource as {} as Tensor).

Hi Ping, you are right, it's better to track this tensor instead of just use dataId, I added the memory test. Previously, the tensor cannot be disposed properly because GPUResource is not recognized as instanceOf Tensor in one place in engine. After the latest change, it should work. Also see the memory test.

tfjs-core/src/tensor.ts, line 392 at r6 (raw file):

Previously, qjia7 (Jiajia Qin) wrote…

Just curious, why support customTexShape? Is there any special requirement for this?

The main use case is if user set the texShape to be the same as the canvas shape, then they can directly render the result on canvas. If they don't specify the texShape, we try to make a squarish texture.

pyu10055

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @lina128, @pyu10055, and @qjia7)

tfjs-backend-webgl/src/backend_webgl_test.ts, line 802 at r7 (raw file):

      const b = tf.add(a, 0);
      b.dataToGPU();
      return b

missing ;

lina128

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @pyu10055 and @qjia7)

tfjs-backend-webgl/src/backend_webgl_test.ts, line 802 at r7 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

missing ;

Done.

xhcao · 2022-04-19T07:24:08Z

Hi, @lina128 , you had replied Jiajia's as below,
"The main use case is if user set the texShape to be the same as the canvas shape, then they can directly render the result on canvas. If they don't specify the texShape, we try to make a squarish texture."

I have a confusion here, If canvas size is 44, and texture size is 22, we could directly render this texture to the canvas, and hardware will use texture LINEAR or NEAREST filter sampling to render the content.
And if texture size 2*2, and its content is [1, 2, 3, 4], and call dataToGPU(..., [4,4]) to create a new texture, whose content is [1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4] (Assume it is correct), these content is expected data for users? Are there rules for result when enlarging the texture size.

lina128 · 2022-04-21T20:55:41Z

Hi @xhcao , I think for WebGL, if the texture is larger than the data size, the unused space will be 0. We try to store data in dense format, so the content will be [1, 2, 3, 4, 0, ..., 0].

xhcao · 2022-04-22T08:07:33Z

Hi, @lina128 , I had tested unit case on webgl backend,
it('uses user defined texShape [3, 3].', () => {
const data = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12];
const a = tf.tensor(data, [1, 3, 4]);
const b = tf.add(a, 0);
const texShape = [3, 3] as [number, number];
const res = b.dataToGPU({customTexShape: texShape});
expectArraysEqual(res.texShape, texShape);

const webGLBackend = tf.backend() as MathBackendWebGL;
const buffer = webGLBackend.gpgpu.createBufferFromTexture(
    res.texture, res.texShape[0], res.texShape[1]);
const vals = webGLBackend.gpgpu.downloadFloat32MatrixFromBuffer(buffer, 36);
console.log(vals);
expectArraysEqual(vals, data);

});
The result is shown as below, it does not fill unused space with 0, and the code is related in

tfjs/tfjs-backend-webgl/src/decode_matrix_gpu.ts

Line 50 in 5c3a89c

ivec2 resTexRC = ivec2(resultUV.yx * vec2(texShape[0], texShape[1]));

Float32Array(36) [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, buffer: ArrayBuffer(144), byteLength: 144, byteOffset: 0, length: 36, Symbol(Symbol.toStringTag): 'Float32Array']

I am confusion why we need to change the shape. We could not ensure the filling content is correct and is expected by Users. What are rules to define the filling behaviors? I think it is incorrect to fill with 0. For example, if original texture is 2x2, and then call dataToGPU with shape 4x4 to create the new texture, most of the new texture is transparent, I think it is not the expected result.

lina128 · 2022-04-22T21:32:56Z

Hi @xhcao, you only take the first 12 elements, which is correct, right? This is the same approach we use for reading data to cpu in WebGL, we create the right size of the buffer and call readPixels: https://github.com/tensorflow/tfjs/blob/master/tfjs-backend-webgl/src/gpgpu_util.ts#L271

Data beyond the size doesn't matter whether it is 0 or some other values, we simply ignore them.

xhcao · 2022-04-24T07:25:45Z

@lina128 , Does it mean the data beyond the size of the new created texture must be written again before it could be rendered to canvas? There may be a wrong understanding of dataToGPU usage for me. Could you simply give an usage example of dataToGPU with custom size here. Thank you.

lina128 · 2022-04-25T16:42:43Z

Hi @xhcao, I see what you mean. There should be another shader to format how it wants to render if the size doesn't match the canvas size. Can you describe a real use case? It will help me prepare the example. Thanks.

xhcao · 2022-04-26T09:33:25Z

Hi, @lina128, actually, I did not know the scenarios which could gain the performance improvement by using custom size option, so I asked you why we needed the custom size option.
Now, I may image a scenario as below.
[1]a tensor (shape: 2x4) calling dataToGPU with custom size (4x4) to create a new bigger texture A(4x4), the first half content of A is equal to the tensor. Then we use another shader to fill the other half content of A, render the texture A to the canvas at last.

We could also use the below steps to implement the above scenario.
[2]A tensor (shape: 2x4) calling dataToGPU without custom size to create a texture A(2x4), the content of A is equal to the tensor, then we use another shader to create the texture B(2x4) and fill the content, individually render the texture A and texture B to the first half and second half of the canvas at last.

I am not sure whether [1] is better than [2], but I sort of agree to add the option to dataToGPU api if there is no side effect.

lina128 · 2022-04-28T18:32:02Z

Hi @xhcao, thank you for providing the imagined scenarios. The actual scenario of using customSize is more like the other way round, the customSize will fit exactly the data, whereas our default (the more squarish texture) will waste some space.

lina128 marked this pull request as ready for review December 19, 2021 00:06

lina128 requested a review from pyu10055 December 19, 2021 00:06

pyu10055 requested changes Dec 19, 2021

View reviewed changes

lina128 commented Dec 20, 2021

View reviewed changes

lina128 requested a review from pyu10055 December 20, 2021 21:13

qjia7 reviewed Dec 21, 2021

View reviewed changes

lina128 commented Jan 5, 2022

View reviewed changes

lina128 mentioned this pull request Jan 5, 2022

[webgl]Allow constructor to take canvas. #5983

Merged

pyu10055 requested changes Jan 5, 2022

View reviewed changes

lina128 commented Jan 5, 2022

View reviewed changes

pyu10055 requested changes Jan 5, 2022

View reviewed changes

lina128 force-pushed the texture branch from 5b7727a to 583ea35 Compare January 5, 2022 23:25

lina128 commented Jan 5, 2022

View reviewed changes

qjia7 reviewed Jan 6, 2022

View reviewed changes

lina128 commented Jan 6, 2022

View reviewed changes

lina128 force-pushed the texture branch from 9f2db6c to d975b22 Compare January 6, 2022 17:39

lina128 requested a review from pyu10055 January 6, 2022 17:58

lina128 force-pushed the texture branch from b96b5aa to a379107 Compare January 6, 2022 19:10

lina128 added 10 commits January 6, 2022 11:19

Merge conflict.

e78bc5c

Merge conflict.

930b65b

WIP

b50a26f

Merge conflict.

b7b6b8e

WIP

91776a5

WIP

674a972

WIP

bc3a444

WIP

4ca43e4

fix test.

889ed54

fix test.

3d411cc

lina128 added 14 commits January 6, 2022 11:19

fix test.

ea1d403

fix test.

acb8f27

fix test.

90e2daa

Clean up.

0443cff

Fix test.

088a01a

Fix test.

68a0f16

Clean up.

d46c47a

Address comments.

46bc834

Fix test.

714b983

Fix test.

b3103c5

Fix test.

07305cf

Fix test.

5a28e10

Fix GPUResource.

e821edd

Add tidy tests.

edc71e9

lina128 force-pushed the texture branch from a379107 to edc71e9 Compare January 6, 2022 19:19

pyu10055 approved these changes Jan 6, 2022

View reviewed changes

Fix lint.

a083092

lina128 commented Jan 6, 2022

View reviewed changes

lina128 merged commit d8c046c into tensorflow:master Jan 6, 2022

lina128 deleted the texture branch January 6, 2022 19:45

jinjingforever mentioned this pull request Jan 12, 2022

tfjs-tflite gives the error RuntimeError: Aborted(). Build with -s ASSERTIONS=1 for more info. #6000

Closed

qjia7 mentioned this pull request Apr 15, 2022

webgpu: support dataToGPU api #6329

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an API to keep data on GPU: dataToGPU #5953

Add an API to keep data on GPU: dataToGPU #5953

lina128 commented Dec 17, 2021 •

edited

Loading

danwexler commented Dec 17, 2021

pyu10055 left a comment

lina128 left a comment

lina128 commented Dec 20, 2021

qjia7 left a comment

qjia7 Dec 21, 2021

lina128 left a comment

pyu10055 left a comment

lina128 left a comment

pyu10055 left a comment

lina128 left a comment

qjia7 Jan 6, 2022

lina128 left a comment

pyu10055 left a comment

lina128 left a comment

xhcao commented Apr 19, 2022

lina128 commented Apr 21, 2022

xhcao commented Apr 22, 2022

lina128 commented Apr 22, 2022

xhcao commented Apr 24, 2022

lina128 commented Apr 25, 2022

xhcao commented Apr 26, 2022

lina128 commented Apr 28, 2022

Add an API to keep data on GPU: dataToGPU #5953

Add an API to keep data on GPU: dataToGPU #5953

Conversation

lina128 commented Dec 17, 2021 • edited Loading

danwexler commented Dec 17, 2021

pyu10055 left a comment

Choose a reason for hiding this comment

lina128 left a comment

Choose a reason for hiding this comment

lina128 commented Dec 20, 2021

qjia7 left a comment

Choose a reason for hiding this comment

qjia7 Dec 21, 2021

Choose a reason for hiding this comment

lina128 left a comment

Choose a reason for hiding this comment

pyu10055 left a comment

Choose a reason for hiding this comment

lina128 left a comment

Choose a reason for hiding this comment

pyu10055 left a comment

Choose a reason for hiding this comment

lina128 left a comment

Choose a reason for hiding this comment

qjia7 Jan 6, 2022

Choose a reason for hiding this comment

lina128 left a comment

Choose a reason for hiding this comment

pyu10055 left a comment

Choose a reason for hiding this comment

lina128 left a comment

Choose a reason for hiding this comment

xhcao commented Apr 19, 2022

lina128 commented Apr 21, 2022

xhcao commented Apr 22, 2022

lina128 commented Apr 22, 2022

xhcao commented Apr 24, 2022

lina128 commented Apr 25, 2022

xhcao commented Apr 26, 2022

lina128 commented Apr 28, 2022

lina128 commented Dec 17, 2021 •

edited

Loading