Add call tree JSON export and web viewer prototype - #32
Conversation
|
Thanks for taking a look. I wanted to keep this change intentionally scoped to a native JSON export plus a small browser prototype, so it can be evaluated as a maintainable extension of TTU’s existing engine rather than a separate reimplementation of trace parsing. Happy to adjust scope, structure, or presentation if there’s a preferred direction for something like this. |
statham-arm
left a comment
There was a problem hiding this comment.
Hello! Thanks for this PR.
I agree that some kind of web frontend would be a nice thing to have. I've had thoughts along those lines myself in the past, but I don't really have the web front end skills to do much with them.
For the same reason, it's going to take me some time to review all of this code. My Javascript is pretty out of date, and I've already spotted three JS features you've used that I didn't already know about. Which is a good thing – I'm learning! – but it will slow down my review. Sorry about that.
But as you say, the details of the JS here are secondary, because it's a sample front end and not the core functionality of the patch. So I've focused my first few review comments on the JSON, which is the most important thing to get right, since it's the stable protocol between the front and back ends.
| json_escape(os, IN.get_symbolic_address(function_entry.addr, true)); | ||
| os << "\n"; | ||
| json_indent(os, depth + 1); | ||
| os << "},\n"; |
There was a problem hiding this comment.
I approve of your choice to include a JSON output formatter of your own instead of dealing with cross-platform dependency pain to import someone else's. But I think it's likely to be reused for more than one thing, so I think a bit more effort in its API would be helpful. Definitely we should avoid explicitly specifying all those nesting depths, if nothing else. And the interleaving of function calls with literal snippets of JSON syntax makes the call site hard to read.
On the other hand, it's very easy to spend way too much effort overengineering a thing like this, and end up with some C++-heavy edifice in which destructors do half the work and the call sites are constrained to keep their variable scopes aligned with the output JSON structure, which isn't always what you most wanted. (Especially in this code base, which hasn't yet committed to C++17, so you can't work around it by having a std::optional which you empty when you're done with it.)
As a compromise in between, how about an API like this?
JsonWriter jw(os); // constructor takes an ostream& and keeps it
jw.obj_open(); // writes a { and increments jw.nesting_depth
jw.obj_field_number("height", 12345);
jw.obj_field_string("name", "Some string that will be escaped");
jw.obj_field_obj_open("subobject");
// now further fields live inside that subobject
jw.obj_field_bool("confused", true);
jw.obj_close();
// now we're back in the top-level object
jw.obj_field_array_open("subarray");
// now we're inside an array, so we mustn't call obj_field_anything
jw.array_entry_string("foo");
jw.array_entry_string("bar");
jw.array_entry_string("baz");
jw.array_close();
// now we're putting fields in the top-level object again
jw.obj_field_null("one_last_thing");
jw.obj_close();Then the JsonWriter would keep track of nesting depths and deal with the prettyprinting, and could enforce by assertion that subobjects and subarrays are all closed in the right order. Call sites like this are kept much terser, with generally one line per field instead of two or four, and far fewer opportunities to make errors in the output JSON syntax. An added bonus is that we could introduce a flag in JsonWriter which turned prettyprinting off completely, packing the data down to minimum size for HTTP transport, and that would be a single operation at construction time instead of affecting every single line of the JSON construction.
But at the same time, each method in the example above is relatively simple, and only has to write appropriate output to os, without needing to create complicated clusters of C++ subobjects with multiple methods, or have a hierarchy of subtypes, or anything that would take far longer to write than the effort it would save.
This should also live in its own header file in include/libtarmac and source file in lib (if the latter is even needed).
| "line": 1494, | ||
| "pc": "0x807c" | ||
| }, | ||
| "callee": { |
There was a problem hiding this comment.
I wonder if you looked at this file with your own eyes at any point 🙂 When I paged down just to scroll past it, these outlying { jumped right out at me. Somewhere in your writing code there's a json_indent call that shouldn't have been there. (Another reason we should leave that to the JsonWriter I suggest.)
| "function_entry": { | ||
| "time": 1, | ||
| "line": 157, | ||
| "pc": "0x8000" |
There was a problem hiding this comment.
I understand why you made pc a string field – it was so that it can also be written in terms of symbols from the ELF file. But it seems quite likely to me that some front-end code would find the pure numeric value of the address useful too. For example, for displaying a sequence of nearby things in address order. Perhaps we could have a number alongside the string, and set the string to null or maybe leave it out completely if it isn't adding anything beyond the number?
|
Thanks for the detailed review — I’ve pushed a follow-up update that addresses the JSON-focused feedback. Changes in this round:
I’ve kept the writer API deliberately small for now, but reusable across future JSON outputs. Happy to keep iterating if you’d like the schema or the writer surface adjusted further. |
|
I’ve pushed one more follow-up on the JSON schema and prototype viewer. The sample frontend was still formatting
The browser prototype now prefers |
|
Thanks again — I’ve now addressed the JSON-writer refactor, cleaned up the schema around addresses, updated the prototype viewer to match, refreshed the fixtures, and fixed the README consistency issue as well. Happy to keep iterating if you’d like any further adjustments to the writer API or the JSON shape. |
|
Any update on this @statham-arm ?? |
Summary
This PR adds a first web-facing slice for Tarmac Trace Utilities.
It introduces:
tarmac-calltree-json, a new tool that exports TTU call tree data as structured JSONCallTreeimplementationMotivation
TTU already provides strong trace parsing, indexing, symbol lookup, and call tree analysis in its native C++ core.
This change exposes part of that functionality in a machine-readable format so that browser-based and other external visualizations can be built on top of the existing TTU engine, without reimplementing Tarmac parsing outside the project.
The included static viewer is intended as a prototype demonstrating one possible web-facing workflow built on top of the JSON export.
What’s Included
Native export support
tarmac-calltree-jsonCallTreePrototype UI
web/Why JSON instead of frontend parsing
The goal here is to keep TTU’s existing C++ engine as the source of truth.
TTU already knows how to:
Exporting JSON from the existing engine gives a stable contract that a frontend can consume, while avoiding duplication of trace-analysis logic in JavaScript.
Example
Then serve the repository and open the prototype viewer:
Open:
Validation
Automated coverage added in this PR includes:
Manual smoke test for the prototype UI:
quicksort.calltree.jsonpython3 -m http.server 8123web/calltree-viewer.htmlquicksortScreenshots
1. Browser-based call tree workbench powered by TTU data
2. Function search across the exported call hierarchy
3. Deep call inspection with entry, exit, and call-path metadata
4. End-to-end flow from native TTU export to browser visualization
Notes
This PR is intentionally scoped as a foundational export + prototype workflow, not a complete web application. The main goal is to provide a machine-readable path from the existing TTU core to browser-based visualization.
Contributor note
This prototype was developed by an external contributor working through SolveFast Labs, with the goal of exploring a maintainable web-facing workflow built on top of TTU’s existing engine.