Customize setTimeout #6694

gyagp · 2022-07-29T14:57:35Z

If the setTimeout nesting level is greater than 5 and timeout is less
than 4ms, timeout will be clamped to 4ms, which hurts the perf. A custom
setTimeout is provided to mitigate the perf impact.

BUG: #6687

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

This change is

gyagp · 2022-07-29T15:00:51Z

This PR is still WIP. @qjia7 @xhcao PTAL to see if you have any high-level comments. Do you have a good idea to write some test cases for this?

qjia7 · 2022-08-01T05:20:03Z

This PR is still WIP. @qjia7 @xhcao PTAL to see if you have any high-level comments. Do you have a good idea to write some test cases for this?

The implementation looks good for me. How about enabling setTimeoutWPM behind a flag for a while? For the test case, maybe add a new file util_base_test.js. Test that repeat call setTimeoutWPM 20(or a larger number) times. And expect the avg time of the last 15 is less than 4 ms or less?

xhcao · 2022-08-01T06:24:55Z

The function could be called by tf.profile, if you want to write unit cases. You could also view the total of tfjs-models' demos, the time could be reduced.

gyagp · 2022-08-03T21:33:07Z

With above changes, I get below from unit test:
LOG: 'averageTime of setTimeoutWPM is 0.06666664282480876 ms'
LOG: 'averageTime of setTimeout is 4 ms'

pyu10055

Reviewable status: 0 of 1 approvals obtained (waiting on @gyagp, @lina128, @Linchenn, @mattsoulanille, @qjia7, and @xhcao)

tfjs-core/src/util_base.ts line 765 at r1 (raw file):

  // If the setTimeout nesting level is greater than 5 and timeout is less than
  // 4ms, timeout will be clamped to 4ms, which hurts the perf. Interleaving
  // window.postMessage and setTimeout will trick the browser and avoid the

when the postMessage call will reset the timeout counting?

tfjs-core/src/util_base.ts line 770 at r1 (raw file):

    fns.push(fn);
    setTimeout(() => {
      window.postMessage({name: messageName, index: fns.length - 1}, '*');

I am curious why you use the index of the functions, instead of simply popping from the function array?

tfjs-core/src/util_base.ts line 780 at r1 (raw file):

      fn();
      handledMessageCount++;
      // console.log(`handledMessageCount=${handledMessageCount},

remove the comments?

pyu10055

Reviewable status: 0 of 1 approvals obtained (waiting on @gyagp, @lina128, @Linchenn, @mattsoulanille, @qjia7, and @xhcao)

tfjs-core/src/util_base.ts line 758 at r1 (raw file):

}

if (!('setTimeoutWPM' in window)) {

is this only targeting browser? might be better to register this on the browser platform file.

mattsoulanille · 2022-08-03T22:17:29Z

tfjs-core/src/util_base.ts

+      // console.log(`handledMessageCount=${handledMessageCount},
+      // fns.length=${fns.length}`);


Suggested change

// console.log(`handledMessageCount=${handledMessageCount},

// fns.length=${fns.length}`);

mattsoulanille · 2022-08-03T22:17:43Z

tfjs-core/src/util_base.ts

+  };
+
+  window.addEventListener('message', handleMessage, true);
+  (window as any).setTimeoutWPM = setTimeoutWPM;


It may be better to store this in _tfGlobals (instead of window), which you can get by calling getGlobalNamespace() from global_util.ts.

Actually, since this relies on window, _tfGlobals may be the wrong place for it. Maybe we should add an optional setTimeoutWPM function to platform.ts and add the function itself to platform_browser.ts (like Ping suggested in his review).

Moved to platform_browser.ts

mattsoulanille · 2022-08-03T22:21:05Z

tfjs-core/src/util_test.ts

+  // If we set a larger number here, an error will be reported as "'expect'
+  // was used when there was no current spec, this could be because an
+  // asynchronous test timed out".


This is because Jasmine does not know you're still testing setTimeout, so the expect called in the callback is actually called during another test (or after all tests are complete, which causes the error you're seeing). Marking the test as async or using a done callback should fix this. Docs

