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.

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

Slow schema documentation tooltip #3259

Open
hoffa opened this issue Mar 21, 2023 · 5 comments
Open

Slow schema documentation tooltip #3259

hoffa opened this issue Mar 21, 2023 · 5 comments
Labels
aws-documents AWS-specific programming languages, DSLs, filetypes, LSP, JSON schema aws-documents-sam-cfn LSP or JSON schema for CFN/SAM template yaml/json files bug We can reproduce the issue and confirmed it is a bug. performance

Comments

@hoffa
Copy link

hoffa commented Mar 21, 2023

Problem

Great to see the unified SAM and CloudFormation schema in the latest release!

I noticed it takes about a second on my computer for the documentation tooltip to show up upon hover:

Screenshot 2023-03-21 at 15 19 54

However, if I use the modeline to manually override with the same schema, it's pretty much instantaneous:

# yaml-language-server: $schema=https://raw.githubusercontent.com/aws/serverless-application-model/main/samtranslator/schema/schema.json

The final visible behavior is identical in both cases.

While in absolute terms it's a small difference, in relative terms it's very noticeable, and hinders the experience of quickly exploring a template (this NN article is also relevant).

Steps to reproduce the issue

  1. Create a new YAML file with the following contents:

    Transform: AWS::Serverless-2016-10-31
    Resources:
      MyTable:
        Type: AWS::Serverless::SimpleTable
        Properties:
          TableName: foo
  2. Hover over TableName. On my computer it takes about a second for the hover tooltip to show up.

  3. Now manually override with the same schema by adding the following at the top:

    # yaml-language-server: $schema=https://raw.githubusercontent.com/aws/serverless-application-model/main/samtranslator/schema/schema.json
  4. Try hovering over TableName. It should be pretty much instantaneous; on my computer it's significantly snappier than before. The tooltip is otherwise exactly the same.

Expected behavior

Documentation should show up as fast as without AWS Toolkit (i.e. using just the YAML extension).

System details (run the AWS: About Toolkit command)

  • OS: Darwin x64 22.3.0
  • Visual Studio Code extension host: 1.76.2
  • AWS Toolkit: 1.65.0
  • node: 16.14.2
  • electron: 19.1.11
@hoffa hoffa added the bug We can reproduce the issue and confirmed it is a bug. label Mar 21, 2023
@justinmk3 justinmk3 added aws-documents-sam-cfn LSP or JSON schema for CFN/SAM template yaml/json files aws-documents AWS-specific programming languages, DSLs, filetypes, LSP, JSON schema labels Mar 21, 2023
@nkomonen-amazon
Copy link
Contributor

This is caused by a limitation of the current implementation. We are plugging in to the VSCode Yaml extension, and the API only allows us to pass the content of a schema, not the parsed schema (AST). Due to this, each interaction requires the entire CFN schema to be parsed, which slows it down a lot.

We are looking to eventually switch to a language server implementation which should resolve this issue.

return readFileSync(vscode.Uri.parse(uri).fsPath).toString()

@hoffa
Copy link
Author

hoffa commented Mar 23, 2023

Due to this, each interaction requires the entire CFN schema to be parsed, which slows it down a lot.

@nkomonen-amazon So there are no improvements that can be done?

Can you help me understand why the schema needs to be passed at all, instead of only passing a reference, and letting the YAML extension do its thing?

@nkomonen-amazon
Copy link
Contributor

nkomonen-amazon commented Mar 27, 2023

@hoffa Theres nothing we can immediately do with the current implementation. The implementation that was originally written was a best effort attempt.

What is happening is we are using the VSCode extension API of the YAML VSCode extension, not the YAML Language Server. Though the VSCode extension builds on top of the language server. And the VSCode extension API is written where it does not cache schema results, instead computing them each time

@hoffa
Copy link
Author

hoffa commented Mar 27, 2023

@nkomonen-amazon I see. Is it possible to make upstream improvements, e.g. add an API that takes a schema URI instead of schema content, and which would under the hood work the same way as when a schema is specified through the modeline (which is fast; for which redhat-developer/yaml-language-server#806 had to be merged for it to work)?

@justinmk3
Copy link
Contributor

Is it possible to make upstream improvements

@hoffa Yes. Help welcome, we don't have plans to drive that.

We are working on an alternative LSP-based approach here:
https://github.com/aws/aws-toolkit-common/tree/main/lsp#aws-documents-language-server

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws-documents AWS-specific programming languages, DSLs, filetypes, LSP, JSON schema aws-documents-sam-cfn LSP or JSON schema for CFN/SAM template yaml/json files bug We can reproduce the issue and confirmed it is a bug. performance
Projects
None yet
Development

No branches or pull requests

3 participants