You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[nightly][performance] 97% of time spent reading type dependencies when --noCheck and --isolatedDeclarations is enabledΒ #58863
Ran the above code using tsc --generateTrace trace_dir which produced the following trace. trace.json
If you load this trace into https://speedscope.app you can see that out of 271ms, about 97% of the time is spent parsing and binding @types/lodash and lib.d.ts and only 6ms is spent on the actual emit.
Well, it's noCheck, not noBind or noParse; unless using one of the transpile functions (which never even hits the FS), all of the same up-front work is going to be done to build the Program before anything else is done. All noCheck did was turn off checking.
Maybe noCheck could effectively disable any and all program construction besides what's listed explicitly in includes/files (so only actually emittable files will be parsed and bound), but I don't know how that would affect buildinfo's contents.
That and it won't do the right thing in the editor; parsing/binding is not lazy so if you do set noCheck in tsconfig, boom... noCheck only disables the collection of diagnostics, it doesn't make the checker "broken" for other uses.
I don't think I even have a mental model around how noCheck ought to behave in an editor context, but regardless of that I don't see why not parsing @types/lodash would break the editor.
noCheck means "don't ask the checker to walk the file and collect diagnostics", but enabling it notably doesn't break the editor for completion, hovers, nor the emitter for type-using emit (i.e. code that does not pass isolatedModules or isolatedDeclarations). Notably, you can have files locally that depend packages in node_modules for emit, and they need to be able to emit as expected too.
Doing better here would of course be nice, just needs some thinking! For code that passes isolatedModules and isolatedDeclarations, skipping a full project load should be safe?
We already have option noResolve to stop adding dependencies so that option can be used to speed up i think . I dont think it should be implicit though
Nice! Having --noResolve seems to remove @types/lodash-related spans from the trace completely. I tried --noLib to see if it'd remove lib.d.ts, but it actually results in some errors
error TS2318: Cannot find global type 'Array'.
error TS2318: Cannot find global type 'Boolean'.
error TS2318: Cannot find global type 'Function'.
error TS2318: Cannot find global type 'IArguments'.
error TS2318: Cannot find global type 'Number'.
error TS2318: Cannot find global type 'Object'.
error TS2318: Cannot find global type 'RegExp'.
error TS2318: Cannot find global type 'String'.
Found 8 errors.
Yeah, you can't really avoid loading lib; transpileModule/transpileDeclaration simulate those basic things just to make things work for that reason, but that doesn't apply to regular runs.
Hm, maybe we should actually be pulling that mini-lib.d.ts into the core compiler and using that when noLib is set? There are types (those above) that are needed for things to work at all, so maybe that would actually be a positive thing...
Seems like I can add both noLib: true and noResolve: true to our script that uses ts.transpileDeclaration and it somehow doesn't produce any diagonostic errors. Just kicked off a build to see what kind of improvement that leads to! π€
Interesting, I just saw our build time go from ~5m49s to ~5m33s with the updated config. I'm not complaining! Note that this is with a parallel builder, so the upper bound for the improvement is more like 16s * # of cores.
Hm, maybe we should actually be pulling that mini-lib.d.ts into the core compiler and using that when noLib is set? There are types (those above) that are needed for things to work at all, so maybe that would actually be a positive thing...
The mini-lib is only sufficient for isolatedDeclarations - for normal (declaration, emitDecoratorMetadata) emit you definitely need full type information accessible (albeit lazily accessed).
I feel like you can probably turn on noResolve if you're turning on noCheck (and don't have declaration or emitDecoratorMetadata set) yourself if you're doing emit-only runs and get all the benefits you're looking for (in terms of skipping binding and parsing of files you're not emitting). We can't really enable it automatically, since we don't really know if your input file list is complete without resolution or not in advance.
After enabling noResolve and noLib and generating another trace on one of our projects that has a longer emit time, I see that most of the time is now spent parsing the tsconfig files of project references. This again seems like something that's not strictly needed if our only goal is to emit declaration files. I don't think there's an existing flag this time that can be used to avoid those? In any case, I think I can work around it by overriding "references" to be an empty array when only emitting declarations.
I'm able to reliably get a compiler crash if I run tsc with --noCheck and then run again with --noCheck --noResolve --noLib. The crash doesn't happen if I clear tsbuildinfo between runs. Unfortunately this doesn't repro with the trivial example I put in the issue description. cc Sheetal Nandi (@sheetalkamat)
~/TypeScript/lib/tsc.js:120150
return some(options.lib, (libFileName) => equalityComparer(file.fileName, resolvedLibReferences.get(libFileName).actual));
^
TypeError: Cannot read properties of undefined (reading 'get')
at ~/TypeScript/lib/tsc.js:120150:103
at some (~/TypeScript/lib/tsc.js:365:13)
at Object.isSourceFileDefaultLibrary (~/TypeScript/lib/tsc.js:120150:14)
at addSourceFile (~/TypeScript/lib/tsc.js:122743:31)
at Object.getAllFilesExcludingDefaultLibraryFile (~/TypeScript/lib/tsc.js:122737:9)
at createBuilderProgramState (~/TypeScript/lib/tsc.js:122901:18)
at createBuilderProgram (~/TypeScript/lib/tsc.js:123684:17)
at createEmitAndSemanticDiagnosticsBuilderProgram (~/TypeScript/lib/tsc.js:124196:10)
at createIncrementalProgram (~/TypeScript/lib/tsc.js:126010:10)
at performIncrementalCompilation (~/TypeScript/lib/tsc.js:125960:26)
Activity
jakebailey commentedon Jun 14, 2024
Well, it's
noCheck, notnoBindornoParse; unless using one of the transpile functions (which never even hits the FS), all of the same up-front work is going to be done to build the Program before anything else is done. AllnoCheckdid was turn off checking.Maybe
noCheckcould effectively disable any and all program construction besides what's listed explicitly inincludes/files(so only actually emittable files will be parsed and bound), but I don't know how that would affectbuildinfo's contents.That and it won't do the right thing in the editor; parsing/binding is not lazy so if you do set
noCheckin tsconfig, boom...noCheckonly disables the collection of diagnostics, it doesn't make the checker "broken" for other uses.MichaelMitchell-at commentedon Jun 14, 2024
I don't think I even have a mental model around how
noCheckought to behave in an editor context, but regardless of that I don't see why not parsing@types/lodashwould break the editor.jakebailey commentedon Jun 14, 2024
noCheckmeans "don't ask the checker to walk the file and collect diagnostics", but enabling it notably doesn't break the editor for completion, hovers, nor the emitter for type-using emit (i.e. code that does not pass isolatedModules or isolatedDeclarations). Notably, you can have files locally that depend packages innode_modulesfor emit, and they need to be able to emit as expected too.Doing better here would of course be nice, just needs some thinking! For code that passes
isolatedModulesandisolatedDeclarations, skipping a full project load should be safe?sheetalkamat commentedon Jun 14, 2024
We already have option
noResolveto stop adding dependencies so that option can be used to speed up i think . I dont think it should be implicit thoughMichaelMitchell-at commentedon Jun 14, 2024
Nice! Having
--noResolveseems to remove@types/lodash-related spans from the trace completely. I tried--noLibto see if it'd removelib.d.ts, but it actually results in some errorsjakebailey commentedon Jun 14, 2024
Yeah, you can't really avoid loading lib; transpileModule/transpileDeclaration simulate those basic things just to make things work for that reason, but that doesn't apply to regular runs.
jakebailey commentedon Jun 14, 2024
Hm, maybe we should actually be pulling that mini-lib.d.ts into the core compiler and using that when noLib is set? There are types (those above) that are needed for things to work at all, so maybe that would actually be a positive thing...
MichaelMitchell-at commentedon Jun 14, 2024
Seems like I can add both
noLib: trueandnoResolve: trueto our script that usests.transpileDeclarationand it somehow doesn't produce any diagonostic errors. Just kicked off a build to see what kind of improvement that leads to! π€jakebailey commentedon Jun 14, 2024
IIRC those are already set by
transpileDeclaration(among other flags), so that shouldn't change the behavior in the API at least.MichaelMitchell-at commentedon Jun 14, 2024
Interesting, I just saw our build time go from ~5m49s to ~5m33s with the updated config. I'm not complaining! Note that this is with a parallel builder, so the upper bound for the improvement is more like 16s * # of cores.
weswigham commentedon Jun 14, 2024
The mini-lib is only sufficient for
isolatedDeclarations- for normal (declaration,emitDecoratorMetadata) emit you definitely need full type information accessible (albeit lazily accessed).I feel like you can probably turn on
noResolveif you're turning onnoCheck(and don't havedeclarationoremitDecoratorMetadataset) yourself if you're doing emit-only runs and get all the benefits you're looking for (in terms of skipping binding and parsing of files you're not emitting). We can't really enable it automatically, since we don't really know if your input file list is complete without resolution or not in advance.MichaelMitchell-at commentedon Jun 14, 2024
Some more interesting findings:
noResolveandnoLiband generating another trace on one of our projects that has a longer emit time, I see that most of the time is now spent parsing the tsconfig files of project references. This again seems like something that's not strictly needed if our only goal is to emit declaration files. I don't think there's an existing flag this time that can be used to avoid those? In any case, I think I can work around it by overriding"references"to be an empty array when only emitting declarations.--noCheckand then run again with--noCheck --noResolve --noLib. The crash doesn't happen if I clear tsbuildinfo between runs. Unfortunately this doesn't repro with the trivial example I put in the issue description. cc Sheetal Nandi (@sheetalkamat)sheetalkamat commentedon Jun 14, 2024
michaelm (@MichaelMitchell-at) may be #58867
MichaelMitchell-at commentedon Jun 14, 2024
Closing this out since it's established that things are generally working as intended. Some productive discussion and ideas came out of this though!