Skip to content

Commit a0dfd45

Browse files
committed
fix: set wallprecheck default flag to false
1 parent 62941d5 commit a0dfd45

5 files changed

Lines changed: 21 additions & 16 deletions

File tree

ddprof-lib/src/main/cpp/arguments.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ class Arguments {
205205
_cpu(-1),
206206
_wall(-1),
207207
_wall_collapsing(false),
208-
_wall_precheck(true),
208+
_wall_precheck(false),
209209
_wall_threads_per_tick(DEFAULT_WALL_THREADS_PER_TICK),
210210
_wallclock_sampler(ASGCT),
211211
_memory(-1),

ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/PrecheckEfficiencyTest.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,11 @@
2525
* wallprecheck=false} so every state is sampled; percentages estimate how often {@code
2626
* wallprecheck=true} would skip {@code SIGVTALRM} based on HotSpot {@code OSThreadState}.
2727
*
28-
* <p><strong>OS-state precheck (default {@code wallprecheck=true})</strong> skips signals when the
29-
* JVM reports {@code SLEEPING} or {@code CONDVAR_WAIT}. That covers Thread.sleep (legacy {@code
30-
* SLEEPING}; JDK 21+ sleep uses wait/park), {@code LockSupport.park}, and other condvar waits — not
31-
* {@code Object.wait} ({@code OBJECT_WAIT}), which the current precheck does not skip.
28+
* <p><strong>OS-state precheck (opt-in via {@code wallprecheck=true}; default is {@code false})</strong>
29+
* skips signals when the JVM reports {@code SLEEPING} or {@code CONDVAR_WAIT}. That covers
30+
* Thread.sleep (legacy {@code SLEEPING}; JDK 21+ sleep uses wait/park), {@code LockSupport.park}, and
31+
* other condvar waits — not {@code Object.wait} ({@code OBJECT_WAIT}), which the current precheck
32+
* does not skip.
3233
*
3334
* <p>Sample classification prefers Java thread name ({@code EVENT_THREAD_NAME}) for the fixed worker
3435
* threads when present, then JFR thread state, then stack strings — some JVMs (e.g. Graal on aarch64)

ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/PrecheckTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import static org.junit.jupiter.api.Assertions.assertTrue;
1212

1313
/**
14-
* Verifies OS-thread-state precheck ({@code wallprecheck=true}, the default): the wall-clock timer
14+
* Verifies OS-thread-state precheck (opt-in via {@code wallprecheck=true}): the wall-clock timer
1515
* thread reads {@code VMThread::osThreadState()} before {@code SIGVTALRM} and skips sending the
1616
* signal when HotSpot reports {@code SLEEPING} or {@code CONDVAR_WAIT}. Legacy JDKs report {@code
1717
* Thread.sleep} as {@code SLEEPING}; JDK 21+ uses a condvar wait path for sleep (often {@code
@@ -59,6 +59,6 @@ public void testSleepingThreadIsNotSampled() throws InterruptedException {
5959

6060
@Override
6161
protected String getProfilerCommand() {
62-
return "wall=1ms"; // wallprecheck=true by default
62+
return "wall=1ms,wallprecheck=true";
6363
}
6464
}

ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/WallclockMitigationsCombinedTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,10 @@ public void precheckAndParkSuppressionWorkTogether() throws Exception {
116116
}
117117
}
118118

119-
/** Enables wall-clock profiling with default precheck behavior for combined assertions. */
119+
/** Enables wall-clock profiling with the OS-state precheck explicitly turned on so both
120+
* Approach A (precheck) and Approach B (park flag) are exercised in this combined test. */
120121
@Override
121122
protected String getProfilerCommand() {
122-
return "wall=1ms,filter=0";
123+
return "wall=1ms,filter=0,wallprecheck=true";
123124
}
124125
}

doc/architecture/TLSContext.md

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -508,12 +508,13 @@ Full benchmark data and analysis:
508508
snapshots for wall-clock collapsing and TaskBlock emission. The parked flag suppresses
509509
wall-clock signals on filtered threads when enabled.
510510

511-
### `wallprecheck` (default `true`)
511+
### `wallprecheck` (default `false`, opt-in)
512512

513-
When thread filtering is enabled and `Arguments::_wall_precheck` is true, the wall-clock
514-
timer samples **HotSpot** `VMThread::osThreadState()` **before** sending `SIGVTALRM`.
515-
Slots must hold a non-null `VMThread*` from registration (`ThreadFilter::setVMThread`)
516-
for the precheck to run; otherwise only Java park-flag suppression applies.
513+
Disabled by default. When explicitly enabled with `wallprecheck=true` *and* thread filtering
514+
is on (`Arguments::_wall_precheck == true`), the wall-clock timer samples **HotSpot**
515+
`VMThread::osThreadState()` **before** sending `SIGVTALRM`. Slots must hold a non-null
516+
`VMThread*` from registration (`ThreadFilter::setVMThread`) for the precheck to run;
517+
otherwise only Java park-flag suppression applies.
517518

518519
On JDK **21+**, `Thread.sleep` often maps to **CONDVAR_WAIT** (not only legacy **SLEEPING**),
519520
same broad category as condvar-based park — see `PrecheckTest` and `wallClock.cpp`.
@@ -524,8 +525,10 @@ JVMTI `MonitorWait` / `MonitorWaited` callbacks are registered only when
524525
`java_version() < 21` (`vmEntry.cpp`). They populate TaskBlock with a **blocker** value
525526
matching **`System.identityHashCode(waitedObject)`** (same convention as the JNI park path).
526527

527-
For **`wallprecheck=false`** (isolation tests such as `ParkTaskBlockTest`), OS-thread-state
528-
suppression is disabled so parked-flag behavior can be tested without condvar/sleep overlap.
528+
With the default **`wallprecheck=false`**, OS-thread-state suppression is off, so parked-flag
529+
behavior can be tested without condvar/sleep overlap (isolation tests such as
530+
`ParkTaskBlockTest`). The precheck path is exercised by `PrecheckTest` and
531+
`WallclockMitigationsCombinedTest`, which both pass `wallprecheck=true` explicitly.
529532

530533
## OTEP References
531534

0 commit comments

Comments
 (0)