Thanks for the suggestion. I used done callback to solve this.

mattsoulanille · 2022-08-03T22:28:25Z

tfjs-core/src/util_test.ts

+        // We don't have expect here as in some browsers, like Chrome Canary,
+        // nesting level threshold is set to 100 instead of 5.


With the fix for totalCount above, you might be able to increase the count above 100 and add an expect. Otherwise, it may be best to remove this test or mark it with xit, since it doesn't appear to be testing anything (unless you're testing that no errors are thrown. If so, you should call expect().nothing()).

Increased to 100

mattsoulanille · 2022-08-03T22:28:43Z

tfjs-core/src/util_test.ts

+  const totalCount = 8;
+  const skipCount = 5;
+
+  it('setTimeout', () => {


Make this test async or add a done callback.

mattsoulanille · 2022-08-03T22:28:56Z

tfjs-core/src/util_test.ts

+    }
+  });
+
+  it('setTimeoutWPM', () => {


Make this test async or add a done callback.

mattsoulanille · 2022-08-03T22:32:42Z

tfjs-core/src/util_test.ts

+  // was used when there was no current spec, this could be because an
+  // asynchronous test timed out".
+  const totalCount = 8;
+  const skipCount = 5;


Nit: Document what this does and why it exists.

Suggested change

const skipCount = 5;

// Skip the first few samples because the browser does not throttle them.

const skipCount = 5;

comments added

mattsoulanille · 2022-08-03T22:35:31Z

tfjs-core/src/util_base.ts

+    }
+  };
+
+  window.addEventListener('message', handleMessage, true);


It might be better to add this event listener when the user first calls setTimeoutWPM. That way, we don't pollute the event listener list if setTimeoutWPM is never used.

Moved to first call to setTimeoutCustom.

Linchenn

Agree with Ping. If this targets to browser, we probably have to move it to browser platform.

LGTM! Thank you Yang so much for investigating and fulfilling it!

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

gyagp · 2022-08-05T09:45:43Z

tfjs-core/src/util_base.ts

