Speedscope export#49
Open
mjambon wants to merge 7 commits into
Open
Conversation
Adds `format=speedscope` to `OCAML_LANDMARKS`, which writes a sampled flame-graph profile openable at https://www.speedscope.app. Implementation: - `src/speedscope_fmt.atd` — ATD schema for the Speedscope file format (source of truth; regenerate with `atdml speedscope_fmt.atd`) - `src/speedscope_fmt.ml[i]` — generated types + Yojson serialisers, tracked in git so the build has no atdml dependency - `src/graph.ml` — `Graph.Speedscope` submodule: DFS over the call graph producing one sampled stack per node with positive self-time; uses `sys_time` (seconds) when collected, CPU cycles (`unit: none`) otherwise - `src/landmark.ml[i]` — new `Speedscope` variant of `profile_format`, env-var parsing, at-exit hook wiring - `src/dune` / `dune-project` — add `yojson` dependency - `tests/speedscope/` — hand-instrumented example with pre-generated `profile.json` checked in for read-without-running convenience Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Move Speedscope export to a standalone Speedscope module (src/speedscope.ml[i]) instead of a submodule of Graph; expose it as Landmark.Speedscope - Regenerate speedscope_fmt.ml[i] using atdml (fixed ATD syntax: annotations need '=', 'shared' is a reserved ATD keyword, 'None' shadows Stdlib.None) - Rename ATD fields per review: unit_ → unit, dollar_schema → schema, shared → profile_shared; use None_ for the "none" value_unit variant - Add link to the TypeScript spec in the ATD file header - Wrap Speedscope doc comment in landmark.mli to ≤80 columns - Rewrite example using PPX [@landmark] decorators with a 3-level call tree (main → prepare/run/summarise, run → phase_a/phase_b) - Add deterministic test (test.ml builds a fixed Graph and exports it; test.out.expected is checked in and verified by dune runtest) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All applicable field and type comments from file-format-spec.ts are now expressed as ATD doc annotations and flow through into the generated speedscope_fmt.mli as OCaml doc comments. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Move long URLs in the file header onto their own indented lines - Condense <doc text="..."> strings that exceeded the column limit - Regenerate speedscope_fmt.ml[i] Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
|
Do you think that a lot of infrastructure is missing so that new export formats could be implemented as a separate library (we can still keep it inside this dune project)? In that case, using |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hello @mlasson,
On @nojb's suggestion, I added Speedscope export to Landmarks. Here's a PR that I created with AI assistance. It's ready for review. Please let me know what you think.
Supported format
Among the various formats supported by Speedscope, three of them are documented as generic, and one of them is Speedscope's own format. I decided to target it and it seems to work fine. I don't have big programs to test it on, though.
JSON support
The original implementation uses it own tiny implementation for exporting to JSON. I'm not sure why but if I assumed it's a matter of reducing external dependencies. Keeping this in mind, here's what I did:
atdmlis not an Opam dependency of thelandmarkspackage. A call toatdml speedscope_fmt.atdwill regeneratespeedscope_fmt.mliandspeedscope_fmt.mlwhen the need arises. These two files are kept alongside source code insrc/.Example
Download
tests/speedscope/profile.jsonand then upload it at https://www.speedscope.app/.Code quality
I paid reasonable attention to the interfaces and to the tests. I did not review
src/speedscope.mlclosely. Trying it out on non-trivial example would be great.