Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP: Refactor folding ranges to use emitter pattern #726

Closed
wants to merge 5 commits into from
Closed

WIP: Refactor folding ranges to use emitter pattern #726

wants to merge 5 commits into from

Conversation

andyw8
Copy link
Contributor

@andyw8 andyw8 commented May 26, 2023

Motivation

Implementation

Automated Tests

Manual Tests

andyw8
andyw8 commented May 26, 2023
View reviewed changes
lib/ruby_lsp/requests/folding_ranges.rb
extend T::Sig

SIMPLE_FOLDABLES = T.let(
[
SyntaxTree::ArrayLiteral,
SyntaxTree::Begin,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-ordered these alphabetically.

@github-actions
Copy link
Contributor

github-actions bot commented May 26, 2023 •
edited
Loading 

Benchmark results in seconds (slowest at top)

          textDocument/completion average: 0.255171 std_dev: 0.004834
          textDocument/diagnostic average: 0.04214 std_dev: 0.009314
      textDocument/selectionRange average: 0.003319 std_dev: 0.000585
   textDocument/documentHighlight average: 0.001465 std_dev: 0.000115
        textDocument/documentLink average: 0.000947 std_dev: 0.00013
            textDocument/codeLens average: 0.000927 std_dev: 7.7e-05
      textDocument/documentSymbol average: 0.000919 std_dev: 0.000213
        textDocument/foldingRange average: 0.000888 std_dev: 4.5e-05
 textDocument/semanticTokens/full average: 0.000882 std_dev: 0.000201
               codeAction/resolve average: 0.000637 std_dev: 7.5e-05
textDocument/semanticTokens/range average: 0.000629 std_dev: 2.7e-05
               textDocument/hover average: 0.000599 std_dev: 7.2e-05
           textDocument/inlayHint average: 0.000453 std_dev: 3.5e-05
    textDocument/onTypeFormatting average: 0.000107 std_dev: 5.6e-05
          textDocument/formatting average: 5.9e-05 std_dev: 0.000153
          textDocument/codeAction average: 3.3e-05 std_dev: 2.9e-05


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

 textDocument/semanticTokens/full unchanged
textDocument/semanticTokens/range unchanged
      textDocument/documentSymbol unchanged
        textDocument/foldingRange faster by 43.005 %
          textDocument/formatting unchanged
          textDocument/diagnostic unchanged
        textDocument/documentLink unchanged
           textDocument/inlayHint unchanged
      textDocument/selectionRange unchanged
   textDocument/documentHighlight unchanged
               textDocument/hover unchanged
          textDocument/codeAction unchanged
    textDocument/onTypeFormatting unchanged
               codeAction/resolve unchanged
          textDocument/completion unchanged
            textDocument/codeLens slower by 13.849 %


At least one benchmark is slower than the main branch.

@andyw8 andyw8 marked this pull request as ready for review May 30, 2023 21:14
@andyw8 andyw8 requested a review from a team as a code owner May 30, 2023 21:14
@andyw8 andyw8 requested review from egiurleo and vinistock May 30, 2023 21:14
@andyw8 andyw8 changed the title WIP: Refactor folding ranges to use emitter pattern (SKIP CI) WIP: Refactor folding ranges to use emitter pattern Jun 7, 2023
@andyw8
Copy link
Contributor Author

andyw8 commented Jun 15, 2023

As agreed with @vinistock, we'll go with the approach in #750 instead.

@andyw8 andyw8 closed this Jun 15, 2023
@greatscotty greatscotty deleted the andyw8/folding-ranges-emitter branch February 9, 2024 21:17
vinistock pushed a commit that referenced this pull request Feb 28, 2024
@github-actions
Merge pull request #726 from Shopify/dependabot/npm_and_yarn/vscode/v…
3e5597e 
…sce-2.20.0

Bump @vscode/vsce from 2.19.0 to 2.20.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@egiurleo egiurleo Awaiting requested review from egiurleo egiurleo was automatically assigned from Shopify/ruby-dev-exp

@vinistock vinistock Awaiting requested review from vinistock vinistock was automatically assigned from Shopify/ruby-dev-exp

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@andyw8