Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SR-11730] [llbuild] Improve GraphViz as export format option to llbuild-analyze #780

Closed
BenchR267 opened this issue Nov 7, 2019 · 9 comments
Closed

[SR-11730] [llbuild] Improve GraphViz as export format option to llbuild-analyze #780

BenchR267 opened this issue Nov 7, 2019 · 9 comments
Labels
llbuild New Feature StarterBug

Comments

@BenchR267
Copy link
Member

Previous ID SR-11730
Radar rdar://problem/56980565
Original Reporter @BenchR267
Type New Feature
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 1
Component/s llbuild
Labels New Feature, StarterBug
Assignee thomas (JIRA)
Priority Medium

md5: 19522583df59e2b8b5eca93f059ec10d

Issue Description:

llbuild-analyze is a local package in llbuild which offers a command line interface for calculating the critical path given a build database. The critical path of a build defines the chain of dependent tasks that has the longest duration when adding up the individual durations of those tasks. The tool offers to export the critical path as a JSON file which can then be used for further investigation.
Adding more serialization options would result in more actionable workflows. This issue tracks adding GraphViz as an export schema option which would allow users to open the created critical path file and view it in tools supporting GraphViz files.
The format is well defined and all the necessary information should be available in the CriticalBuildPath object that gets returned by the algorithm - it contains all tasks that were part of the build (including their timing information) and the dependency chain that forms the critical path.
The tool lives in products/llbuild-analyze/Sources/CriticalPathTool.swift and offers the basic infrastructure of supporting multiple export formats. Those consist of json and GraphViz at the moment while GraphViz is very rudimentary.
The current implementation exports all tasks which is a huge output even for small projects. A more actionable approach would export just the elements in the critical path with an option to add direct dependencies for a better overview.
Optionally there could be a command line option to specify how many transitive dependencies should be part of the output.
@swift-ci
Copy link

swift-ci commented Nov 8, 2019

Comment by Thomas Bartelmess (JIRA)

@BenchR267 if you don't mind, I am grabbing this one.

@BenchR267
Copy link
Member Author

thomas (JIRA User) Awesome, thank you! Let me know if you have any questions.

@BenchR267
Copy link
Member Author

thomas (JIRA User) do you need any assistance for this task?

@swift-ci
Copy link

Comment by Thomas Bartelmess (JIRA)

@BenchR267 Sorry for the radio-silence. I was working on this but I ran into issues with the schema version of the sqlite file.

The current version of llbuild when I build it from the repo doesn't generate the a database with the schema version of code to read the database expects.

Is there something I am missing? What schema version of is the HEAD version of llbuild supposed to generate?

@BenchR267
Copy link
Member Author

thomas (JIRA User) don't worry. How did you generate the build database and how did you try to read it in? I just tried the following which succeeded for me:

  • generate a build database with the `testCouldNotOpenDatabaseErrors` test in llbuild.xcodeproj

  • copy that build database to another directory (Desktop in my case)

  • start the llbuild-analyze tool with the arguments `critical-path ~/Desktop/build.db`

That uses the client schema version of 9 by default. Using another version needs the `--clientSchemaVersion $VERSION` parameter but that depends on the client schema version that got specified when creating the database. The above mentioned test uses 9, yours might used another one?

@swift-ci
Copy link

Comment by Thomas Bartelmess (JIRA)

@BenchR267 Here is my initial run.

  • I've added a protocol for nodes in the graph (GraphVizNode) to provide names used in the GraphViz graph.

  • I've added struct to represent GraphViz edges (DirectedEdge)

  • The graph is now first build in a in-memory representation as Set<DirectedEdge> and can be filtered to include all vertices or just the critical path.

  • I've added a CommandLineArgumentChoices protocol to improve the readability of --help.

Could you have a look what is currently there and if that is what you had in mind?
master...tbartelmess:improved-graphviz-output.

If this is ok as an I would like to add some tests for this and open a PR.

I would also like an add more options on what is included in the graph, as well as an option to include timing information?
Should I open a separate issue for that?

@BenchR267
Copy link
Member Author

thomas (JIRA User) That looks great! I haven't tried it out but the code contains everything I thought about.
I welcome more additions very much but please create more issue(s) for that. Thanks!

@swift-ci
Copy link

swift-ci commented Jan 2, 2020

Comment by Thomas Bartelmess (JIRA)

PR is here: #621

@BenchR267
Copy link
Member Author

Awesome, thank you thomas (JIRA User)! Merged in 96023dd

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
llbuild New Feature StarterBug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@BenchR267 @swift-ci