GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Trace key operations by amcasey · Pull Request #40063 · microsoft/TypeScript

Conversation

@amcasey

@amcasey Andrew Casey (amcasey) commented Aug 15, 2020

Copy link
Copy Markdown
Member

Produce output in Chrome's event tracing format so that it can be viewed in Chrome/Edge.

Hat tip to Sam Saccone (@samccone) for suggesting the format.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

noinkling, Bnaya, lencioni, Quramy, and captain-yossarian reacted with thumbs up emoji DanielRosenwasser, JakeGinnivan, and izhan reacted with hooray emoji DanielRosenwasser, JakeGinnivan, samccone, mshustov, izhan, jonaskello, Bnaya, Quramy, and tylersmalley reacted with heart emoji DanielRosenwasser, JakeGinnivan, kumar303, Bnaya, and kenotron reacted with rocket emoji
All reactions
  • 👍 5 reactions
  • 🎉 3 reactions
  • ❤️ 9 reactions
  • 🚀 5 reactions
@typescript-bot TypeScript Bot (typescript-bot) added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Aug 15, 2020
This was referenced Aug 15, 2020
Transformer times #39990
Closed
Add a switch for reporting expensive statements #37785
Closed
@amcasey

Andrew Casey (amcasey) commented Aug 15, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

There are some noteworthy rough edges:

  1. I haven't thought through how to separate/combine the traces for different projects in build mode, so presently I just insert a counter into each path to prevent clobbering.
  2. The new System APIs are ugly, but I think it's important to write as you go, so that you get something if you have to kill a very long build (e.g. customers reporting tens of minutes)
  3. Some types are still only readable by means of typeToTypeNode and I'd really like to eliminate that because of how slow it is (not urgent since it happens after profiling is complete)

Updates:

  1. Only use counters in build mode and print a separate legend file mapping them back to tsconfig paths.
  2. Reverted - Ryan Cavanaugh (@RyanCavanaugh) suggested using fs directly.
  3. Will likely wait for a future PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@amcasey

Andrew Casey (amcasey) commented Aug 15, 2020

Copy link
Copy Markdown
Member Author

CI failures are because I didn't update the harness System impl(s). That can wait until next week.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@amcasey

Andrew Casey (amcasey) commented Aug 17, 2020

Copy link
Copy Markdown
Member Author

There are deliberately few trace events in the checker to avoid (excessively) skewing the runtime. If people would like to see more, e.g. for debugging, my inclination would be to add a separate mode (i.e. profiling trace vs activity trace). Note that the persistence format is JSON, so it's very easy to create enormous trace files (e.g. adding inferFromTypes tripled the file size for one sample project).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request Aug 18, 2020
TypeScript 4.1 Iteration Plan #40124
Closed
@amcasey Andrew Casey (amcasey) closed this Aug 21, 2020
@amcasey Andrew Casey (amcasey) reopened this Aug 21, 2020
@amcasey Andrew Casey (amcasey) mentioned this pull request Aug 21, 2020
Checking is slow in project consuming xstate types #39826
Closed
@rbuckton

Ron Buckton (rbuckton) commented Aug 21, 2020

Copy link
Copy Markdown
Contributor

Have you looked at the sources for ts-perf? I made some headway doing the same thing with the existing performance.mark function and have code for generating Chrome timelines.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@amcasey

Andrew Casey (amcasey) commented Aug 21, 2020

Copy link
Copy Markdown
Member Author

Rough instructions for using it in its present state:

  1. Append --generateTrace {folderName} to your regular tsc command line
  2. Look in the resulting folder. If you used build mode, there will be a legend.json telling you what went where. Otherwise, there will be a trace.json file and a types.json files.
  3. Navigate to edge://tracing or chrome://tracing and Load trace.json
  4. Expand Process 1 with the little triangle in the left sidebar
  5. Click on different blocks to see their payloads in the bottom pane
  6. Open types.json in an editor
  7. When you see a type ID in the tracing output, go-to-line {id} to find data about that type
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Andrew Casey (amcasey) added 3 commits August 21, 2020 17:07
@amcasey
Trace key operations
5d60972
Produce output in Chrome's event tracing format so that it can be viewed
in Chrome/Edge.
@amcasey
Revert System changes and consume fs directly
a0479da
@amcasey
Trace structuredTypeRelatedTo, rather than recursiveTypeRelatedTo
00804d8
@amcasey
Andrew Casey (amcasey) force-pushed the ChromeTracing branch from b3861c8 to 00804d8 Compare August 22, 2020 00:10
Andrew Casey (amcasey) added 3 commits August 21, 2020 17:39
@amcasey
Record the recursion ID of each type, if available
7b40895
@amcasey
Add instantaneous events when depth limits are hit
36489ff
@amcasey
Correct bottoming-out tracing for recursiveTypeRelatedTo
ef1481c
@amcasey

Andrew Casey (amcasey) commented Sep 4, 2020

Copy link
Copy Markdown
Member Author

