-
Notifications
You must be signed in to change notification settings - Fork 4
Adds --json and other params #6
base: main
Are you sure you want to change the base?
Changes from all commits
79800da
28ff6e2
0a85c53
f427eb8
d2a3a94
332e89d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,4 +2,5 @@ | |
!*.d.ts | ||
*.map | ||
tsconfig.json | ||
*.tsbuildinfo | ||
*.tsbuildinfo | ||
vendor |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
{ | ||
"files.exclude": { | ||
"**/.git": true, | ||
"**/.svn": true, | ||
"**/.hg": true, | ||
"**/CVS": true, | ||
"**/.DS_Store": true, | ||
"**/*.js": true, | ||
"**/*.d.ts": true, | ||
"**/*.js.map": true | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,37 @@ | ||
if (process.argv.length !== 3 && process.argv.length != 4) { | ||
// node ./analyze-trace.js vendor/mui/trace.json vendor/mui/types.json --json output.json | ||
|
||
type Opts = { | ||
json?: string, | ||
thresholdDuration?: string | ||
minDuration?: string | ||
minPercentage?: string | ||
} | ||
|
||
const args: string[] = [] | ||
const opts: Opts = {} | ||
|
||
let foundOpt: string | undefined = undefined | ||
process.argv.forEach((arg, i) => { | ||
if (foundOpt) { | ||
opts[foundOpt] = arg | ||
foundOpt = undefined | ||
return | ||
} | ||
|
||
if (arg.startsWith("--")) { | ||
foundOpt = arg.replace("--", "") | ||
} else { | ||
args.push(arg) | ||
} | ||
}); | ||
|
||
if (args.length !== 3 && args.length != 4) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect you'll need to update these length checks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yargs'll fix that |
||
const path = require("path"); | ||
console.error(`Usage: ${path.basename(process.argv[0])} ${path.basename(process.argv[1])} trace_path [type_path]`); | ||
console.error(`Options: --json [path] Prints a JSON object of the results to stdout`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it print to stdout or |
||
console.error(` --thresholdDuration [default: 50000] How many ms should a span with children use for highlighting`); | ||
console.error(` --minDuration [default: 10000] How long should a single span take before being classed as interesting`); | ||
console.error(` --minPercentage [default: 0.6] The threshold for being interesting based on % of call stack`); | ||
process.exit(1); | ||
} | ||
|
||
|
@@ -28,9 +59,9 @@ if (typesPath && !fs.existsSync(typesPath)) { | |
process.exit(3); | ||
} | ||
|
||
const thresholdDuration = 5E5; // microseconds | ||
const minDuration = 1E5; // microseconds | ||
const minPercentage = 0.6; | ||
const thresholdDuration = Number(opts.thresholdDuration) || 5E5; // microseconds | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ignorant question: is this different from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, basically the exact same, I just find Number to feel more explicit |
||
const minDuration = Number(opts.minDuration) || 1E5; // microseconds | ||
const minPercentage = Number(opts.minPercentage) || 0.6; | ||
|
||
main().catch(err => console.error(`Internal Error: ${err.message}\n${err.stack}`)); | ||
|
||
|
@@ -123,9 +154,9 @@ function parse(tracePath: string): Promise<ParseResult> { | |
}); | ||
} | ||
|
||
|
||
async function main(): Promise<void> { | ||
const { minTime, maxTime, spans, unclosedStack } = await parse(tracePath); | ||
|
||
if (unclosedStack.length) { | ||
console.log("Trace ended unexpectedly"); | ||
|
||
|
@@ -162,18 +193,41 @@ async function main(): Promise<void> { | |
} | ||
} | ||
|
||
await printHotStacks(root); | ||
await makeHotStacks(root); | ||
} | ||
|
||
type TreeNode = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, maybe that doesn't work with recursive types? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. interface will be fine, they can recurse too |
||
type: string | ||
time?: string | ||
message: string | ||
terseMessage: string | ||
start?: { | ||
file: string | ||
offset?: number | ||
}, | ||
end?: { | ||
file: string | ||
offset?: number | ||
} | ||
children: TreeNode[] | ||
} | ||
|
||
async function printHotStacks(root: EventSpan): Promise<void> { | ||
async function makeHotStacks(root: EventSpan): Promise<void> { | ||
if (typesPath) { | ||
await addTypeTrees(root); | ||
} | ||
|
||
const positionMap = await getNormalizedPositions(root); | ||
|
||
const tree = await makePrintableTree(root, /*currentFile*/ undefined, positionMap); | ||
if (Object.entries(tree).length) { | ||
|
||
if (tree && Object.entries(tree).length) { | ||
if (opts.json) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intuitively, I would have expected json output to replace the normal output, not supplement it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're not wrong to feel that, but I think there's still value in keeping the logs (for example, we can show the terminal output in a build log) |
||
fs.writeFileSync(opts.json, JSON.stringify(tree, null, " ")) | ||
} | ||
console.log("Hot Spots"); | ||
console.log(treeify.asTree(tree, /*showValues*/ false, /*hideFunctions*/ true)); | ||
const consoleTree = treeNodeToTreeifyTree(tree!) | ||
console.log(treeify.asTree(consoleTree, /*showValues*/ false, /*hideFunctions*/ true)); | ||
} | ||
else { | ||
console.log("No hot spots found") | ||
|
@@ -296,38 +350,46 @@ async function getTypes(): Promise<readonly any[]> { | |
return typesCache!; | ||
} | ||
|
||
async function makePrintableTree(curr: EventSpan, currentFile: string | undefined, positionMap: PositionMap): Promise<{}> { | ||
// Sort slow to fast | ||
let childTree = {}; | ||
|
||
async function makePrintableTree(curr: EventSpan, currentFile: string | undefined, positionMap: PositionMap): Promise<TreeNode | undefined> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Printable" was my word for "treeify-compatible", so "makeTree" is probably fine now. |
||
if (curr.event?.name === "checkSourceFile") { | ||
currentFile = curr.event.args!.path; | ||
} | ||
|
||
if (curr.children.length) { | ||
const sortedChildren = curr.children.sort((a, b) => (b.end - b.start) - (a.end - a.start)); | ||
for (const child of sortedChildren) { | ||
Object.assign(childTree, await makePrintableTree(child, currentFile, positionMap)); | ||
const node = eventToTreeNode(); | ||
if (node) { | ||
node.time = `${Math.round((curr.end - curr.start) / 1000)}ms` | ||
|
||
if (curr.children.length) { | ||
const sortedChildren = curr.children.sort((a, b) => (b.end - b.start) - (a.end - a.start)); | ||
const nodes: TreeNode[] = [] | ||
|
||
for (const child of sortedChildren) { | ||
const tree = await makePrintableTree(child, currentFile, positionMap) | ||
if (tree) nodes.push(tree) | ||
} | ||
|
||
node.children = nodes | ||
} | ||
} | ||
|
||
if (curr.typeTree) { | ||
Object.assign(childTree, updateTypeTreePositions(curr.typeTree)); | ||
if (curr.typeTree && node) { | ||
updateTypeTreePositions(node, curr.typeTree); | ||
} | ||
|
||
if (curr.event) { | ||
const eventStr = eventToString(); | ||
if (eventStr) { | ||
let result = {}; | ||
result[`${eventStr} (${Math.round((curr.end - curr.start) / 1000)}ms)`] = childTree; | ||
return result; | ||
} | ||
} | ||
return node; | ||
|
||
return childTree; | ||
function eventToTreeNode(): TreeNode | undefined { | ||
const treeNode: TreeNode = { | ||
message: "", | ||
terseMessage: "Hot Spots", | ||
type: "hot-spots", | ||
children: [] | ||
} | ||
if (!curr.event) return treeNode | ||
|
||
function eventToString(): string | undefined { | ||
const event = curr.event!; | ||
const event = curr.event; | ||
treeNode.type = event.name | ||
|
||
switch (event.name) { | ||
// TODO (https://github.com/amcasey/ts-analyze-trace/issues/2) | ||
// case "findSourceFile": | ||
|
@@ -336,28 +398,54 @@ async function makePrintableTree(curr: EventSpan, currentFile: string | undefine | |
// case "emit": | ||
// return `Emit`; | ||
case "checkSourceFile": | ||
return `Check file ${formatPath(currentFile!)}`; | ||
treeNode.message = `Check file ${formatPath(currentFile!)}` | ||
treeNode.terseMessage = `Check file ${path.basename(currentFile!)}` | ||
treeNode.start = { | ||
file: currentFile! | ||
} | ||
|
||
return treeNode | ||
|
||
case "structuredTypeRelatedTo": | ||
const args = event.args!; | ||
return `Compare types ${args.sourceId} and ${args.targetId}`; | ||
treeNode.message = `Compare types ${args.sourceId} and ${args.targetId}`; | ||
treeNode.terseMessage = `Compare types ${args.sourceId} and ${args.targetId}`; | ||
// TODO: Add start and end links | ||
return | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
case "getVariancesWorker": | ||
return `Determine variance of type ${event.args!.id}`; | ||
treeNode.message = `Compute variance of type ${event.args!.id}`; | ||
treeNode.terseMessage = `Compute variance of type ${event.args!.id}`; | ||
return | ||
|
||
default: | ||
if (event.cat === "check" && event.args && event.args.pos && event.args.end) { | ||
if (positionMap.has(currentFile!)) { | ||
const updatedPos = positionMap.get(currentFile!)!.get(event.args.pos.toString())!; | ||
const updatedEnd = positionMap.get(currentFile!)!.get(event.args.end.toString())!; | ||
return `${unmangleCamelCase(event.name)} from (line ${updatedPos[0]}, char ${updatedPos[1]}) to (line ${updatedEnd[0]}, char ${updatedEnd[1]})`; | ||
treeNode.message = `${unmangleCamelCase(event.name)} from (line ${updatedPos[0]}, char ${updatedPos[1]}) to (line ${updatedEnd[0]}, char ${updatedEnd[1]})`; | ||
treeNode.terseMessage = unmangleCamelCase(event.name); | ||
treeNode.start = { | ||
file: currentFile!, | ||
offset: event.args.pos, | ||
} | ||
treeNode.end = { | ||
file: currentFile!, | ||
offset: event.args.end, | ||
} | ||
return treeNode; | ||
} | ||
else { | ||
return `${unmangleCamelCase(event.name)} from offset ${event.args.pos} to offset ${event.args.end}`; | ||
treeNode.message = `${unmangleCamelCase(event.name)} from offset ${event.args.pos} to offset ${event.args.end}` | ||
treeNode.terseMessage = unmangleCamelCase(event.name); | ||
return treeNode; | ||
} | ||
} | ||
return undefined; | ||
} | ||
} | ||
|
||
function updateTypeTreePositions(typeTree: any): any { | ||
function updateTypeTreePositions(node: TreeNode, typeTree: any): any { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Passing in a root node so it can recurse, maybe I should rename it to say it will recurse |
||
if (!typeTree) return; | ||
|
||
let newTree = {}; | ||
|
@@ -377,7 +465,7 @@ async function makePrintableTree(curr: EventSpan, currentFile: string | undefine | |
typeString = typeString.replace(path, formatPath(path)); | ||
} | ||
|
||
newTree[typeString] = updateTypeTreePositions(subtree); | ||
newTree[typeString] = updateTypeTreePositions(node, subtree); | ||
} | ||
|
||
return newTree; | ||
|
@@ -415,4 +503,21 @@ function unmangleCamelCase(name: string) { | |
|
||
function getLineCharMapKey(line: number, char: number) { | ||
return `${line},${char}`; | ||
} | ||
} | ||
|
||
function treeNodeToTreeifyTree(node: TreeNode) { | ||
const obj = {} | ||
const toKey = (node: TreeNode) => `${node.message} (${node.time})` | ||
|
||
let value: any | null = null | ||
if (node.children){ | ||
let newValue = {} | ||
node.children.forEach(c => { | ||
newValue[toKey(c)] = treeNodeToTreeifyTree(c) | ||
}); | ||
value = newValue | ||
} | ||
obj[toKey(node)] = value | ||
|
||
return obj | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,35 @@ | ||
if (process.argv.length !== 3) { | ||
const args: string[] = [] | ||
const opts: string[] = [] | ||
|
||
let foundOpt: string | undefined = undefined | ||
process.argv.forEach((arg, i) => { | ||
if (foundOpt) { | ||
opts.push(foundOpt, arg) | ||
foundOpt = undefined | ||
return | ||
} | ||
|
||
if (arg.startsWith("--")) { | ||
foundOpt = arg | ||
} else { | ||
args.push(arg) | ||
} | ||
}); | ||
|
||
if (args.length !== 3) { | ||
const path = require("path"); | ||
console.error(`Usage: ${path.basename(process.argv[0])} ${path.basename(process.argv[1])} trace_dir`); | ||
console.error(`Options: --json [path] Prints a JSON object of the results to stdout`); | ||
console.error(` --thresholdDuration [default: 50000] How many ms should a span with children use for highlighting`); | ||
console.error(` --minDuration [default: 10000] How long should a single span take before being classed as interesting`); | ||
console.error(` --minPercentage [default: 0.6] The threshold for being interesting based on % of call stack`); | ||
process.exit(1); | ||
} | ||
|
||
import cp = require("child_process"); | ||
import fs = require("fs"); | ||
import os = require("os"); | ||
import crypto = require("crypto"); | ||
import path = require("path"); | ||
|
||
import plimit = require("p-limit"); | ||
|
@@ -34,6 +57,7 @@ interface Project { | |
|
||
interface ProjectResult { | ||
project: Project; | ||
jsonPath: string | undefined | ||
stdout: string; | ||
stderr: string; | ||
exitCode: number | undefined; | ||
|
@@ -82,6 +106,19 @@ async function main(): Promise<boolean> { | |
async function analyzeProjects(projects: readonly Project[]): Promise<boolean> { | ||
const results = await Promise.all(projects.map(p => limit(analyzeProject, p))); | ||
|
||
if (opts.includes("--json")) { | ||
const writePath = opts[opts.indexOf("--json") + 1] | ||
const allJSONs = results.map(p => { | ||
if(!p.jsonPath || !fs.existsSync(p.jsonPath)) return | ||
return { | ||
trace: p.project.tracePath, | ||
configFilePath: p.project.configFilePath, | ||
repo: JSON.parse(fs.readFileSync(p.jsonPath!, "utf8")) | ||
} | ||
}).filter(Boolean) | ||
fs.writeFileSync(writePath, JSON.stringify(allJSONs, null, " ")) | ||
} | ||
|
||
const hadHotSpots: (ProjectResult & { score: number })[] = []; | ||
const hadErrors: ProjectResult[] = []; | ||
for (const result of results) { | ||
|
@@ -152,8 +189,17 @@ async function analyzeProject(project: Project): Promise<ProjectResult> { | |
args.push(project.typesPath); | ||
} | ||
|
||
// If it's going to include a JSON path, make it per-trace | ||
let jsonPath: string | undefined = undefined | ||
if (opts.includes("--json")) { | ||
const hash = crypto.createHash('sha256').update(project.tracePath).digest("hex"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we hashing files? I don't think there's a way to update one trace out of a family - are we guarding against having to redo work on crash? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Basically you ask the project "give me a JSON file at this path" - but to generate that each sub-project in the project will generate its own JSON file and these need unique names, so we take the path to the project and turn it into a SHA so they don't collide |
||
jsonPath = path.join(os.tmpdir(), hash + ".json") | ||
} | ||
|
||
return new Promise<ProjectResult>(resolve => { | ||
const child = cp.fork(path.join(__dirname, "analyze-trace"), args, { stdio: "pipe", env: { FORCE_COLOR: '1' } }); | ||
const cmd = path.join(__dirname, "analyze-trace") | ||
const childArgs = args.concat(opts) | ||
const child = cp.fork(cmd, childArgs, { stdio: "pipe", env: { FORCE_COLOR: '1' } }); | ||
|
||
let stdout = ""; | ||
let stderr = ""; | ||
|
@@ -164,6 +210,7 @@ async function analyzeProject(project: Project): Promise<ProjectResult> { | |
child.on("exit", (code, signal) => { | ||
resolve({ | ||
project, | ||
jsonPath, | ||
stdout: stdout.trim(), | ||
stderr: stderr.trim(), | ||
exitCode: code ?? undefined, | ||
|
@@ -175,4 +222,4 @@ async function analyzeProject(project: Project): Promise<ProjectResult> { | |
|
||
function isFile(path: string): Promise<boolean> { | ||
return fs.promises.stat(path).then(stats => stats.isFile()).catch(_ => false); | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to just pull in yargs. It's already there indirectly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to come back, doh - OK, I'm good for this