quisquous / cactbot

FFXIV TypeScript Raiding Overlay
Apache License 2.0
792 stars 379 forks source link

`tsc` execution speed improvements #4508

Open valarnin opened 2 years ago

valarnin commented 2 years ago

Summary

There are a number of code paths that slow tsc execution.

Steps taken to check:

  1. npm i -g @typescript/analyze-trace
  2. npm run tsc-no-emit -- --generateTrace ../cactbot-tracing
  3. npx analyze-trace ../cactbot-tracing --force-millis 300

Results:

Hot Spots
├─ Check file /mnt/c/Users/valarnin/Source/valarnin-cactbot/ui/raidboss/popup-text.ts (2467ms)
│  └─ Check expression from (line 697, char 11) to (line 697, char 39) (1918ms)
│     └─ Check expression from (line 697, char 31) to (line 697, char 38) (1915ms)
├─ Check file /mnt/c/Users/valarnin/Source/valarnin-cactbot/ui/raidboss/data/05-shb/eureka/delubrum_reginae_savage.ts (721ms)
│  └─ Check variable declaration from (line 95, char 7) to (line 3679, char 2) (633ms)
│     └─ Check expression from (line 95, char 38) to (line 3679, char 2) (509ms)
│        └─ Check expression from (line 174, char 13) to (line 2567, char 4) (326ms)
├─ Check file /mnt/c/Users/valarnin/Source/valarnin-cactbot/ui/raidboss/data/05-shb/ultimate/the_epic_of_alexander.ts (573ms)
│  └─ Check variable declaration from (line 352, char 7) to (line 3191, char 2) (505ms)
│     └─ Check expression from (line 352, char 38) to (line 3191, char 2) (375ms)
│        └─ Check expression from (line 553, char 13) to (line 2701, char 4) (308ms)
├─ Check file /mnt/c/Users/valarnin/Source/valarnin-cactbot/test/helper/test_trigger.ts (540ms)
│  └─ Check deferred node from (line 391, char 34) to (line 562, char 4) (485ms)
│     └─ Check variable declaration from (line 520, char 20) to (line 520, char 28) (407ms)
│        └─ Check expression from (line 520, char 32) to (line 520, char 50) (407ms)
├─ Check file /mnt/c/Users/valarnin/Source/valarnin-cactbot/ui/raidboss/data/05-shb/eureka/zadnor.ts (451ms)
│  └─ Check variable declaration from (line 105, char 7) to (line 2316, char 2) (425ms)
│     └─ Check expression from (line 105, char 38) to (line 2316, char 2) (324ms)
│        └─ Check expression from (line 109, char 13) to (line 1659, char 4) (262ms)
├─ Check file /mnt/c/Users/valarnin/Source/valarnin-cactbot/ui/raidboss/data/04-sb/ultimate/unending_coil_ultimate.ts (434ms)
│  └─ Check variable declaration from (line 175, char 7) to (line 2036, char 2) (374ms)
│     └─ Check expression from (line 175, char 38) to (line 2036, char 2) (270ms)
│        └─ Check expression from (line 219, char 13) to (line 1575, char 4) (231ms)
├─ Check file /mnt/c/Users/valarnin/Source/valarnin-cactbot/ui/raidboss/data/05-shb/raid/e12s.ts (370ms)
│  └─ Check variable declaration from (line 278, char 7) to (line 2063, char 2) (297ms)
│     └─ Check expression from (line 278, char 38) to (line 2063, char 2) (207ms)
│        └─ Check expression from (line 281, char 13) to (line 1674, char 4) (179ms)
└─ Check file /mnt/c/Users/valarnin/Source/valarnin-cactbot/ui/raidboss/data/05-shb/eureka/delubrum_reginae.ts (315ms)
   └─ Check variable declaration from (line 43, char 7) to (line 1918, char 2) (291ms)
      └─ Check expression from (line 43, char 38) to (line 1918, char 2) (223ms)
         └─ Check expression from (line 46, char 13) to (line 1275, char 4) (159ms)

Duplicate packages
├─ schema-utils
│  ├─ Version 4.0.0 from /mnt/c/Users/valarnin/Source/valarnin-cactbot/node_modules/copy-webpack-plugin/node_modules/schema-utils
│  ├─ Version 4.0.0 from /mnt/c/Users/valarnin/Source/valarnin-cactbot/node_modules/webpack-dev-server/node_modules/schema-utils
│  └─ Version 3.1.1 from /mnt/c/Users/valarnin/Source/valarnin-cactbot/node_modules/webpack/node_modules/schema-utils
└─ ajv
   ├─ Version 8.11.0 from /mnt/c/Users/valarnin/Source/valarnin-cactbot/node_modules/ajv
   └─ Unknown version from /mnt/c/Users/valarnin/Source/valarnin-cactbot/node_modules/copy-webpack-plugin/node_modules/ajv