Having applied this to several repos now, I feel pretty confident that this is useful in its current state. Enriching it with additional tracepoints (e.g. for inference) or output (e.g. stack traces) can follow in future PRs.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@amcasey
Andrew Casey (amcasey) marked this pull request as ready for review September 4, 2020 20:29
@sandersn Nathan Shively-Sanders (sandersn) assigned Ron Buckton (rbuckton) Sep 8, 2020
@amcasey Andrew Casey (amcasey) mentioned this pull request Sep 9, 2020
Support additional trace points #40460
Open
7 tasks
@amcasey
Andrew Casey (amcasey) merged commit 45dedd6 into microsoft:master Sep 10, 2020
@amcasey
Andrew Casey (amcasey) deleted the ChromeTracing branch September 10, 2020 20:01
rbuckton
Ron Buckton (rbuckton) reviewed Sep 10, 2020
View reviewed changes
Comment thread src/compiler/tracing.ts
export function startTracing(configFilePath: string | undefined, traceDir: string, isBuildMode: boolean) {
Debug.assert(!traceFd, "Tracing already started");

if (fs === undefined) {

@rbuckton Ron Buckton (rbuckton) Sep 10, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

In the future we might be able to move some of this logic to a conditionally loaded module, similar to the extended debug information for flow nodes in ~/src/debug/dbg.ts loaded by ~/src/compiler/debug.ts:

TypeScript/src/compiler/debug.ts

Lines 655 to 668 in d896353

// attempt to load extended debugging information
try {
if (sys && sys.require) {
const basePath = getDirectoryPath(resolvePath(sys.getExecutingFilePath()));
const result = sys.require(basePath, "./compiler-debug") as RequireResult<ExtendedDebugModule>;
if (!result.error) {
result.module.init(ts);
extendedDebugModule = result.module;
}
}
}
catch {
// do nothing
}

I've been thinking about moving more of ts.Debug into the external file to reduce the file size of tsc.js, typescript.js, and tsserver.js as well.

This obviously isn't something we need to do now, but we might want to consider putting it on the backlog.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

amcasey reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@orta Orta Therox (orta) mentioned this pull request Sep 11, 2020
Adds support for the requires in TS 4.1b microsoft/monaco-typescript#69
Merged
@AnyhowStep AnyhowStep mentioned this pull request Oct 9, 2020
Debugging long check times with TypeScript? #24435
Closed
@nfour Sam J (nfour) mentioned this pull request Oct 11, 2020
Extremely high compilation time for production build on v5 TypeStrong/fork-ts-checker-webpack-plugin#453
Closed
@piotr-oles Piotr Oleś (piotr-oles) mentioned this pull request Oct 23, 2020
feat: add support for "generateTrace" tsconfig option TypeStrong/fork-ts-checker-webpack-plugin#523
Merged
@phryneas

Lenz Weber-Tronic (phryneas) commented Nov 2, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Andrew Casey (@amcasey) I'm having the problem that checkExpression calls within a checkSourceFile call often check nodes from completely different files, so just having kind, pos and end is not enough to identify the node in any way.

I am running this patch against 4.1.0-beta to also get the fileName - but I guess that is not really effective.
Could this be done somehow in a better way - and would a PR for this be welcome?

diff --git a/lib/tsc.js b/lib/tsc.js
index c78b8d698ea730be5a363592a25eb5eb9815753b..4333d58e45d2b2a55724ce7f7894c482223fc36c 100644
--- a/lib/tsc.js
+++ b/lib/tsc.js
@@ -59317,7 +59317,13 @@ var ts;
             }
         }
         function checkExpression(node, checkMode, forceTuple) {
-            ts.tracing.begin("check", "checkExpression", { kind: node.kind, pos: node.pos, end: node.end });
+            var parent = node.parent;
+            while (parent && parent.kind !== 300 /*SyntaxKind.SourceFile*/) {
+                parent = parent.parent
+            }
+            var sourceFile = parent;
+
+            ts.tracing.begin("check", "checkExpression", { kind: node.kind, pos: node.pos, end: node.end, sourceFile: sourceFile && sourceFile.fileName || "unknown" });
             var saveCurrentNode = currentNode;
             currentNode = node;
             instantiationCount = 0;
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@amcasey

Andrew Casey (amcasey) commented Nov 2, 2020

Copy link
Copy Markdown
Member Author

Lenz Weber-Tronic (@phryneas) Thanks for the patch! I had an early version that included similar code (there's actually a helper for walking up to the source file) but concluded that the extra tree walking was distorting the timing and the paths were bloating the log file excessively. Since readability wasn't a goal for v1, I left it out. When I've encountered the problem in practice, I've used code inspection on the frames that are in the right file plus types from later in the call stack to work out what was going on.

p.s. Sorry about the confusion this caused you - it's frustrating to waste time on things like that.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@rbuckton Ron Buckton (rbuckton) rbuckton left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@rbuckton Ron Buckton (rbuckton)

Labels

For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

PR Backlog
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@amcasey @rbuckton @phryneas @typescript-bot
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.