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

[plugin] fetching tasks is slow for big multi-root workspaces #7672

Closed
akosyakov opened this issue Apr 27, 2020 · 9 comments · Fixed by #7676
Closed

[plugin] fetching tasks is slow for big multi-root workspaces #7672

akosyakov opened this issue Apr 27, 2020 · 9 comments · Fixed by #7676
Assignees
Labels
multi-root issues related to multi-root support performance issues related to performance plug-in system issues related to the plug-in system tasks issues related to the task system

Comments

@akosyakov
Copy link
Member

validateSchema should be computed once and cached

but I am not sure that we need it at all, does VS Code validate task configs?

We removed such logic already for preferences, since it is slow and VS Code does not care about.

cc @elaihau @RomanNikitenko

@akosyakov akosyakov added performance issues related to performance tasks issues related to the task system labels Apr 27, 2020
@akosyakov akosyakov added the critical critical bugs / problems label Apr 27, 2020
@akosyakov

This comment has been minimized.

@akosyakov akosyakov added multi-root issues related to multi-root support plug-in system issues related to the plug-in system labels Apr 27, 2020
@akosyakov akosyakov assigned akosyakov and unassigned akosyakov Apr 27, 2020
@akosyakov akosyakov removed the critical critical bugs / problems label Apr 27, 2020
@akosyakov
Copy link
Member Author

akosyakov commented Apr 27, 2020

I was able to reproduce it with a bit of customizations JLab project:

