-
-
Couldn't load subscription status.
- Fork 33.6k
cli: add --trace-atomics-wait flag
#33292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6386c1a
1ee0a80
f982160
4fb193d
d412f82
a32ae40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| 'use strict'; | ||
| /* global SharedArrayBuffer */ | ||
|
|
||
| const common = require('../common.js'); | ||
| const bench = common.createBenchmark(main, { | ||
| n: [1e7] | ||
| }); | ||
|
|
||
| function main({ n }) { | ||
| const i32arr = new Int32Array(new SharedArrayBuffer(4)); | ||
| bench.start(); | ||
| for (let i = 0; i < n; i++) | ||
| Atomics.wait(i32arr, 0, 1); // Will return immediately. | ||
| bench.end(n); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -229,11 +229,56 @@ int Environment::InitializeInspector( | |
| } | ||
| #endif // HAVE_INSPECTOR && NODE_USE_V8_PLATFORM | ||
|
|
||
| #define ATOMIC_WAIT_EVENTS(V) \ | ||
| V(kStartWait, "started") \ | ||
| V(kWokenUp, "was woken up by another thread") \ | ||
| V(kTimedOut, "timed out") \ | ||
| V(kTerminatedExecution, "was stopped by terminated execution") \ | ||
| V(kAPIStopped, "was stopped through the embedder API") \ | ||
| V(kNotEqual, "did not wait because the values mismatched") \ | ||
|
|
||
| static void AtomicsWaitCallback(Isolate::AtomicsWaitEvent event, | ||
| Local<v8::SharedArrayBuffer> array_buffer, | ||
| size_t offset_in_bytes, int64_t value, | ||
| double timeout_in_ms, | ||
| Isolate::AtomicsWaitWakeHandle* stop_handle, | ||
| void* data) { | ||
| Environment* env = static_cast<Environment*>(data); | ||
|
|
||
| const char* message; | ||
| switch (event) { | ||
| #define V(key, msg) \ | ||
| case Isolate::AtomicsWaitEvent::key: \ | ||
| message = msg; \ | ||
| break; | ||
| ATOMIC_WAIT_EVENTS(V) | ||
| #undef V | ||
| } | ||
|
|
||
| fprintf(stderr, | ||
| "(node:%d) [Thread %" PRIu64 "] Atomics.wait(%p + %zx, %" PRId64 | ||
| ", %.f) %s\n", | ||
| static_cast<int>(uv_os_getpid()), | ||
| env->thread_id(), | ||
| array_buffer->GetBackingStore()->Data(), | ||
| offset_in_bytes, | ||
| value, | ||
| timeout_in_ms, | ||
| message); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other places we output diagnostics to the console we typically prepend pid detail. Would be good to be consistent with that here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be useful when mixing multiple threads and processes so one wouldn't get There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, done! 👍 |
||
|
|
||
| void Environment::InitializeDiagnostics() { | ||
| isolate_->GetHeapProfiler()->AddBuildEmbedderGraphCallback( | ||
| Environment::BuildEmbedderGraph, this); | ||
| if (options_->trace_uncaught) | ||
| isolate_->SetCaptureStackTraceForUncaughtExceptions(true); | ||
| if (options_->trace_atomics_wait) { | ||
| isolate_->SetAtomicsWaitCallback(AtomicsWaitCallback, this); | ||
| AddCleanupHook([](void* data) { | ||
| Environment* env = static_cast<Environment*>(data); | ||
| env->isolate()->SetAtomicsWaitCallback(nullptr, nullptr); | ||
| }, this); | ||
| } | ||
|
|
||
| #if defined HAVE_DTRACE || defined HAVE_ETW | ||
| InitDTrace(this); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| 'use strict'; | ||
| require('../common'); | ||
| const assert = require('assert'); | ||
| const child_process = require('child_process'); | ||
| const { Worker } = require('worker_threads'); | ||
|
|
||
| if (process.argv[2] === 'child') { | ||
| const i32arr = new Int32Array(new SharedArrayBuffer(8)); | ||
| assert.strictEqual(Atomics.wait(i32arr, 0, 1), 'not-equal'); | ||
| assert.strictEqual(Atomics.wait(i32arr, 0, 0, 10), 'timed-out'); | ||
|
|
||
| new Worker(` | ||
| const i32arr = require('worker_threads').workerData; | ||
| Atomics.store(i32arr, 1, -1); | ||
| Atomics.notify(i32arr, 1); | ||
| Atomics.wait(i32arr, 1, -1); | ||
| `, { eval: true, workerData: i32arr }); | ||
|
|
||
| Atomics.wait(i32arr, 1, 0); | ||
| assert.strictEqual(Atomics.load(i32arr, 1), -1); | ||
| Atomics.store(i32arr, 1, 0); | ||
| Atomics.notify(i32arr, 1); | ||
| return; | ||
| } | ||
|
|
||
| const proc = child_process.spawnSync( | ||
| process.execPath, | ||
| [ '--trace-atomics-wait', __filename, 'child' ], | ||
| { encoding: 'utf8', stdio: [ 'inherit', 'inherit', 'pipe' ] }); | ||
|
|
||
| if (proc.status !== 0) console.log(proc); | ||
| assert.strictEqual(proc.status, 0); | ||
|
|
||
| const SABAddress = proc.stderr.match(/Atomics\.wait\((?<SAB>.+) \+/).groups.SAB; | ||
| const actualTimeline = proc.stderr | ||
| .replace(new RegExp(SABAddress, 'g'), '<address>') | ||
| .replace(new RegExp(`\\(node:${proc.pid}\\) `, 'g'), '') | ||
| .replace(/\binf(inity)?\b/gi, 'inf') | ||
| .replace(/\r/g, '') | ||
| .trim(); | ||
| console.log('+++ normalized stdout +++'); | ||
| console.log(actualTimeline); | ||
| console.log('--- normalized stdout ---'); | ||
|
|
||
| const begin = | ||
| `[Thread 0] Atomics.wait(<address> + 0, 1, inf) started | ||
| [Thread 0] Atomics.wait(<address> + 0, 1, inf) did not wait because the \ | ||
| values mismatched | ||
| [Thread 0] Atomics.wait(<address> + 0, 0, 10) started | ||
| [Thread 0] Atomics.wait(<address> + 0, 0, 10) timed out`; | ||
|
|
||
| const expectedTimelines = [ | ||
| `${begin} | ||
| [Thread 0] Atomics.wait(<address> + 4, 0, inf) started | ||
| [Thread 1] Atomics.wait(<address> + 4, -1, inf) started | ||
| [Thread 0] Atomics.wait(<address> + 4, 0, inf) was woken up by another thread | ||
| [Thread 1] Atomics.wait(<address> + 4, -1, inf) was woken up by another thread`, | ||
| `${begin} | ||
| [Thread 0] Atomics.wait(<address> + 4, 0, inf) started | ||
| [Thread 0] Atomics.wait(<address> + 4, 0, inf) was woken up by another thread | ||
| [Thread 1] Atomics.wait(<address> + 4, -1, inf) started | ||
| [Thread 1] Atomics.wait(<address> + 4, -1, inf) did not wait because the \ | ||
| values mismatched`, | ||
| `${begin} | ||
| [Thread 0] Atomics.wait(<address> + 4, 0, inf) started | ||
| [Thread 1] Atomics.wait(<address> + 4, -1, inf) started | ||
| [Thread 0] Atomics.wait(<address> + 4, 0, inf) was woken up by another thread | ||
| [Thread 1] Atomics.wait(<address> + 4, -1, inf) did not wait because the \ | ||
| values mismatched`, | ||
| `${begin} | ||
| [Thread 0] Atomics.wait(<address> + 4, 0, inf) started | ||
| [Thread 0] Atomics.wait(<address> + 4, 0, inf) did not wait because the \ | ||
| values mismatched | ||
| [Thread 1] Atomics.wait(<address> + 4, -1, inf) started | ||
| [Thread 1] Atomics.wait(<address> + 4, -1, inf) did not wait because the \ | ||
| values mismatched` | ||
| ]; | ||
|
|
||
| assert(expectedTimelines.includes(actualTimeline)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might as well just don't print the timeout if it's infinity to match what you'd expect the call would look like
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t mind being explicit here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer being explicit here also. The only think I would worry about is using the abbreviation
infforInfinity. It's a minor issue, however, as folks who would typically be looking at this should be sophisticated enough in their understanding of the mechanism to know whatinfmeans.