I'm not sure what can be done about large trigger files, but the popup-text.ts and test_trigger.ts cases could probably be improved?

quisquous commented 2 years ago

Oooh, interesting! Thanks for the analysis. Which popup-text.ts lines are those? I'm not quite sure I follow what the output is pointing at here.

valarnin commented 2 years ago

Sorry, I had forgotten that I had played around with fixes between when I originally ran the first trace and when I re-did the analysis for opening this issue. Updated the trace outputs above to actually match the main code.

It's a standard trace tree, so it starts from more generic (file-level) and works down to more specific (exact token).

So for popup-text.ts, it's saying type checking the entire file takes 2467ms to compile.

Type checking the segment from 697:11 -> 697:39 took 1918ms of that 2467ms time. (this.ProcessTrigger(trigger))

Type checking the segment from 697:31 -> 697:38 took 1915ms of that 1918ms time. (trigger)

Basically, it's saying that type checking the trigger parameter is taking almost 2 full seconds.

trim21 commented 2 years ago

I did a little digging in here, and this is my tracing baseline:

```text > tsc --noEmit --generateTrace ../cactbot-tracing real 0m35.580s user 0m0.045s sys 0m0.120s Hot Spots ├─ Check file c:\users\trim21\proj\cactbot\ui\raidboss\popup-text.ts (1231ms) │ └─ Check expression from (line 728, char 11) to (line 728, char 39) (937ms) │ └─ Check expression from (line 728, char 31) to (line 728, char 38) (934ms) ├─ Check file c:\users\trim21\proj\cactbot\test\helper\test_trigger.ts (602ms) │ └─ Check deferred node from (line 39, char 25) to (line 682, char 2) (437ms) │ └─ Check expression from (line 217, char 3) to (line 275, char 5) (436ms) │ └─ Check expression from (line 217, char 33) to (line 275, char 4) (436ms) │ └─ Check expression from (line 273, char 21) to (line 273, char 84) (436ms) │ └─ Check expression from (line 273, char 35) to (line 273, char 41) (436ms) │ └─ Check expression from (line 271, char 18) to (line 271, char 31) (436ms) │ └─ Check expression from (line 271, char 18) to (line 271, char 24) (435ms) │ └─ Check expression from (line 243, char 20) to (line 243, char 30) (424ms) │ └─ Check expression from (line 243, char 20) to (line 243, char 27) (424ms) ├─ Check file c:\users\trim21\proj\cactbot\ui\raidboss\data\05-shb\eureka\delubrum_reginae_savage.ts (411ms) │ └─ Check variable declaration from (line 94, char 7) to (line 3712, char 2) (371ms) │ └─ Check expression from (line 94, char 38) to (line 3712, char 2) (318ms) │ └─ Check expression from (line 173, char 13) to (line 2600, char 4) (192ms) ├─ Check file c:\users\trim21\proj\cactbot\node_modules\typescript\lib\lib.dom.d.ts (385ms) ├─ Check file c:\users\trim21\proj\cactbot\ui\raidboss\data\06-ew\ultimate\dragonsongs_reprise_ultimate.ts (361ms) │ └─ Check variable declaration from (line 169, char 7) to (line 3548, char 2) (307ms) │ └─ Check expression from (line 169, char 38) to (line 3548, char 2) (243ms) │ └─ Check expression from (line 257, char 13) to (line 2872, char 4) (196ms) ├─ Check file c:\users\trim21\proj\cactbot\ui\raidboss\data\05-shb\ultimate\the_epic_of_alexander.ts (330ms) │ └─ Check variable declaration from (line 351, char 7) to (line 3195, char 2) (294ms) │ └─ Check expression from (line 351, char 38) to (line 3195, char 2) (229ms) │ └─ Check expression from (line 552, char 13) to (line 2705, char 4) (196ms) ├─ Check file c:\users\trim21\proj\cactbot\ui\raidboss\data\05-shb\eureka\zadnor.ts (290ms) │ └─ Check variable declaration from (line 103, char 7) to (line 2316, char 2) (275ms) │ └─ Check expression from (line 103, char 38) to (line 2316, char 2) (218ms) │ └─ Check expression from (line 107, char 13) to (line 1657, char 4) (182ms) ├─ Check file c:\users\trim21\proj\cactbot\ui\raidboss\data\06-ew\raid\p8s.ts (260ms) │ └─ Check variable declaration from (line 157, char 7) to (line 2789, char 2) (216ms) │ └─ Check expression from (line 157, char 38) to (line 2789, char 2) (170ms) │ └─ Check expression from (line 212, char 13) to (line 2521, char 4) (156ms) └─ Check file c:\users\trim21\proj\cactbot\ui\raidboss\data\04-sb\ultimate\unending_coil_ultimate.ts (222ms) └─ Check variable declaration from (line 177, char 7) to (line 2112, char 2) (192ms) └─ Check expression from (line 177, char 38) to (line 2112, char 2) (152ms) └─ Check expression from (line 221, char 13) to (line 1622, char 4) (132ms) Duplicate packages ├─ ajv │ ├─ Version 6.12.6 from C:/Users/Trim21/proj/cactbot/node_modules/ajv │ └─ Version 8.11.0 from C:/Users/Trim21/proj/cactbot/node_modules/copy-webpack-plugin/node_modules/ajv └─ schema-utils ├─ Version 4.0.0 from C:/Users/Trim21/proj/cactbot/node_modules/copy-webpack-plugin/node_modules/schema-utils └─ Version 3.1.1 from C:/Users/Trim21/proj/cactbot/node_modules/webpack/node_modules/schema-utils ```

