Performance regression with project build in da663656 ...

[robertknight]

Pressing F6 to build my project went from about 5 seconds to several minutes.

Adding logging to the build() function in projectService.ts shows that calls to emitFile() are taking ~1500-3000ms per file and my project has ~170 TS source files.

Via git bisect I've narrowed it down to either c797b2d or da66365 (the former is a fix needed to actually build the project, so I can't test between these two without source changes).

commit c797b2de7e693fa65f6bea241c6e3df013d06fd8
Author: Basarat Syed <[email protected]>
Date:   Thu Sep 3 10:15:10 2015 +1000

    fix(typescript) latest version of typescript expects `package.json` to be passed in in the initial file set

commit da66365677b489c73cdfb716c83f6933fcba3b97
Author: Basarat Syed <[email protected]>
Date:   Thu Sep 3 09:08:59 2015 +1000

    chore(ntypescript) update to latest

[robertknight]

Running atom-typescript/node_modules/ntypescript/bin/tsc directly takes ~3.5s and I get similar results with the current stable version of TypeScript, so the issue appears relate to the way the compiler is being invoked rather than an issue with the tool itself.

[basarat]

I'll have a look 🌹

Notes to self:

• project needs tsd reinstall

[basarat]

A normal call to services.getEmitOutput(filePath); takes under a 50ms as shown below:

HILD ERR STDERR: C:/REPOS/atom-typescript/lib/main/lang/fixmyts/quickFixRegistry.ts

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: Duration: 11ms

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: C:/REPOS/atom-typescript/lib/main/lang/fixmyts/quickFixes/addClassMember.ts

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: Duration: 23ms

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: C:/REPOS/atom-typescript/lib/main/lang/fixmyts/quickFixes/addClassMethod.ts

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: Duration: 26ms

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: C:/REPOS/atom-typescript/lib/main/lang/fixmyts/quickFixes/addImportStatement.ts

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: Duration: 16ms

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: C:/REPOS/atom-typescript/lib/main/lang/fixmyts/quickFixes/equalsToEquals.ts

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: Duration: 9ms

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: C:/REPOS/atom-typescript/lib/main/lang/fixmyts/quickFixes/extractVariable.ts

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: Duration: 23ms

But in your code it is taking 1500ms! :

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: c:/REPOS/passcards/addons/firefox/src/main.ts: 1657ms

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: c:/REPOS/passcards/addons/firefox/src/panel_content.ts: 1474ms

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: c:/REPOS/passcards/cli/agent.ts: 1411ms

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: c:/REPOS/passcards/cli/agent_server.ts: 1398ms

C:\REPOS\atom-typescript\dist\worker\lib\workerLib.js:175 CHILD ERR STDERR: c:/REPOS/passcards/cli/agent_test.ts: 1797ms

How to test

Wrap this line as follows:

let start = new Date();
var output = services.getEmitOutput(filePath);
console.error(`${filePath}: ${(new Date()).getTime() - start.getTime()}ms`)

Recommend raising this with Microsoft/TypeScript.

[basarat]

Has to do with compiler options. The following definitely speeds is back to 3.5s mark:

    "compilerOptions": {
        "target": "es5",
        "module": "commonjs",
        "declaration": false,
        "noImplicitAny": false,
        "removeComments": true,
        "noLib": false,
        "preserveConstEnums": true,
        "outDir": "../dist",
        "sourceMap": false,
        "jsx": "react"
    },

[basarat]

The culprit is "noEmitOnError": true,. Please raise with Microsoft/TypeScript or change the tsconfig to allow emit on error 🌹

[robertknight]

Will do. Thanks @basarat !

[basarat]

From the linked issue microsoft/TypeScript#4894 (comment), this is by design 🌹

[robertknight]

Thanks for investigating. The explanation makes sense, although it should be possible to preserve the purpose of 'noEmitOnError', making sure that the JS on disk only reflects the last-successfully-typechecked state of the project, in an incremental way. That's something to be discussed upstream I think.

A short term measure in atom-typescript would be to abort the build with an error if noEmitOnError is set, rather than building the project very very slowly.

[basarat]

I've created a separate issue to track your ideas : #603