Skip to content

Conversation

@annxingyuan
Copy link
Contributor

@annxingyuan annxingyuan commented Feb 3, 2020

This PR fixes #2624

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


This change is Reviewable

@annxingyuan annxingyuan self-assigned this Feb 3, 2020
@annxingyuan
Copy link
Contributor Author

@nsthorat @dsmilkov ping :D

Copy link
Contributor

@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.

Reviewed 4 of 4 files at r1.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @annxingyuan, @dsmilkov, and @nsthorat)


tfjs-core/src/backends/webgl/backend_webgl.ts, line 530 at r1 (raw file):

    };

    if (env().getNumber('WEBGL_DISJOINT_QUERY_TIMER_EXTENSION_VERSION') > 0) {

I think you want WEBGL_DISJOINT_QUERY_TIMER_EXTENSION_RELIABLE


tfjs-core/src/backends/webgl/backend_webgl.ts, line 538 at r1 (raw file):

              .map(d => `${d.name}: ${d.ms}`)
              .join(', ');
    } else {

possible to add a test for this where we set the flag explicitly to false and check the output?

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 4 of 4 files at r1.
Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @annxingyuan and @dsmilkov)

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.

No additional comments (+1 to Nikhil's comments)

Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @annxingyuan and @dsmilkov)

Copy link
Contributor Author

@annxingyuan annxingyuan 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: :shipit: complete! 2 of 1 approvals obtained


tfjs-core/src/backends/webgl/backend_webgl.ts, line 530 at r1 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

I think you want WEBGL_DISJOINT_QUERY_TIMER_EXTENSION_RELIABLE

Done


tfjs-core/src/backends/webgl/backend_webgl.ts, line 538 at r1 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

possible to add a test for this where we set the flag explicitly to false and check the output?

Done

@annxingyuan annxingyuan merged commit 0db0373 into master Feb 5, 2020
@annxingyuan annxingyuan deleted the time_nms branch February 5, 2020 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tf.time() kernelMs returns zero in Firefox

4 participants