Migrate DocumentHighlight to Listener pattern

[st0012]

Since DocumentHighlight focuses on the type of the visited node instead of the visiting event, I think it makes sense to introduce a new on_visit event for it. It saves us from expanding the nodes list in HighlightTarget into a list of fragmented on_<node> events.

It should also benefit other requests that currently define the visit method.

Motivation

Implementation

Automated Tests

Manual Tests

[github-actions]

Benchmark results in seconds (slowest at top)

          textDocument/completion average: 0.260739 std_dev: 0.00525
          textDocument/diagnostic average: 0.043481 std_dev: 0.008692
      textDocument/selectionRange average: 0.00342 std_dev: 0.000649
   textDocument/documentHighlight average: 0.001832 std_dev: 0.000148
 textDocument/semanticTokens/full average: 0.001728 std_dev: 0.000337
      textDocument/documentSymbol average: 0.001728 std_dev: 9.2e-05
            textDocument/codeLens average: 0.001723 std_dev: 0.000186
        textDocument/documentLink average: 0.001717 std_dev: 0.000173
        textDocument/foldingRange average: 0.001601 std_dev: 0.000103
textDocument/semanticTokens/range average: 0.000889 std_dev: 5.8e-05
               codeAction/resolve average: 0.000662 std_dev: 9.8e-05
               textDocument/hover average: 0.000621 std_dev: 8.7e-05
           textDocument/inlayHint average: 0.00061 std_dev: 6.3e-05
    textDocument/onTypeFormatting average: 0.000118 std_dev: 7.7e-05
          textDocument/formatting average: 7.3e-05 std_dev: 0.000304
          textDocument/codeAction average: 4.0e-05 std_dev: 3.3e-05


================================================================================
Comparison with main branch:

 textDocument/semanticTokens/full unchanged
textDocument/semanticTokens/range slower by 12.46 %
      textDocument/documentSymbol unchanged
        textDocument/foldingRange unchanged
          textDocument/formatting unchanged
          textDocument/diagnostic unchanged
        textDocument/documentLink unchanged
           textDocument/inlayHint slower by 16.504 %
      textDocument/selectionRange unchanged
   textDocument/documentHighlight slower by 24.3 %
               textDocument/hover unchanged
          textDocument/codeAction unchanged
    textDocument/onTypeFormatting unchanged
               codeAction/resolve unchanged
          textDocument/completion unchanged
            textDocument/codeLens unchanged


At least one benchmark is slower than the main branch.

[st0012]

I've rerun benchmark around 8 times and documentHighlight is consistently ~25% slower, so I think it's safe to say this approach causes performance regression. But I'm not sure why this is the case 🤔

[st0012]

Wrt the slowdown, I notice that it also happens to other migration PRs:

• Migrate semantic highlighting to the listener pattern #727 sees all related requests slow down:
  • textDocument/semanticTokens/full slower by 71.448 %
  • textDocument/documentSymbol slower by 78.778 %
  • textDocument/documentLink slower by 108.592 %
  • textDocument/codeLens slower by 112.232 %
• Migrate inlay hint to listener #703 sees textDocument/inlayHint slower by 20.868 %
• Migrate document link to listener #660 sees textDocument/documentLink slower by 54.743 %

So it's likely not specific to this change. We should look into what's causing the slowdown though OR if our benchmark is too sensitive.

[vinistock]

Not a blocker, but just a thought: what if we move this to Executor? Would that maybe simplify things a bit?

[st0012]

Consider the data type it uses (HighlightTarget) is still specific to this request, I prefer keeping it at the same place until we share it somewhere else.