[wasm] Add flag WASM_THREAD_POOL_SIZE by axinging · Pull Request #4942 · tensorflow/tfjs

axinging · 2021-04-16T04:54:28Z

This flag indicates how many threads can be used in multi-thread mode, it helps us understand the performance difference between wasm and webgpu/webgl.

This change is

qjia7

@lina128 @jinjingforever take a look, thanks.

qjia7 · 2021-04-16T07:51:02Z

-        disposeData: module.cwrap('dispose_data', voidReturnType, ['number']),
-        dispose: module.cwrap('dispose', voidReturnType, []),
+        getThreadPoolSize:
+            module.cwrap('get_thread_pool_size', 'number', [voidType]),


nit: Does it work to change [voidType] -> [] so that you don't need to change voidType name?

Yes, this also works.

qjia7 · 2021-04-16T08:11:51Z

    this.wasm.tfjs.init();
+    // Register the used thread pool size flag. Done it here to avoid circular
+    // import.
+    env().registerFlag('WASM_THREAD_POOL_SIZE', () => {


Usually, we register flag in flags_wasm.ts. And call env().set('WASM_THREAD_POOL_SIZE', this.getThreadPoolSize());. But I am not sure if it is always the rules.

Currently backend_wasm.ts deppends on flags_wasm.ts.
If we put those code in flags_wasm.ts, flags_wasm.ts will also depends on backend_wasm.ts, the test case will pass, but the yarn lint will fail due to circular imports.

Is there a good way to pass variable in .ts file to .cc? I'm not sure if this can work: move calculation logic to flags_wasm.ts, and register flag there. In backend.cc, provide a set method. Then in backend_wasm.ts, set the threads count by calling the set method.

gyagp · 2021-04-16T16:36:13Z

    this.wasm.tfjs.init();
+    // Register the used thread pool size flag. Done it here to avoid circular
+    // import.
+    env().registerFlag('WASM_THREAD_POOL_SIZE', () => {


Is there a good way to pass variable in .ts file to .cc? I'm not sure if this can work: move calculation logic to flags_wasm.ts, and register flag there. In backend.cc, provide a set method. Then in backend_wasm.ts, set the threads count by calling the set method.

gyagp · 2021-04-16T16:38:21Z

+    // Register the used thread pool size flag. Done it here to avoid circular
+    // import.
+    env().registerFlag('WASM_THREAD_POOL_SIZE', () => {
+      return this.getThreadPoolSize();


Actually this is not the size of pool, but count of threads in pool. According to https://github.com/Maratyszcza/pthreadpool/blob/master/include/pthreadpool.h#L77, please change the name to threads_count.

rename done

huningxin · 2021-04-21T01:32:56Z

+#ifdef __EMSCRIPTEN__
+EMSCRIPTEN_KEEPALIVE
+#endif
+const size_t get_threads_count() { return backend::threads_count; }


FYI. There is a pthreadpool API to query the number of threads in a thread pool

https://github.com/Maratyszcza/pthreadpool/blob/master/include/pthreadpool.h#L86

lina128

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

tfjs-backend-wasm/src/backend_wasm.ts, line 49 at r4 (raw file):

    // Register the used threads count flag. Done it here to avoid circular
    // import.
    env().registerFlag('WASM_THREADS_COUNT', () => {

Is this flag tunable?

axinging · 2021-04-27T07:46:31Z

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

tfjs-backend-wasm/src/backend_wasm.ts, line 49 at r4 (raw file):
    // Register the used threads count flag. Done it here to avoid circular
    // import.
    env().registerFlag('WASM_THREADS_COUNT', () => {
Is this flag tunable?

Current this flag is untuable. The set pthread pool size logic is not done yet.

This flag tells how many threads can be used in multi-thread mode.

axinging · 2021-05-18T01:54:29Z

I will close this, and will provide a better solution for this.

google-cla Bot added the cla: yes label Apr 16, 2021

axinging force-pushed the wasm_flagthreadpoolsize branch from 9f2bb46 to 867e5c9 Compare April 16, 2021 07:06

qjia7 reviewed Apr 16, 2021

View reviewed changes

qjia7 requested review from jinjingforever and lina128 April 16, 2021 08:12

axinging force-pushed the wasm_flagthreadpoolsize branch 2 times, most recently from 7b9d5c1 to d1715b4 Compare April 16, 2021 10:59

jinjingforever approved these changes Apr 16, 2021

View reviewed changes

gyagp reviewed Apr 19, 2021

View reviewed changes

axinging force-pushed the wasm_flagthreadpoolsize branch from f3363a7 to f574ce7 Compare April 19, 2021 06:00

axinging changed the title ~~[wasm] Add flag WASM_THREAD_POOL_SIZE~~ [wasm] Add flag WASM_THREADS_COUNT Apr 19, 2021

axinging force-pushed the wasm_flagthreadpoolsize branch 2 times, most recently from bd4edd8 to a30f6d3 Compare April 19, 2021 06:07

huningxin reviewed Apr 21, 2021

View reviewed changes

axinging force-pushed the wasm_flagthreadpoolsize branch from a30f6d3 to 042c9e6 Compare April 21, 2021 02:48

lina128 requested a review from mattsoulanille April 22, 2021 17:48

lina128 reviewed Apr 22, 2021

View reviewed changes

qjia7 mentioned this pull request Apr 23, 2021

[wasm] Fix wasm backend not starting any threads #4957

Merged

axinging force-pushed the wasm_flagthreadpoolsize branch from 042c9e6 to d1715b4 Compare April 27, 2021 01:08

axinging changed the title ~~[wasm] Add flag WASM_THREADS_COUNT~~ [wasm] Add flag WASM_THREAD_POOL_SIZE Apr 27, 2021

axinging force-pushed the wasm_flagthreadpoolsize branch from ac8b4ad to f9b635c Compare April 27, 2021 04:10

axinging mentioned this pull request Apr 27, 2021

navigator.hardwareConcurrency and native pthread gets different thread pool size #4995

Closed

axinging force-pushed the wasm_flagthreadpoolsize branch from f9b635c to 0717b84 Compare May 6, 2021 06:41

[wasm] Add flag WASM_THREAD_POOL_SIZE

27db3e1

This flag tells how many threads can be used in multi-thread mode.

axinging force-pushed the wasm_flagthreadpoolsize branch from 0717b84 to 27db3e1 Compare May 13, 2021 08:01

axinging closed this May 18, 2021

axinging deleted the wasm_flagthreadpoolsize branch September 22, 2021 06:55

axinging restored the wasm_flagthreadpoolsize branch September 22, 2021 06:55

Conversation

axinging commented Apr 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qjia7 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lina128 left a comment

Choose a reason for hiding this comment

Uh oh!

axinging commented Apr 27, 2021

Uh oh!

axinging commented May 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

axinging commented Apr 16, 2021 •

edited

Loading