Skip to content

Move profiler_tick into spawn_blocking#132

Merged
rcoh merged 3 commits into
mainfrom
spawn-blocking-ops
May 15, 2026
Merged

Move profiler_tick into spawn_blocking#132
rcoh merged 3 commits into
mainfrom
spawn-blocking-ops

Conversation

@rcoh
Copy link
Copy Markdown
Collaborator

@rcoh rcoh commented Apr 14, 2026

📬 Issue #, if available: #131

✍️ Description of changes: dial9 revealed long polls caused by profiler_tick (~20ms)

🔏 By submitting this pull request

  • I confirm that I've made a best effort attempt to update all relevant documentation.
  • I confirm that my contribution is made under the terms of the Apache 2.0 license.

@rcoh rcoh force-pushed the spawn-blocking-ops branch from ce2e9de to 7d3402f Compare April 14, 2026 17:33
…ment

- Add TickError::JfrFileMissing variant instead of misusing EmptyInactiveFile
  for the case where jfr_file is None in tick_blocking
- Clarify Drop comment to note it intentionally skips the status reset
  that self.stop() would do since the struct is being dropped
@rcoh rcoh marked this pull request as ready for review May 5, 2026 17:59
@rcoh rcoh requested review from jlizen May 5, 2026 18:01
Copy link
Copy Markdown
Collaborator

@jlizen jlizen left a comment

Choose a reason for hiding this comment

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

So we still block in the ProfilerState::Drop and try_final_report? Seems fine, this is a clear improvement.

Comment thread src/profiler.rs
)
}

async fn profiler_tick<E: ProfilerEngine>(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should we add a doc comment that this is not cancel-safe anymore? since if it is dropped inside an eg select loop, we permanently lose profiler state

Comment thread src/profiler.rs
};

let report_metadata = ReportMetadata {
instance: agent_metadata.as_ref().unwrap(),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

good cleanup

@jlizen
Copy link
Copy Markdown
Collaborator

jlizen commented May 14, 2026

@rcoh this one is ready for you if you didn't see, idk if you care to make the tweaks or merge

@rcoh rcoh merged commit f111162 into main May 15, 2026
22 checks passed
@rcoh rcoh deleted the spawn-blocking-ops branch May 16, 2026 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants