Repository navigation
Add async-profiler 4.5 (from source) - #900
Conversation
Low-overhead sampling CPU/alloc/lock profiler for the JVM. Builds the native agent and asprof with upstream's Makefile, rebuilds the embedded helper classes from source, and writes the API / converter jars with DeterministicJar (sorted entries, fixed timestamp). Tests use event=itimer so they need no perf_events privileges. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/build |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: adding async-profiler version 4.5 from source. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
The source build is solid: it recompiles the prebuilt .class files, libstdc++ is statically linked so glibc alone is right, the globs match, and the itimer tests are good. Changes needed:
build.sh:13re-exportsSOURCE_DATE_EPOCH=1784505600. AGENTS.md says the sandbox sets it and the pattern is to override the consumer instead (seepackages/nspr/build.sh); no other package sets it non-zero. It isn't needed: with the sandbox's 0,__DATE__and thejfrconvdate at line 73 are already deterministic. Drop it, or scope it to thesedat line 73. (minimal-checkonly catches=0; the policy still applies.)- Rebase needed. The branch predates #897, which made
jdkpoint at Temurin 25. CI built and tested against 21 but it will ship on 25. Please rebase and re-run/build. The PR body's "Temurin 21's javac" also needs updating.--release 8still works on 25, but the rebuild should show it. - Nit:
DeterministicJar.java:5is an identical copy frompackages/tlaplus, and its comment still explains TLA+'s need for directory entries. Please make the comment generic.
Generated by Claude Code
Review fixes: drop the hardcoded SOURCE_DATE_EPOCH (the sandbox already sets one, so __DATE__ and jfrconv's build date stay deterministic), and make the DeterministicJar.java header describe the general case instead of tlaplus's lookup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @norrietaylor. All three are addressed in 2b2fc5d (on top of a merge of main, c4db3f8):
🤖 Generated with Claude Code |
There was a problem hiding this comment.
All three points are addressed:
- The
SOURCE_DATE_EPOCHoverride is gone; it's only read atbuild.sh:74, underset -u. - #897 is merged in, so
jdkresolves to jdk-25, and the PR body is updated. - The
DeterministicJarheader is generic.
One thing before merge: the minimal build on 2b2fc5d took 52s, the same as the original JDK 21 run, and it has no output. So GitHub doesn't show whether the package and its tests really rebuilt on Temurin 25. Please confirm on minimal.dev or post /build.
Nit: DeterministicJar.java:9 says to keep the two copies in step, but packages/tlaplus/DeterministicJar.java doesn't point back.
Generated by Claude Code
|
@norrietaylor, here's the confirmation that #900 rebuilt on Temurin 25, from the build bot's own records. Our branch can't take The run for 2b2fc5d (run
The JDK changed between the two heads, so the input closure is new:
The signed spec hashes match On the DeterministicJar nit: agreed. I'll add the pointer back in 🤖 Generated with Claude Code |
What
Adds
packages/async-profiler(v4.5, Apache-2.0), the low-overhead sampling CPU / allocation / lock profiler for the JVM. It is item "async-profiler" in the Java loadout checklist in #899.Built from source, not from the per-arch release tarballs:
libasyncProfiler.soandasprofcome from upstream's Makefile (targetsbuild/lib/libasyncProfiler.so build/bin/asprof). Upstream'salltarget downloads nothing; only the test targets fetch jars, and they are not run..incbin(src/helper/one/profiler/*.class) are committed prebuilt in upstream's tree.build.shdeletes them and recompiles them from the.javanext to them, using upstream's own%.classrule (-source 8 -target 8 -g:none).async-profiler.jar(Java API) andjfr-converter.jarare compiled withjavac --release 8, as upstream does. They are written with theDeterministicJarhelper frompackages/tlaplus(sorted entries, one fixed timestamp), becausejar cfm/cfestamps build time on every entry.jfrconvis upstream's/bin/shlauncher with the converter jar appended, assembled the same way the Makefile does it.Layout
usr/bin/asprof../lib/libasyncProfiler.so, so/usr/bin+/usr/libkeeps upstream'sbin/+lib/relationship.usr/bin/jfrconvusr/lib/libasyncProfiler.sousr/include/asprof.husr/share/async-profiler/{async-profiler,jfr-converter}.jarruntime_deps:
glibc, plusbashfor thejfrconvlauncher andjdk. There is nolibstdc++orlibgccdependency, because upstream links both statically into the.so(-static-libstdc++ -static-libgcc).asprofis linked with the gcc driver and pulls in no C++ runtime.Reproducibility
-ffile-prefix-map=$(pwd)=/builddir -gno-record-gcc-switches -Wl,--build-id=none. These go throughCFLAGS_EXTRA/CXXFLAGS_EXTRA, because the Makefile's recipes ignoreLDFLAGS.asprof --versionand jattach print__DATE__, andjfrconv --versiongets a build date instead of$(shell date). All of them come from the sandbox'sSOURCE_DATE_EPOCH, so they're deterministic without the package setting one.strip -gis applied to the.so.Tests
None of the tests need perf privileges. Every profiling run uses
event=itimer(setitimer + SIGPROF), because the defaultevent=cpuneeds perf_events, which sandboxes usually forbid.smoke:asprof --versionandjfrconv --versionboth report 4.5.agent: compiles a busy-loop program, runs it with-agentpath:/usr/lib/libasyncProfiler.so=start,event=itimer,file=out.collapsed, and requiresBusy.burnCpuin the stacks. It repeats the run with.jfroutput, then requiresjfrconvto produce collapsed stacks containing the same frame and an HTML flame graph.attach:asprof -e itimer -d 2attaches to an already-running JVM through HotSpot dynamic attach (no ptrace) and must seeSpin.spinLoop.Notes for review
DeterministicJar.javais now duplicated withpackages/tlaplus. Sharing it would need a common package; I left that for later.jdk, since jdk: current LTS (meta over jdk-25); add jdk-25 (Temurin 25) and jdk-21 (Temurin 21) #897) rather than upstream's build JDK, the bytes differ from upstream's committed.classfiles. They are still class-file version 52 (Java 8).Refs #899
🤖 Generated with Claude Code
Summary by CodeRabbit
asprofandjfrconvcommand-line tools, a native profiling agent, and Java profiling libraries.