Skip to content

Cache the Starlark profiler's native thread ID - #31185

Closed
dzbarsky wants to merge 1 commit into
bazelbuild:masterfrom
dzbarsky:codex/cache-profiler-tid-upstream
Closed

dzbarsky wants to merge 1 commit into
bazelbuild:masterfrom
dzbarsky:codex/cache-profiler-tid-upstream

Conversation

@dzbarsky

@dzbarsky dzbarsky commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Cache the native thread ID in a Java ThreadLocal<Integer> in CpuProfiler. Repeated registration and restoration of StarlarkThread now reuse the cached ID, avoiding both JNI calls and native thread-ID lookups. The C++ implementation is unchanged.

Starlark map_each repeatedly registers and restores the current thread while CPU profiling is enabled.

Validation: EvalTests and ScriptTest passed. A two-thread check confirmed one native lookup per thread across 2,000 nested registrations, and a CPU profiling run produced 207 samples with the expected function names. Google Java Format and git diff --check passed.

RELNOTES: None

-zbarskybot

@github-actions github-actions Bot added team-Starlark-Interpreter Issues involving the Starlark interpreter used by Bazel awaiting-review PR is awaiting review from an assigned reviewer labels Sep 18, 2026
@fmeum
fmeum requested a review from meisterT September 18, 2026 07:11
JNIEnv* env, jobject instance) {
return gettid();
// Keep TLS initialization out of gettid(), which the signal handler also uses.
static thread_local const pid_t tid = gettid();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of doing this - or in addition? - should we do add a ThreadLocal<Integer> on the Java side?

Use a ThreadLocal<Integer> in CpuProfiler to reuse the native thread ID
when registering and restoring StarlarkThread instances. This avoids
repeated JNI calls as well as native thread-ID lookups during map_each.
Leave the native implementation and signal handler unchanged.

Validation: EvalTests and ScriptTest passed. A two-thread check confirmed
one native lookup per thread across 2,000 nested registrations. An actual
CPU profiling run produced samples with the expected function names.

RELNOTES: None
@dzbarsky
dzbarsky force-pushed the codex/cache-profiler-tid-upstream branch from 865777c to d528eb2 Compare October 6, 2026 06:31
@meisterT meisterT added copybara:import Trigger copybara to import the PR and removed awaiting-review PR is awaiting review from an assigned reviewer labels Oct 6, 2026
@copybara-service copybara-service Bot closed this in e3af39d Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

copybara:import Trigger copybara to import the PR team-Starlark-Interpreter Issues involving the Starlark interpreter used by Bazel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants