Store sampling context in ContinuationPreservedEmbedderData #186

szegedi · 2024-12-11T16:22:57Z

What does this PR do?:
Moves profiler's sampling context into the V8 isolate's ContinuationPreservedEmbedderData (CPED.) In fact, moves it into a private symbol-keyed property within the object as Node.js version 24 and later will store AsyncContextFrame objects in the CPED.

Motivation:
Before this change, we rely on async_hooks in dd-trace-js to update the sampling context on every context switch. It is both somewhat costly to do this on every async context switch and we should also migrate away from async hooks, as profiling is their last user, they'll be deprecated in Node, and they bring their own set of performance overhead by using them.

This PR comes with a document explaining the design in it.

Additional Notes:

The code using this in dd-trace-js' wall.js sets the context on AsyncLocalStorage.{enterWith|run}, and the before callback of AsyncHooks.createHook. With CPED storage enabled, we can eliminate the before callback and "only" keep the AsyncLocalStorage.{enterWith|run} callbacks – they are the minimum amount of instrumentation needed for the feature to work correctly then.

This branch can be tested with the same-named branch in dd-trace-js.

Jira: PROF-11771

datadog-datadog-prod-us1 · 2025-02-27T10:33:16Z

ts/src/time-profiler.ts

@@ -66,6 +66,7 @@ export interface TimeProfilerOptions {
  withContexts?: boolean;
  workaroundV8Bug?: boolean;
  collectCpuTime?: boolean;
+  useCPED?: boolean;


⚪ Code Quality Violation

Prop name (useCPED) doesn't match rule (is|has) (...read more)

Enforces a consistent naming pattern for boolean props.

The pattern is: "^(is|has)[A-Z]([A-Za-z0-9]?)+" to enforce is and has prefixes.

github-actions · 2025-03-20T15:16:29Z

Overall package size

Self size: 1.62 MB
Deduped: 1.99 MB
No deduping: 1.99 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | source-map | 0.7.4 | 226 kB | 226 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | p-limit | 3.1.0 | 7.75 kB | 13.78 kB | | delay | 5.0.0 | 11.17 kB | 11.17 kB | | node-gyp-build | 3.9.0 | 8.81 kB | 8.81 kB |

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

pr-commenter · 2025-03-20T15:17:16Z

Benchmarks

Benchmark execution time: 2025-06-25 12:14:59

Comparing candidate commit 4916ae7 in PR branch szegedi/cped-context with baseline commit 35b1a4f in branch main.

Found 4 performance improvements and 0 performance regressions! Performance is the same for 88 metrics, 28 unstable metrics.

scenario:profiler-heavy-load-no-wall-profiler-22

🟩 cpu_user_time [-21.749ms; -5.730ms] or [-6.022%; -1.587%]

scenario:profiler-light-load-no-wall-profiler-18

🟩 cpu_user_time [-9.313ms; -1.376ms] or [-10.301%; -1.522%]

scenario:profiler-light-load-no-wall-profiler-22

🟩 cpu_user_time [-7.611ms; -1.403ms] or [-10.240%; -1.888%]

scenario:profiler-light-load-no-wall-profiler-24

🟩 cpu_user_time [-7.640ms; -1.590ms] or [-9.830%; -2.046%]

…preserved embedder data

Also optimize away some setting of null/undefined as context

nsavoire · 2025-06-24T13:49:17Z

bindings/profilers/wall.cc

@@ -59,6 +58,89 @@ using namespace v8;

 namespace dd {

+class SignalMutex {


nit: sounds more like a Guard or a Lock

nsavoire · 2025-06-24T13:50:07Z

bindings/profilers/wall.cc

+
+  ContextPtr Get() const { return context; }
+
+  friend class WallProfiler;


nit: Why not make methods public instead ?

nsavoire · 2025-06-24T13:51:52Z

bindings/profilers/wall.cc

+
+// This class is used to update the context count metric when it goes out of
+// scope.
+class ContextCountMetricUpdater {


nit: you could use a defer instead

nsavoire

Really nice work !

- SignalMutex -> SignalGuard - Use defer instead of ContextCountMetricUpdater - make PersistentContextPtr methods public instead of declaring a friend class

r1viollet · 2025-06-25T14:25:35Z

🚀

szegedi changed the base branch from main to szegedi/atomic-context December 11, 2024 20:10

szegedi force-pushed the szegedi/cped-context branch from 5b796ea to 1b15c3a Compare December 11, 2024 20:48

szegedi changed the base branch from szegedi/atomic-context to main December 11, 2024 20:53

szegedi force-pushed the szegedi/cped-context branch from 1b15c3a to 3decb46 Compare December 11, 2024 21:04

szegedi mentioned this pull request Dec 11, 2024

use atomic load/store on curContext_ #187

Closed

szegedi force-pushed the szegedi/cped-context branch 4 times, most recently from bfa23b5 to 32b2170 Compare December 17, 2024 11:26

szegedi force-pushed the szegedi/cped-context branch from 32b2170 to 9f398f2 Compare January 20, 2025 16:43

szegedi mentioned this pull request Feb 26, 2025

Various improvements to profiling sample label generator DataDog/dd-trace-js#5316

Merged

szegedi force-pushed the szegedi/cped-context branch from 9f398f2 to 965a627 Compare February 26, 2025 16:40

szegedi changed the base branch from main to szegedi/async-id-2 February 26, 2025 16:41

szegedi force-pushed the szegedi/cped-context branch from 965a627 to 0c1791e Compare February 26, 2025 16:51

szegedi force-pushed the szegedi/async-id-2 branch from a2384e5 to bb0bc2d Compare February 27, 2025 10:30

szegedi force-pushed the szegedi/cped-context branch from 0c1791e to 8f88f3d Compare February 27, 2025 10:32

datadog-datadog-prod-us1 bot reviewed Feb 27, 2025

View reviewed changes

szegedi force-pushed the szegedi/cped-context branch from 8f88f3d to 717f17b Compare March 5, 2025 12:53

szegedi force-pushed the szegedi/async-id-2 branch from 3c86526 to 79992a0 Compare March 6, 2025 17:05

Base automatically changed from szegedi/async-id-2 to main March 19, 2025 15:27

szegedi force-pushed the szegedi/cped-context branch from 717f17b to 2b9041d Compare March 20, 2025 15:12

szegedi changed the title ~~Experiment for storing profiling context in the ContinuationPreservedEmbeddedData~~ Store profiling context in ContinuationPreservedEmbeddedData Mar 20, 2025

szegedi changed the title ~~Store profiling context in ContinuationPreservedEmbeddedData~~ Store sampling context in ContinuationPreservedEmbeddedData Mar 20, 2025

szegedi added the semver-minor Usually minor non-breaking improvements label Mar 20, 2025

szegedi force-pushed the szegedi/cped-context branch from 2b9041d to c475e2f Compare March 20, 2025 16:14

szegedi force-pushed the szegedi/cped-context branch 3 times, most recently from 9ba3bf1 to 9c4090e Compare May 7, 2025 12:00

szegedi added 9 commits June 17, 2025 15:37

format

d0537a9

Encapsulate the pair of pointers with atomic switch into a class

916d921

Allow use of CPED to store sampling context

c837e59

Test useCPED when on Node 24

23889f0

Design doc explaining how the sampling context works in continuation-…

aa86b63

…preserved embedder data

Addressing simpler points of review feedback

79114c1

Remove AtomicContextPtr in favor of setInProgress_

9ae8154

Move PersistentContextPtr to top

5e2043c

Expose a counter of number of live context pointers.

5b06bbe

Also optimize away some setting of null/undefined as context

szegedi force-pushed the szegedi/cped-context branch from 34b19c4 to 5b06bbe Compare June 17, 2025 13:48

nsavoire reviewed Jun 24, 2025

View reviewed changes

nsavoire approved these changes Jun 24, 2025

View reviewed changes

nsavoire previously approved these changes Jun 24, 2025

View reviewed changes

Review feedback:

604518f

- SignalMutex -> SignalGuard - Use defer instead of ContextCountMetricUpdater - make PersistentContextPtr methods public instead of declaring a friend class

szegedi dismissed nsavoire’s stale review via 604518f June 25, 2025 09:19

szegedi requested a review from nsavoire June 25, 2025 09:19

nsavoire previously approved these changes Jun 25, 2025

View reviewed changes

Fix format

4916ae7

szegedi dismissed nsavoire’s stale review via 4916ae7 June 25, 2025 12:10

szegedi enabled auto-merge (squash) June 25, 2025 12:48

nsavoire approved these changes Jun 25, 2025

View reviewed changes

szegedi merged commit 868fca6 into main Jun 25, 2025
56 checks passed

szegedi deleted the szegedi/cped-context branch June 25, 2025 13:41

szegedi mentioned this pull request Jun 25, 2025

Update package-lock.json after previous commit #223

Merged

szegedi added a commit that referenced this pull request Jun 26, 2025

Store sampling context in ContinuationPreservedEmbedderData (#186)

98aba40

szegedi mentioned this pull request Jun 26, 2025

v5.9.0 #224

Merged

szegedi added a commit that referenced this pull request Jun 26, 2025

Store sampling context in ContinuationPreservedEmbedderData (#186)

d5b5283

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Store sampling context in ContinuationPreservedEmbedderData #186

Store sampling context in ContinuationPreservedEmbedderData #186

Uh oh!

szegedi commented Dec 11, 2024 •

edited

Loading

Uh oh!

datadog-datadog-prod-us1 bot Feb 27, 2025

Uh oh!

github-actions bot commented Mar 20, 2025 •

edited

Loading

Uh oh!

pr-commenter bot commented Mar 20, 2025 •

edited

Loading

Uh oh!

nsavoire Jun 24, 2025

Uh oh!

nsavoire Jun 24, 2025

Uh oh!

nsavoire Jun 24, 2025

Uh oh!

nsavoire left a comment

Uh oh!

Uh oh!

r1viollet commented Jun 25, 2025

Uh oh!

Uh oh!

		@@ -59,6 +58,89 @@ using namespace v8;

		namespace dd {

		class SignalMutex {


		ContextPtr Get() const { return context; }

		friend class WallProfiler;

Store sampling context in ContinuationPreservedEmbedderData #186

Store sampling context in ContinuationPreservedEmbedderData #186

Uh oh!

Conversation

szegedi commented Dec 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

datadog-datadog-prod-us1 bot Feb 27, 2025

Choose a reason for hiding this comment

⚪ Code Quality Violation

Uh oh!

github-actions bot commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overall package size

Uh oh!

pr-commenter bot commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

scenario:profiler-heavy-load-no-wall-profiler-22

scenario:profiler-light-load-no-wall-profiler-18

scenario:profiler-light-load-no-wall-profiler-22

scenario:profiler-light-load-no-wall-profiler-24

Uh oh!

nsavoire Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

nsavoire Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

nsavoire Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

nsavoire left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

r1viollet commented Jun 25, 2025

Uh oh!

Uh oh!

szegedi commented Dec 11, 2024 •

edited

Loading

github-actions bot commented Mar 20, 2025 •

edited

Loading

pr-commenter bot commented Mar 20, 2025 •

edited

Loading