docs: add sftrace skill for XRay performance tracing#179
docs: add sftrace skill for XRay performance tracing#179stormslowly wants to merge 3 commits intomainfrom
Conversation
Migrate sftrace skill from rspack monorepo, adapted for rspack-resolver: - Use RUSTFLAGS for XRay instrumentation instead of SFTRACE=1 env - Reference resolver binding path instead of rspack binding - Use pnpm test as default traced command
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The new sftrace skill documentation is a valuable addition, providing clear steps for performance analysis. I've identified a couple of areas where the documentation could be improved for better maintainability and user experience. Specifically, the BENCH_BIN definition is repeated multiple times, and the TRACE_DIR variable in the optional analysis and visualization steps could be made more explicit to avoid manual replacement.
| BENCH_BIN="$(ls -t target/profiling/deps/resolver-* | grep -v '\.d$' | head -1)" | ||
| ``` |
There was a problem hiding this comment.
The command to define BENCH_BIN is repeated several times throughout the document (e.g., lines 47-48, 58-59, 83-84). To improve maintainability and reduce redundancy, consider defining this variable once at the beginning of the 'Workflow' section or at least before its first use, and then referencing it. This ensures that if the naming convention or path changes, only one place needs to be updated.
| BENCH_BIN="$(ls -t target/profiling/deps/resolver-* | grep -v '\.d$' | head -1)" | |
| ``` | |
| BENCH_BIN="$(ls -t target/profiling/deps/resolver-* | grep -v '\.d$' | head -1)" | |
There was a problem hiding this comment.
Pull request overview
Adds a new .agents skill document describing how to use sftrace (LLVM XRay instrumentation) to collect and analyze function-level traces for rspack-resolver, including Node binding and cargo bench workflows.
Changes:
- Introduces an end-to-end
sftracetracing workflow (build, instrument, record) for both N-API tests and the Criterion benchmark binary. - Adds optional filtering guidance via symbol extraction and
sftrace filter. - Documents post-processing workflows for analysis (polars) and visualization (perfetto protobuf output).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| | name | type | description | | ||
| | -------- | ----------- | -------------------------------------------------------------------------- | | ||
| | frame_id | uint64 | a unique id for each frame. a function's entry and exit have same frame id | | ||
| | parent | uint64 | point to previous frame id. zero means non-existent | | ||
| | tid | uint32 | thread id | |
| git clone https://github.com/quininer/sftrace | ||
| cd sftrace | ||
| cargo build --release | ||
| mkdir "$(./target/release/sftrace record --print-solib-install-dir)" | ||
| cp ./target/release/libsftrace.so "$(./target/release/sftrace record --print-solib-install-dir)/" | ||
| ``` |
| XRay instrumentation must be enabled via RUSTFLAGS. There are two targets: the Node.js binding and the Rust benchmark binary. | ||
|
|
||
| #### 2a) Node.js binding | ||
|
|
||
| ```sh | ||
| RUSTFLAGS="-Zinstrument-xray=always" pnpm build:binding:profiling | ||
| ``` | ||
|
|
||
| The `.node` binding file will be output to the project root (e.g. `resolver.darwin-arm64.node`). | ||
|
|
||
| #### 2b) Benchmark binary | ||
|
|
||
| ```sh | ||
| RUSTFLAGS="-Zinstrument-xray=always" cargo +nightly bench --profile profiling --no-run | ||
| ``` |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2bc192b3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Full trace | ||
| sftrace record -o "$TRACE_DIR/sf.log" -- "$BENCH_BIN" --bench |
There was a problem hiding this comment.
--bench flag from direct bench runs
These commands execute the compiled benchmark binary directly, but this repo’s bench target is harness = false (Cargo.toml:23-25) and uses criterion_main!(resolver) (benches/resolver.rs:357-358). In codspeed-criterion-compat, that generated main only parses an optional positional FILTER and --exact, so passing libtest’s --bench flag makes the binary exit on an unknown argument instead of recording any trace.
Useful? React with 👍 / 👎.
Summary
pnpm test) andcargo benchbenchmark binaryTest plan
RUSTFLAGS="-Zinstrument-xray=always" pnpm build:binding:profilingbuilds correctlyRUSTFLAGS="-Zinstrument-xray=always" cargo +nightly bench --profile profiling --no-runproduces instrumented binarysftrace recordcaptures traces from both binding and benchmark targets