@@ -303,7 +303,7 @@ export function rightPad(a: string, size: number): string {

 export function repeatedTry(
    checkFn: () => boolean, delayFn = (counter: number) => 0,
-    maxCounter?: number): Promise<void> {
+    maxCounter?: number, scheduleFn?: Function): Promise<void> {


environment.ts imports util_base, so we couldn't import environment in util_base, which will result in circular import. Thus I introduce scheduleFn here. If we need a custom scheduling function other than setTimeout, we can pass the function in. In our case, it's setTimeoutCustom() from WebGL side. In this way, repeatedTry() keeps to be platform independent.

gyagp · 2022-08-05T09:48:11Z

tfjs-core/src/flags.ts

@@ -82,3 +82,6 @@ ENV.registerFlag('ENGINE_COMPILE_ONLY', () => false);

 /** Whether to enable canvas2d willReadFrequently for GPU backends */
 ENV.registerFlag('CANVAS2D_WILL_READ_FREQUENTLY_FOR_GPU', () => false);
+
+/** Whether to use setTimeoutCustom */
+ENV.registerFlag('USE_SETTIMEOUTCUSTOM', () => false);


I changed to this name as even if we set it, we can't guarantee postMessage version setTimeout is called, as window might be not available in browser.

gyagp · 2022-08-05T09:49:27Z

tfjs-core/src/platforms/platform.ts

@@ -46,4 +46,6 @@ export interface Platform {
  encode(text: string, encoding: string): Uint8Array;
  /** Decode the provided bytes into a string using the provided encoding. */
  decode(bytes: Uint8Array, encoding: string): string;
+
+  setTimeoutCustom?(functionRef: Function, delay: number): void;


I changed the parameter names to be aligned with setTimeout definition in browser (https://developer.mozilla.org/en-US/docs/Web/API/setTimeout).

gyagp

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

tfjs-core/src/util_base.ts line 758 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

is this only targeting browser? might be better to register this on the browser platform file.

moved to platform_browser

tfjs-core/src/util_base.ts line 765 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

when the postMessage call will reset the timeout counting?

The trick here is if we interleave the setTimeout and window.postMessage, it will break the nesting rule of setTimeout (calling setTimeout inside callback).

tfjs-core/src/util_base.ts line 770 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

I am curious why you use the index of the functions, instead of simply popping from the function array?

We may call setTimeout with different delays, which means the sequence of call may not be the sequence of callbacks. For example, setTimeout(cb0, 2); setTimeout(cb1, 1); cb1 should be called first, then cb0.

tfjs-core/src/util_base.ts line 780 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

remove the comments?

done

gyagp · 2022-08-05T15:31:03Z

Thank you all for the comments! I think I have addressed all of them. Could you please take another look?
I also tested bodypix-MobileNetV1-image-0.25 and got below data (best time, average time):
[setTimeout]
16.5, 22.5
15.3, 20.7
15.8, 21.4
17.6, 23.2
17.4, 22.5

[setTimeoutCustom]
13.1, 16.0
13.2, 18.3
13.6, 15.8
12.7, 17.0
11.7, 15.2

The data shows we can get 2-4ms back.

It seems the bots are still not satisfied, but I couldn't figure out the reason after checking the detailed log. Could you please point me to the reason? Thanks!

Linchenn

Thank you Yang! LGTM.

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

mattsoulanille · 2022-08-05T16:25:10Z

tfjs-core/src/platforms/platform_browser_test.ts

+    let count = 0;
+    let startTime = performance.now();
+    let totalTime = 0;
+    if (env().platformName === 'browser') {


Can this check for the browser platform happen before this spec?

if (env().platformName === 'browser') { it('setTimeout', (done) => { ... }); }

Actually, if all the tests in this describe block need to run in the browser, you can replace it with

describeWithFlags('setTimeout', BROWSER_ENVS, () => { ... });

Then, you won't have to check the platform on each test.

describeWithFlags
BROWSER_ENVS

mattsoulanille · 2022-08-06T00:54:19Z

tfjs-core/src/util_base.ts

@@ -303,7 +303,7 @@ export function rightPad(a: string, size: number): string {

 export function repeatedTry(
    checkFn: () => boolean, delayFn = (counter: number) => 0,
-    maxCounter?: number): Promise<void> {
+    maxCounter?: number, scheduleFn?: Function): Promise<void> {


Use setTimeout as the default value (see next comment for why).

Suggested change

maxCounter?: number, scheduleFn?: Function): Promise<void> {

maxCounter?: number, scheduleFn = setTimeout): Promise<void> {

The types here might not be correctly inferred, so you might have to manually specify

scheduleFn: (callback: () => void, time: number) => void

or something similar.

mattsoulanille · 2022-08-06T00:55:16Z

tfjs-core/src/util_base.ts

+      if (typeof scheduleFn === 'undefined') {
+        setTimeout(tryFn, nextBackoff);
+      } else {
+        scheduleFn(tryFn, nextBackoff);
+      }


Set the default value of scheduleFunction to setTimeout to avoid this if statement.

Suggested change

if (typeof scheduleFn === 'undefined') {

setTimeout(tryFn, nextBackoff);

} else {

scheduleFn(tryFn, nextBackoff);

}

scheduleFn(tryFn, nextBackoff);

If the setTimeout nesting level is greater than 5 and timeout is less than 4ms, timeout will be clamped to 4ms, which hurts the perf. A custom setTimeout is provided to mitigate the perf impact.

gyagp

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

tfjs-core/src/util_base.ts line 328 at r4 (raw file):

Previously, mattsoulanille (Matthew Soulanille) wrote…

Set the default value of scheduleFunction to setTimeout to avoid this if statement.
      scheduleFn(tryFn, nextBackoff);

This looks more clean. done!

tfjs-core/src/platforms/platform_browser_test.ts line 102 at r4 (raw file):

Previously, mattsoulanille (Matthew Soulanille) wrote…

Actually, if all the tests in this describe block need to run in the browser, you can replace it with
describeWithFlags('setTimeout', BROWSER_ENVS, () => {
  ...
});
Then, you won't have to check the platform on each test.

describeWithFlags
BROWSER_ENVS

BROWSER_ENVS is much better. Thanks for the suggestion!

qjia7 · 2022-08-07T01:50:21Z

tfjs-core/src/util_base.ts

@@ -321,7 +323,7 @@ export function repeatedTry(
        reject();
        return;
      }
-      setTimeout(tryFn, nextBackoff);
+      scheduleFn(tryFn, nextBackoff);


How do you think to always call setTimeoutCustom(tryFn, nextBackoff) here? And move setTimeoutCustom(tryFn, nextBackoff) to a common place since your current implementation setTimeoutCustom is already a common way and not only for webgl backend? In this case, you don't need to change gpgpu_context.ts?

We should replace setTimeout with a custom one only in browser and window is ready. So platform browser may be a good place to host it.
util_base should not be dependent on any env, but setTimeoutCustom relies on env (for both platform check and flag check). So I'm afraid even with your suggestion, we still need to pass setTimeoutCustom as a parameter (Actually I introduced scheduleFn to avoid the dependency with env).

gyagp · 2022-08-08T04:47:10Z

@pyu10055 Can you review again the latest changes? Thanks!

lina128

Thank you Yang! LGTM

vladmandic · 2022-08-09T12:01:41Z

I love this improvement as it results in quite significant performance improvements

but at the moment it causes conflict with @tensorflow/tfjs-backend-wasm as soon as that module is loaded
regardless if its used or not (IMO its due to bad behavior by EMScripten, but its a conflict non-the-less),

see #6687 (comment) for details

gyagp · 2022-08-09T14:25:46Z

Thanks @vladmandic , I will investigate this.

pyu10055

Thanks @gyagp looks good to me, just couple minor comments.

Reviewable status: complete! 3 of 1 approvals obtained (waiting on @gyagp, @mattsoulanille, @pyu10055, @qjia7, and @xhcao)

tfjs-core/src/platforms/platform.ts line 50 at r3 (raw file):

Previously, gyagp (Yang Gu) wrote…

I changed the parameter names to be aligned with setTimeout definition in browser (https://developer.mozilla.org/en-US/docs/Web/API/setTimeout).

To align with setTimeout, should it return the id for allowing clearTimeout API?

tfjs-core/src/platforms/platform_browser_test.ts line 112 at r5 (raw file):

      if (count === totalCount) {
        const averageTime = totalTime / (totalCount - skipCount);
        console.log(`averageTime of setTimeout is ${averageTime} ms`);

remove the console log

tfjs-core/src/platforms/platform_browser_test.ts line 140 at r5 (raw file):

      if (count === totalCount) {
        const averageTime = totalTime / (totalCount - skipCount);
        console.log(`averageTime of setTimeoutCustom is ${averageTime} ms`);

remove the console log

tfjs-core/src/platforms/platform_browser_test.ts line 141 at r5 (raw file):

        const averageTime = totalTime / (totalCount - skipCount);
        console.log(`averageTime of setTimeoutCustom is ${averageTime} ms`);
        if (window) {

since you are specifying browser_env, there is no need to test window variable, and the false branch should not be triggered

gyagp

Reviewable status: complete! 3 of 1 approvals obtained (waiting on @gyagp, @mattsoulanille, @pyu10055, @qjia7, and @xhcao)

tfjs-core/src/platforms/platform.ts line 50 at r3 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

To align with setTimeout, should it return the id for allowing clearTimeout API?

I don't want to introduce the complexity to implement clearTimeoutCustom. Without it, it's useless to return id, so I intentionally set it as void.

tfjs-core/src/platforms/platform_browser_test.ts line 112 at r5 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

remove the console log

This is in the test and I intentionally added this to understand the setTimeout delay on various configurations, like node, safari, etc. The results may change as time goes by. For example, Chrome is increasing the throttle nesting level from 5 to 100, which may cause failure of this test in future. With this info, it should be easier to understand the reason.

tfjs-core/src/platforms/platform_browser_test.ts line 140 at r5 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

remove the console log

same as above

tfjs-core/src/platforms/platform_browser_test.ts line 141 at r5 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

since you are specifying browser_env, there is no need to test window variable, and the false branch should not be triggered

I was thinking about running this in a worker. It should be unnecessary. Will remove in a follow-up PR.

pyu10055

Reviewed 3 of 6 files at r2, 2 of 4 files at r3, 1 of 2 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: complete! 3 of 1 approvals obtained (waiting on @gyagp, @mattsoulanille, @pyu10055, @qjia7, and @xhcao)

tfjs-core/src/platforms/platform_browser_test.ts line 141 at r5 (raw file):

Previously, gyagp (Yang Gu) wrote…

I was thinking about running this in a worker. It should be unnecessary. Will remove in a follow-up PR.

we run worker tests separately, and those tests are typically a separate file.

gyagp · 2022-09-27T15:16:31Z

@pyu10055 @mattsoulanille, given related Emscripten issues were fixed (Thanks @vladmandic !) and @mattsoulanille upgraded Emscripten to include the fixes, can we merge this change now?

* Customize setTimeout (tensorflow#6694) If the setTimeout nesting level is greater than 5 and timeout is less than 4ms, timeout will be clamped to 4ms, which hurts the perf. A custom setTimeout is provided to mitigate the perf impact. BUG: tensorflow#6687 Co-authored-by: Na Li <[email protected]> * Upgrade windows BrowserStack chrome to 104 (tensorflow#6866) * webgpu: Disable importExternalTexture (tensorflow#6868) WebGPU Working Group recently found some problem with importExtenalTexture in spec, so we have to disable it temporarily. Co-authored-by: Yang Gu <[email protected]> Co-authored-by: Na Li <[email protected]> Co-authored-by: Matthew Soulanille <[email protected]>

@Brianzheng123

* Customize setTimeout (tensorflow#6694) If the setTimeout nesting level is greater than 5 and timeout is less than 4ms, timeout will be clamped to 4ms, which hurts the perf. A custom setTimeout is provided to mitigate the perf impact. BUG: tensorflow#6687 Co-authored-by: Na Li <[email protected]> * Upgrade windows BrowserStack chrome to 104 (tensorflow#6866) * webgpu: Disable importExternalTexture (tensorflow#6868) WebGPU Working Group recently found some problem with importExtenalTexture in spec, so we have to disable it temporarily. * Refactored Resizing Layer Unit Tests (#38) * Rescaling Preprocessing Layer Co-authored-by: David Kim (@koyykdy) <[email protected]> Brian Zheng (@Brianzheng123) <[email protected]> * PR issues resolved * linting and PR issues resolved Co-authored-by: Adam Lang (@AdamLang96) <[email protected]> Co-authored-by: (@Brianzheng123) <[email protected]> * initial implementation of image preprocessing: resizing layer, and associated unit tests. Comments and refactoring for image scaling layer * refactoring in computeOutputShape for image resizing layer * Unit tests for image resizing preprocessing layer expanded and refactored * refactored unit tests for resizing layer * Preprocessing-Resizing layer unit test expansion and refactoring. Co-authored-by: Adam Lang <@AdamLang96> ([email protected]) * cleaning up commit diffs * cleaning up commit diffs * PR commit suggestions accepted - code refactored to reflect changes * resizing layer unit test refactoring Co-authored-by: AdamLang96 <[email protected]> * Linting issue resolved: unused import statement culled (#39) * Rescaling Preprocessing Layer Co-authored-by: David Kim (@koyykdy) <[email protected]> Brian Zheng (@Brianzheng123) <[email protected]> * PR issues resolved * linting and PR issues resolved Co-authored-by: Adam Lang (@AdamLang96) <[email protected]> Co-authored-by: (@Brianzheng123) <[email protected]> * initial implementation of image preprocessing: resizing layer, and associated unit tests. Comments and refactoring for image scaling layer * refactoring in computeOutputShape for image resizing layer * Unit tests for image resizing preprocessing layer expanded and refactored * refactored unit tests for resizing layer * Preprocessing-Resizing layer unit test expansion and refactoring. Co-authored-by: Adam Lang <@AdamLang96> ([email protected]) * cleaning up commit diffs * cleaning up commit diffs * PR commit suggestions accepted - code refactored to reflect changes * resizing layer unit test refactoring * linting issues resolved: unusued import statement culled Co-authored-by: AdamLang96 <[email protected]> * Update jasmine_util.ts (tensorflow#6872) FIX * webgl: Fix NaN issue (tensorflow#6828) Fix tensorflow#6822 Problem 1： On some GPUs, even if a and b are both non-NaN, the value of isNaN in vec4 isNaN = min(vec4(isnan(a)) + vec4(isnan(b)), vec4(1.0)); are still larger than 0., which misleads all values become NAN. 2： After resolving NAN issue, the result is still incorrect. It seems that the isnan_custom is not well supported on the problem GPU. After switching back to builtin isnan, everything works well. Solution: Use the bool type bvec4 instead of float type vec4 to calculate isNaN to avoid the the float precision issue when comparing with zero. Meanwhile, add an env flag WEBGL2_ISNAN_CUSTOM to allow user to specify which isnan to use. * Upgrade nodejs to 18.7.0 (tensorflow#6863) * Upgrade nodejs to 18.7.0 * Fix hash table test string not passed as base64 * fixed prelu fusing code that pre-maturely neg the const on multiply (tensorflow#6876) Co-authored-by: RajeshT <[email protected]> * Update tfjs-layers/src/layers/preprocessing/image_resizing.ts Co-authored-by: Matthew Soulanille <[email protected]> Co-authored-by: Yang Gu <[email protected]> Co-authored-by: Na Li <[email protected]> Co-authored-by: Matthew Soulanille <[email protected]> Co-authored-by: AdamLang96 <[email protected]> Co-authored-by: Linchenn <[email protected]> Co-authored-by: Jiajia Qin <[email protected]> Co-authored-by: Ping Yu <[email protected]> Co-authored-by: RajeshT <[email protected]> Co-authored-by: Matthew Soulanille <[email protected]> Co-authored-by: Yang Gu <[email protected]> Co-authored-by: Na Li <[email protected]> Co-authored-by: Matthew Soulanille <[email protected]> Co-authored-by: AdamLang96 <[email protected]> Co-authored-by: Linchenn <[email protected]> Co-authored-by: Jiajia Qin <[email protected]> Co-authored-by: Ping Yu <[email protected]> Co-authored-by: RajeshT <[email protected]> Co-authored-by: Matthew Soulanille <[email protected]>

hujiajie · 2022-12-08T08:17:56Z

tfjs-core/src/platforms/platform_browser_test.ts

+  // Skip the first few samples because the browser does not clamp the timeout
+  const skipCount = 5;
+
+  it('setTimeout', (done) => {


@gyagp I began to see this test fail on Linux, maybe related with https://chromestatus.com/feature/5710690097561600

@hujiajie What's your channel and version of Chrome? I tried both Stable (108.0.5359.124) and Dev (110.0.5478.4) on Linux, but the nesting levels are still 5. I also double checked Windows, with Stable (108.0.5359.125) and Canary (111.0.5491.0), and the nesting levels are also 5.
My testing program is timer.html at https://github.com/webatintel/webatintel.github.io. You need to download it to local (otherwise the result is impacted by the network condition) and type "/timer.html?test=setTimeout&maxCount=30" in the address bar.

Ah, it seems chromestatus is lying here, and the feature is still in field trial. What if you add --enable-features=MaxUnthrottledTimeoutNestingLevel?

With this option, nesting level is 15. I will do the change once Chrome really enables this in the stable channel.

gyagp force-pushed the settimeout branch from be3d082 to d4a2ab1 Compare August 3, 2022 21:19

gyagp requested review from mattsoulanille, pyu10055, lina128, qjia7 and xhcao August 3, 2022 21:33

gyagp mentioned this pull request Aug 3, 2022

major difference in performance between sync and async methods to download tensor data #6687

Closed

gyagp requested a review from Linchenn August 3, 2022 21:43

pyu10055 requested changes Aug 3, 2022

View reviewed changes

mattsoulanille requested changes Aug 3, 2022

View reviewed changes

Linchenn reviewed Aug 4, 2022

View reviewed changes

gyagp force-pushed the settimeout branch from 4fca3e4 to ae45f6a Compare August 5, 2022 09:33

gyagp commented Aug 5, 2022

View reviewed changes

gyagp force-pushed the settimeout branch from ae45f6a to 2cb9ebe Compare August 5, 2022 10:58

gyagp requested review from mattsoulanille, pyu10055 and Linchenn August 6, 2022 00:36

Linchenn reviewed Aug 6, 2022

View reviewed changes

mattsoulanille requested changes Aug 6, 2022

View reviewed changes

Yang Gu added 4 commits August 6, 2022 09:16

Customize setTimeout

4d743e6

If the setTimeout nesting level is greater than 5 and timeout is less than 4ms, timeout will be clamped to 4ms, which hurts the perf. A custom setTimeout is provided to mitigate the perf impact.

Add flag and unit test

2502a86

Move to platform_browser

b87bf71

Address Matt's comments

f9af357

gyagp force-pushed the settimeout branch from cc75f04 to f9af357 Compare August 6, 2022 15:35

gyagp commented Aug 6, 2022

View reviewed changes

gyagp requested a review from mattsoulanille August 6, 2022 16:23

qjia7 reviewed Aug 7, 2022

View reviewed changes

mattsoulanille approved these changes Aug 7, 2022

View reviewed changes

lina128 approved these changes Aug 9, 2022

View reviewed changes

pyu10055 approved these changes Aug 11, 2022

View reviewed changes

gyagp commented Aug 11, 2022

View reviewed changes

Address Ping's comments

eb706a9

gyagp requested a review from pyu10055 August 11, 2022 10:38

Yang Gu and others added 2 commits August 11, 2022 18:38

Merge branch 'master' into settimeout

315f614

Merge branch 'master' into settimeout

eed68b4

pyu10055 approved these changes Aug 15, 2022

View reviewed changes

Merge branch 'master' into settimeout

113a984

mattsoulanille merged commit ba66eb0 into tensorflow:master Sep 27, 2022

gyagp deleted the settimeout branch September 28, 2022 01:20

dydavidkim mentioned this pull request Sep 29, 2022

Refactored PR Comment Issues, Plus Pulling in updates to tfjs master repository CodeSmithDSMLProjects/tfjs#40

Merged

hujiajie reviewed Dec 8, 2022

View reviewed changes

		// console.log(`handledMessageCount=${handledMessageCount},
		// fns.length=${fns.length}`);

		// We don't have expect here as in some browsers, like Chrome Canary,
		// nesting level threshold is set to 100 instead of 5.

	const skipCount = 5;
	// Skip the first few samples because the browser does not throttle them.
	const skipCount = 5;

	maxCounter?: number, scheduleFn?: Function): Promise<void> {
	maxCounter?: number, scheduleFn = setTimeout): Promise<void> {

Customize setTimeout #6694

Customize setTimeout #6694

Conversation

gyagp commented Jul 29, 2022 • edited Loading

gyagp commented Jul 29, 2022

qjia7 commented Aug 1, 2022

xhcao commented Aug 1, 2022

gyagp commented Aug 3, 2022

pyu10055 left a comment

Choose a reason for hiding this comment

pyu10055 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Linchenn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gyagp left a comment

Choose a reason for hiding this comment

gyagp commented Aug 5, 2022

Linchenn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gyagp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gyagp commented Aug 8, 2022

lina128 left a comment

Choose a reason for hiding this comment

vladmandic commented Aug 9, 2022 • edited Loading

gyagp commented Aug 9, 2022

pyu10055 left a comment

Choose a reason for hiding this comment

gyagp left a comment

Choose a reason for hiding this comment

pyu10055 left a comment

Choose a reason for hiding this comment

gyagp commented Sep 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gyagp commented Jul 29, 2022 •

edited

Loading

vladmandic commented Aug 9, 2022 •

edited

Loading