Refactor navigation bar

[ghost]

Fixes #8779, #5258, #4191, and #8218.
Gets us closer to #7523 (combines TS and JS code paths for navigation bar).
Takes over from #8958 and does not conflict with current master.

[ghost]

Here's a summary of performance tests:

• TS:
  • Large: 50% more time and memory
  • Small: A little more
• JS:
  • Large: A little less
  • Small: Took a lot more time for some reason. This was a very small file so could be a fluke.

Keep in mind that the refactor has been heavily optimized, and if the old version were similarly optimized it would probably be twice as fast.

How tests were done:

• Pasted texts into the navbar01 fourslash test (and added four slashes).
• Changed fourslash.ts to not parse markers, and to run navbar 150 times, collecting results in an array.
• Profiled with runtests-browser and chrome debugging tools. For memory testing used the 'timeline' and took difference between start and end memory. Keep in mind that results are being accumulated in an array with 150 elements, in addition to any garbage being generated.

checker.ts (large TS file):

• Old:
  • SessionClient.getNavigationBarItems: 5305ms
  • getNavigationBarItems (navigationBar.ts): 4705ms
  • Memory: 340MB
• Refactor:
  • SessionClient.getNavigationBarItems: 5741ms
  • getNavigationBarItems (navigationBar.ts): 6697ms
  • Memory: 550MB

tsc.ts (small TS file):

• Old:
  SessionClient.getNavigationBarItems: 433ms
  • getNavigationBarItems (navigationBar.ts): 426ms
  • Memory: 32MB
• Refactor:
  • SessionClient.getNavigationBarItems: 511ms
  • getNavigationBarItems (navigationBar.ts): 426ms
  • Memory: 41MB

typeScriptServices.js (large JS file):

• Old:
  • SessionClient.getNavigationBarItems: 27903ms
  • getNavigationBarItems (navigationBar.ts): 23051ms
  • Memory: 712MB
• Refactor:
  • SessionClient.getNavigationBarItems: 19268ms
  • getNavigationBarItems (navigationBar.ts): 20901ms
  • Memory: 790MB

chalk.js (small JS file):

• Old:
  • SessionClient.getNavigationBarItems: 107ms
  • getNavigationBarItems (navigationBar.ts): 43ms
  • Memory: 13MB
• Refactor:
  • SessionClient.getNavigationBarItems: 132ms
  • getNavigationBarItems (navigationBar.ts): 82ms
  • Memory: 20MB

Here are some things that helped with performance:

• Don't use closures in the recursion with forEachChild; use an external stack instead.
• Node.getText() and getStart() are slow if you omit the sourceFile argument, as they try to get it by climbing up parents.
• LSHost.positionToLineOffset and SessionClient.lineOffsetToPosition similarly have optional arguments which, if omitted, lead to redundant work. Functions like these could be causing performance issues elsewhere.
• undefined apparently leads to deoptimization in Chrome, so void 0 is better. (wrong; this does not help when running normally, it only has an illusory benefit during profiling)
• if (foo) is slower than if (foo !== void 0). (same)
• Using undefined values for fields is faster than making the fields optional.

Here are some things that still hurt us:

• We spend a lot of time in sortNodes and decoding/decorating navigation bar items.
• forEachChild also spends a lot of time recursing through tokens, which we just ignore.

[weswigham]

I mentioned it in person, but how does this compare to an always undefined (never passed) function argument? (IE, visitNode<T>(cbNode: (node: Node) => T, node: Node, missing?: undefined): T).

[ghost]

After running some more tests, the difference between implicit casts and void 0 appears to exist entirely within the imagination of the Chrome profiler. When running normally, it doesn't seem to make any difference.

[mhegazy]

👍