Currently (after the patch of new trigger style), the slowness of popup-text.ts file is beacuse when getting or setting a property of trigger (like trigger.id), it need to narrow the type of trigger (here it's BaseTrigger<RaidbossData, "GameLog"> & PartialNetRegexTrigger<"GameLog"> & Partial<BaseTrigger<RaidbossData, "None"> & PartialRegexTrigger & PartialNetRegexTrigger<...>>) | ... 39 more ... | (BaseTrigger<...> & ... 2 more ...) to anyone of them, and it's really slow (tsc even can't show the full list!).

And it's not hard to solve, we create a new variable const processed: ProcessedTrigger = {} and copy all fields, and set property to this new processed trigger, and orderedTriggers.push(processed);

roughly like this:

```diff const id = trigger.id; + if (trigger.disabled === true) + continue; + if (!isRegexTrigger(trigger) && !isNetRegexTrigger(trigger)) { console.error(`Trigger ${id}: has no regex property specified`); continue; } - this.ProcessTrigger(trigger); - let found = false; + const processed: ProcessedTrigger = { + id: trigger.id, + filename: trigger.filename, + condition: trigger.condition, + preRun: trigger.preRun, + delaySeconds: trigger.delaySeconds, + durationSeconds: trigger.durationSeconds, + suppressSeconds: trigger.suppressSeconds, + promise: trigger.promise, + sound: trigger.sound, + soundVolume: trigger.soundVolume, + response: trigger.response, + alarmText: trigger.alarmText, + alertText: trigger.alertText, + infoText: trigger.infoText, + tts: trigger.tts, + run: trigger.run, + output: TriggerOutputProxy.makeOutput( + trigger, + this.options.DisplayLanguage, + this.options.PerTriggerAutoConfig, + ), + }; const triggerObject: { [key: string]: unknown } = trigger; @@ -739,13 +765,11 @@ export class PopupText { const defaultRegex = trigger.regex; const localeRegex = triggerObject[regexParserLang]; if (localeRegex instanceof RegExp) { - trigger.localRegex = Regexes.parse(localeRegex); - orderedTriggers.push(trigger); + processed.localRegex = Regexes.parse(localeRegex); found = true; } else if (defaultRegex) { const trans = translateRegex(defaultRegex, this.parserLang, set.timelineReplace); - trigger.localRegex = Regexes.parse(trans); - orderedTriggers.push(trigger); + processed.localRegex = Regexes.parse(trans); found = true; } } @@ -762,12 +786,11 @@ export class PopupText { trigger.localNetRegex = Regexes.parse(localeNetRegex); orderedTriggers.push(trigger); found = true; - } else if (defaultNetRegex) { + } else { // simple netRegex trigger, need to build netRegex and translate if (defaultNetRegex instanceof RegExp) { const trans = translateRegex(defaultNetRegex, this.parserLang, set.timelineReplace); - trigger.localNetRegex = Regexes.parse(trans); - orderedTriggers.push(trigger); + processed.localNetRegex = Regexes.parse(trans); found = true; } else { if (trigger.type === undefined) { @@ -779,17 +802,18 @@ export class PopupText { trigger.type, translateRegexBuildParam(defaultNetRegex, this.parserLang, set.timelineReplace), ); - trigger.localNetRegex = Regexes.parse(re); - orderedTriggers.push(trigger); + processed.localNetRegex = Regexes.parse(re); found = true; } } } if (!found) { - console.error('Trigger ' + trigger.id + ': missing regex and netRegex'); + console.error(`Trigger ${processed.id ?? ''}: missing regex and netRegex`); continue; } + + orderedTriggers.push(processed); } ```

But popup-text.ts only take 1s in my all 35s building.

most of the time is spending on checking the type of TriggerSet<Data>, and to be more to be more specific, checking triggers?: NetRegexTrigger<Data>[];, and I didn't find a real solution for that.

There are 41 netLog types and we have no way to reduce that, so in any case there will be more than (41+m)*N type union to narrow, but we can try to reduce the union of trigger type by make N=1.

by merging BaseTrigger with PartialNetTrigger, this reduce 35s to 31s:

This doesn't make any actually change on types, it just speedup type narrow on triggers, this even help to reduce the time spending on popup-text.ts

```diff diff --git a/types/trigger.d.ts b/types/trigger.d.ts index 686c4f0b7..043834603 100644 --- a/types/trigger.d.ts +++ b/types/trigger.d.ts @@ -106,8 +106,15 @@ export type TriggerField = { +export type BaseTrigger = Omit< + BaseNetTrigger, + 'type' | 'netRegex' +>; + +type BaseNetTrigger = { id: string; + type?: Type; + netRegex: NetParams[Type] | CactbotBaseRegExp; disabled?: boolean; condition?: TriggerField; preRun?: TriggerField; @@ -128,20 +135,13 @@ export type BaseTrigger = outputStrings?: OutputStrings; }; -// new trigger type, regex is build by core. -type PartialNetRegexTrigger = { - type?: T; - netRegex: NetParams[T] | CactbotBaseRegExp; -}; - export type NetRegexTrigger = TriggerTypes extends infer T - ? T extends TriggerTypes ? (BaseTrigger & PartialNetRegexTrigger) + ? T extends TriggerTypes ? BaseNetTrigger : never : never; export type GeneralNetRegexTrigger = - & BaseTrigger - & PartialNetRegexTrigger; + BaseNetTrigger; type PartialRegexTrigger = { regex: RegExp; @@ -183,8 +183,8 @@ export type BaseTriggerSet = { overrideTimelineFile?: boolean; timelineFile?: string; timeline?: TimelineField; - triggers?: (NetRegexTrigger | DisabledTrigger)[]; - timelineTriggers?: (TimelineTrigger | DisabledTrigger)[]; + triggers?: NetRegexTrigger[]; + timelineTriggers?: TimelineTrigger[]; timelineReplace?: TimelineReplacement[]; timelineStyles?: TimelineStyle[]; }; @@ -202,11 +202,7 @@ export type TriggerSet = // Less strict type for user triggers + built-in triggers, including deprecated fields. export type LooseTimelineTrigger = Partial>; -export type LooseTrigger = Partial< - & BaseTrigger - & PartialRegexTrigger - & PartialNetRegexTrigger<'None'> ->; +export type LooseTrigger = Partial & PartialRegexTrigger>; export type LooseTriggerSet = & Exclude>, 'triggers' | 'timelineTriggers'> ```
```text > cactbot@0.27.21 tsc-no-emit > tsc --noEmit --generateTrace ../cactbot-tracing real 0m31.967s user 0m0.060s sys 0m0.107s Hot Spots ├─ Check file c:\users\trim21\proj\cactbot\ui\raidboss\data\05-shb\eureka\delubrum_reginae_savage.ts (397ms) │ └─ Check variable declaration from (line 94, char 7) to (line 3712, char 2) (298ms) │ └─ Check expression from (line 94, char 38) to (line 3712, char 2) (248ms) ├─ Check file c:\users\trim21\proj\cactbot\node_modules\typescript\lib\lib.dom.d.ts (380ms) ├─ Check file c:\users\trim21\proj\cactbot\ui\raidboss\popup-text.ts (362ms) ├─ Check file c:\users\trim21\proj\cactbot\ui\raidboss\data\06-ew\ultimate\dragonsongs_reprise_ultimate.ts (246ms) │ └─ Check variable declaration from (line 169, char 7) to (line 3548, char 2) (200ms) │ └─ Check expression from (line 169, char 38) to (line 3548, char 2) (173ms) │ └─ Check expression from (line 257, char 13) to (line 2872, char 4) (126ms) ├─ Check file c:\users\trim21\proj\cactbot\ui\raidboss\data\05-shb\ultimate\the_epic_of_alexander.ts (230ms) │ └─ Check variable declaration from (line 351, char 7) to (line 3195, char 2) (206ms) │ └─ Check expression from (line 351, char 38) to (line 3195, char 2) (178ms) │ └─ Check expression from (line 552, char 13) to (line 2705, char 4) (143ms) └─ Check file c:\users\trim21\proj\cactbot\ui\raidboss\data\05-shb\eureka\zadnor.ts (226ms) └─ Check variable declaration from (line 103, char 7) to (line 2316, char 2) (216ms) └─ Check expression from (line 103, char 38) to (line 2316, char 2) (195ms) └─ Check expression from (line 107, char 13) to (line 1657, char 4) (159ms) No duplicate packages found ```

And currently we have 254 raidboss triggers, I guess maybe is just too many files for tsc...