{
  // See https://go.microsoft.com/fwlink/?LinkId=733558
  // for the documentation about the tasks.json format
  "version": "2.0.0",
  "tasks": [
    {
      "type": "npm",
      "script": "build",
      "problemMatcher": [
        "$tsc"
      ]
    }
  ]
}
  • create a multi-root workspace from this JSON (replace file:///workspace with your absolute path) :
{
   "folders": [
      {
         "path": "file:///workspace/jupyterlab"
      },
      {
         "path": "file:///workspace/jupyterlab/dev_mode"
      },
      {
         "path": "file:///workspace/jupyterlab/examples"
      },
      {
         "path": "file:///workspace/jupyterlab/packages"
      },
      {
         "path": "file:///workspace/jupyterlab/packages/services/examples/node"
      },
      {
         "path": "file:///workspace/jupyterlab/packages/services/examples/browser"
      },
      {
         "path": "file:///workspace/jupyterlab/packages/services/examples/typescript-browser-with-output"
      },
      {
         "path": "file:///workspace/jupyterlab/buildutils"
      },
      {
         "path": "file:///workspace/jupyterlab/buildutils/template"
      },
      {
         "path": "file:///workspace/jupyterlab/buildutils/buildutils/test-template"
      },
      {
         "path": "file:///workspace/jupyterlab/tests"
      },
      {
         "path": "file:///workspace/jupyterlab/testutils"
      },
      {
         "path": "file:///workspace/jupyterlab/jupyterlab/tests/mock_packages/extension"
      }
   ],
   "settings": {}
}
  • start Theia and pass the path to the multi-root workspace
  • open README.md as an editor and for preview
  • reload the page
  • try to navigate in the explorer, till npm scripts appear
  • notice how sluggish it is and how long does it take for npm scripts to appear

The profiling snapshot: https://drive.google.com/file/d/1pyCUMaGm-7scjJ-KMqHmfm2gINs6BdVx/view?usp=sharing

There are 2 hot spots:

  • isTaskConfigValid (it only happens if you have tasks.json)

Screenshot 2020-04-27 at 14 59 41

- comparing tasks in `$fetchTasks` from the plugin system:

Screenshot 2020-04-27 at 14 59 51

@akosyakov akosyakov changed the title isTaskConfigValid is slow [plugin] fetching tasks is slow for big multi-root workspaces Apr 27, 2020
@akosyakov
Copy link
Member Author

isTaskConfigValid is handled in #7676

@akosyakov akosyakov self-assigned this Apr 28, 2020
@akosyakov
Copy link
Member Author

@elaihau Do you know how to test whether we need to call compareTasks in $fetchTasks? I wonder do we need it at all or we simply should return all tasks? How did you validate VS Code extensions' expectations?

@akosyakov
Copy link
Member Author

akosyakov commented Apr 28, 2020

VS Code does not remove duplicate, it returns all tasks:
Screenshot 2020-04-28 at 10 35 14

I does not look like $fetchTasks is aligned with VS Code extensions' expectations. That's what Theia gives:
Screenshot 2020-04-28 at 10 44 16

Tested with https://github.com/akosyakov/vscode-fetch-tasks-test and adding following tasks.json under .vscode in this repo:

{
  // See https://go.microsoft.com/fwlink/?LinkId=733558
  // for the documentation about the tasks.json format
  "version": "2.0.0",
  "tasks": [
    {
      "label": "build",
      "type": "shell",
      "command": "msbuild",
      "args": [
        // Ask msbuild to generate full paths for file names.
        "/property:GenerateFullPaths=true",
        "/t:build",
        // Do not generate summary otherwise it leads to duplicate errors in Problems panel
        "/consoleloggerparameters:NoSummary"
      ],
      "group": "build",
      "presentation": {
        // Reveal the output only if unrecognized errors occur.
        "reveal": "silent"
      },
      // Use the standard MS compiler pattern to detect errors, warnings and infos
      "problemMatcher": "$msCompile"
    },
    {
      "label": "build",
      "type": "shell",
      "command": "msbuild",
      "args": [
        // Ask msbuild to generate full paths for file names.
        "/property:GenerateFullPaths=true",
        "/t:build",
        // Do not generate summary otherwise it leads to duplicate errors in Problems panel
        "/consoleloggerparameters:NoSummary"
      ],
      "group": "build",
      "presentation": {
        // Reveal the output only if unrecognized errors occur.
        "reveal": "silent"
      },
      // Use the standard MS compiler pattern to detect errors, warnings and infos
      "problemMatcher": "$msCompile"
    }
  ]
}

@elaihau
Copy link
Contributor

elaihau commented Apr 29, 2020

@elaihau Do you know how to test whether we need to call compareTasks in $fetchTasks? I wonder do we need it at all or we simply should return all tasks? How did you validate VS Code extensions' expectations?

If we don't want to

  • take care of identical tasks, or
  • filter out the invalid task configs,
    we probably can make $fetchTasks() a lot faster

I took a look at the $fetchTasks() function in tasks-main.ts and noticed that it does not take the cutomized detected tasks into consideration.
Say, you have npm: build defined in package.json, and customize it in tasks.json this way:

{
  "type": "npm",
  "script": "build",
  "label": "Build the project"
}

The current $fetchTasks() simply returns the npm: build - the label you added to the task (i.e., "Build the project") is not taken care of. Is it something we need to fix? @akosyakov

@akosyakov
Copy link
Member Author

akosyakov commented Apr 29, 2020

If we don't want to take care of identical tasks, or filter out the invalid task configs, we probably can make $fetchTasks() a lot faster

I've simplified it in aba5392 Could you have a look there? But it only to make it faster it does not make sure that it aligned with VS Code extensions' expectations.

I took a look at the $fetchTasks() function in tasks-main.ts and noticed that it does not take the cutomized detected tasks into consideration.
Say, you have npm: build defined in package.json, and customize it in tasks.json this way:

I've opened #7681 for VS Code compatibility. Ideally results should be aligned as much as possible. But you can also try to investigate whether it's worth an effort, i.e. whether it can affect some extensions, users and how many. Let's have a look at it separate from performance optimisations.

@elaihau
Copy link
Contributor

elaihau commented May 2, 2020

I've simplified it in aba5392 Could you have a look there? But it only to make it faster it does not make sure that it aligned with VS Code extensions' expectations.

@akosyakov
Sorry for the late response.
I believe we missed the part that avoids duplications.

I tried it in vscode with an extension.
If a detected task is configured in tasks.json, fetchTasks() in vscode returns one task.
In the current Theia, it returns both.

However, in vscode, same tasks, or tasks with same labels, can be contributed by different providers multiple times.

Do you want me to create an issue for it?

@akosyakov
Copy link
Member Author

@elaihau It should be covered by doing #7681. That we have broken tests if we miss something again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multi-root issues related to multi-root support performance issues related to performance plug-in system issues related to the plug-in system tasks issues related to the task system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants