Run Task related refactoring

[RomanNikitenko]

What it does

1. Change the order of search task configuration at running
At the moment at running a task the order of search is:

• search a config among provided/detected tasks
• search a config among configured tasks
• search a config among customized tasks

It works and sure it founds the corresponding config for running.
The problem is: it does extra requests when it tries to find a configured task configuration.

Let's say user runs some configured task:

• task service tries to find a config among provided tasks first
• it tries to get cached provided task
• sure, there is no configured task among provided tasks, so it tries to get all provided tasks
• it gets all providers, fetches and caches all provided tasks
• it doesn't find the corresponding config, so next it tries to get a config among configured tasks

I think we can change the order and do search among configured tasks first. It will allow to avoid extra requests: it just check if a task exists in map - we don't read tasks.json file at this step, so it can not slow down a search.
So:

• if we are looking for a configured task, it finds the corresponding config and do not search among provided tasks
• if we are looking for a provided task, it just check if a task exists in map, then it tries to find among provided tasks

2. Separate running a compound task
At the moment for any task config we do extra logic related to running a compound task.

But on the next step we need it if a task is compound only.

My propose is: separate this logic in another method and execute it if it's really required.

How to test

It's just refactoring, so we should avoid a regression.

1. Try to run any configured task
2. Try to run any detected task
3. Try to run a compound task

For compound tasks testing I used in tasks.json file configurations like:

"tasks": [
    {
      "type": "shell",
      "label": "1111",
      "command": "sleep 2 && echo 111111",
      "dependsOn": "2222"
    },
    {
      "type": "shell",
      "label": "2222",
      "command": "sleep 2 && echo 222222",
      "dependsOn": "3333"
    },
    {
      "type": "shell",
      "label": "3333",
      "command": "sleep 2 && echo 33333"
    }
  ]

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[RomanNikitenko]

@elaihau
could you take a look, is it important to keep the current order of search or we can change it
thanks in advance!

[elaihau]

I smoke tested

• configured tasks
• detected tasks
• customized detected tasks
• task dependencies
  and they worked properly.

Thank you Roman !

[akosyakov]

I think changes looks good. Not sure about the motivation. Is it performance optimisation? Usually we test them by looking at numbers before and after to understand whether refactoring is worth it.

[RomanNikitenko]

Not sure about the motivation. Is it performance optimisation?

yes, the PR allows to:

• avoid execution compound task related logic for ordinary tasks. For example, why we first get all tasks for a workspace and on the next step check if it's really required.
• skip extra steps at running configured task (get all providers, fetch and cache all provided tasks)

[akosyakov]

I have not measured anything, but changes sound reasonable.

[RomanNikitenko]

Not sure about the motivation. Is it performance optimisation? Usually we test them by looking at numbers before and after to understand whether refactoring is worth it.

@akosyakov
I did some measure using console.time()-console.timeEnd() for simple (without dependsOn) configured shell and detected tsc tasks. I used repo from How to test section of the PR #7676 to have a lot of tasks.

I had 3 points for master branch:

• get config
• get workspace tasks
• run: between starting point and this point

Please see details:

I had only 2 points for my branch because get workspace tasks part is not involved in running simple (without dependsOn) task process after applying my changes.

[RomanNikitenko]

Results, gifs
You can skip this comment and go to the next one with screenshots

For master branch
Detected task:

Configured task:

For my branch
Detected task:

Configured task:

[RomanNikitenko]

The difference is:

Detected task, master

Detected task, my branch

Configured, master

Configured, my branch

[RomanNikitenko]

So, you can see that:

For a configured task get config step reduced from ~300 ms to 0.035 ms because my changes allows to skip extra steps at running configured task (get all providers, fetch and cache all provided tasks) and just take the corresponding config from map.

For a detected task get config step after my changes we have the same time ~ 60 ms - so after my changes we don't have some regression for getting a configuration of detected tasks

For both configured and detected simple task time between starting point and this point was reduced from 700-1000 ms to 50-60 ms because my changes allows to avoid execution compound task related logic at running a simple task

According to the screenshots above you can see that we have huge time for getting workspace tasks - about 600-800 ms. So we should consider if we can reduce this time and improve our code.

p.s. I think that measurement depends on number of detected and configured tasks in a workspace, but it should reflect our problems in general.

@akosyakov @elaihau
wdyt?
is it good way to detect problem places?
should we have an issue to investigate get workspace tasks delay as possible place for improvement?
please let me know if you can see something wrong in my measurement

[RomanNikitenko]

I need to have these changes merged to continue investigate and detect delay at running tasks, so I'm merging this PR.

But we can continue to discuss here the result of the measurement.

[tsmaeder]

@RomanNikitenko if I understand the reasoning correctly, the idea is that if we have a detected task, we will look for a customization for that task anyway, so it doesn't matter whether we find the customized task or the contributed task...it will result in the same task being executed, right?

[RomanNikitenko]

@tsmaeder

There were two improvements:

• separation compound task related logic
• prevention extra steps at running a configured task

First one has influence on all simple tasks: configured, detected and customized.
Second one reduces search time for a configured task because:

skip extra steps at running configured task (get all providers, fetch and cache all provided tasks)

Regarding

the idea is that if we have a detected task, we will look for a customization for that task anyway, so it doesn't matter whether we find the customized task or the contributed task...it will result in the same task being executed, right?

Sorry, I don't have a ready answer...