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

Saving .py files delayed by Getting code actions from ''Python' #4263

Closed
armenzg opened this issue Apr 19, 2023 · 51 comments
Closed

Saving .py files delayed by Getting code actions from ''Python' #4263

armenzg opened this issue Apr 19, 2023 · 51 comments
Assignees
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version P2

Comments

@armenzg
Copy link

armenzg commented Apr 19, 2023

Type: Bug

Behaviour

Expected vs. Actual

I expect to save a Python file and not have to wait over 15+ seconds to save.
Instead I get Getting code actions from ''Python'". Before, it used to be Jupyter getting on the way but after I uninstalled it, the problem came directly from the "Python" extension.

This is similar to microsoft/vscode-python#19808, however, I can't make any comments.

I think I've noticed this issue in the last month or two.

It does not happen all the time but when it does, it happens often.

Here's a screenshot:
image

Steps to reproduce:

  1. Edit Python file
  2. Hit Command + S to save the file

Diagnostic data

  • Python version (& distribution if applicable, e.g. Anaconda): 3.8.13
  • Type of virtual environment used (e.g. conda, venv, virtualenv, etc.): Venv
  • Value of the python.languageServer setting: Default
Output for Python in the Output panel (ViewOutput, change the drop-down the upper-right of the Output panel to Python)

LINTING: Skipping linting from Python extension, since Flake8 extension is installed and enabled.
> ./.venv/bin/black --diff --quiet ./src/sentry/issues/escalating.py.d3fb48e40859389f827f7b92bdb1d899.tmp
cwd: .
LINTING: Skipping linting from Python extension, since Flake8 extension is installed and enabled.
> ./.venv/bin/python ~/.vscode/extensions/ms-python.python-2023.6.1/pythonFiles/testing_tools/run_adapter.py discover pytest -- --rootdir . -s --cache-clear tests
cwd: .
[ERROR 2023-3-19 9:2:20.223]: Error discovering pytest tests:
 [n [Error]: ============================= test session starts ==============================
platform darwin -- Python 3.8.13, pytest-7.2.1, pluggy-0.13.1
rootdir: /Users/armenzg/code/sentry, configfile: pyproject.toml
plugins: forked-1.4.0, fail-slow-0.3.0, rerunfailures-11.0, sentry-0.1.11, xdist-3.0.2, typeguard-2.13.3, cov-4.0.0, django-4.4.0

###################
#
# Frontend assets last built 3562 seconds ago, skipping rebuilds for another 38 seconds.
# Delete the file: `.webpack.meta` to rebuild.
#
###################

collected 17246 items / 4 errors

##### SKIPPING A MILLION LINES HERE

=========================== short test summary info ============================
ERROR tests/sentry/celery/test_app.py - NameError: name 'logging' is not defined
ERROR tests/sentry/issues/test_escalating.py - NameError: name 'logging' is n...
ERROR tests/sentry/issues/test_escalating_issues_alg.py - NameError: name 'lo...
ERROR tests/sentry/tasks/test_weekly_escalating_forecast.py - NameError: name...
!!!!!!!!!!!!!!!!!!! Interrupted: 4 errors during collection !!!!!!!!!!!!!!!!!!!!
================== 17246 tests collected, 4 errors in 18.80s ===================

Traceback (most recent call last):
  File "/Users/armenzg/.vscode/extensions/ms-python.python-2023.6.1/pythonFiles/testing_tools/run_adapter.py", line 22, in <module>
    main(tool, cmd, subargs, toolargs)
  File "/Users/armenzg/.vscode/extensions/ms-python.python-2023.6.1/pythonFiles/testing_tools/adapter/__main__.py", line 99, in main
    parents, result = run(toolargs, **subargs)
  File "/Users/armenzg/.vscode/extensions/ms-python.python-2023.6.1/pythonFiles/testing_tools/adapter/pytest/_discovery.py", line 47, in discover
    raise Exception("pytest discovery failed (exit code {})".format(ec))
Exception: pytest discovery failed (exit code 2)

	at ChildProcess.<anonymous> (/Users/armenzg/.vscode/extensions/ms-python.python-2023.6.1/out/client/extension.js:2:238829)
	at Object.onceWrapper (node:events:646:26)
	at ChildProcess.emit (node:events:526:28)
	at maybeClose (node:internal/child_process:1092:16)
	at Socket.<anonymous> (node:internal/child_process:451:11)
	at Socket.emit (node:events:526:28)
	at Pipe.<anonymous> (node:net:687:12)]

User Settings


languageServer: "Pylance"

linting
• flake8Enabled: true
• flake8Path: "<placeholder>"
• pycodestylePath: "<placeholder>"

formatting
• provider: "black"
• blackPath: "<placeholder>"

testing
• pytestArgs: "<placeholder>"
• pytestEnabled: true
• pytestPath: "<placeholder>"

Extension version: 2023.6.1
VS Code version: Code 1.77.3 (Universal) (704ed70d4fd1c6bd6342c436f1ede30d1cff4710, 2023-04-12T09:19:37.325Z)
OS version: Darwin arm64 22.3.0
Modes:
Sandboxed: No

System Info
Item Value
CPUs Apple M1 (8 x 24)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_renderer: enabled_on
video_decode: enabled
video_encode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
webgpu: disabled_off
Load (avg) 3, 4, 3
Memory (System) 16.00GB (0.03GB free)
Process Argv --crash-reporter-id 8fcb9b29-2bdc-4a61-9b8c-1be28aead4fa
Screen Reader no
VM 0%
A/B Experiments
vsliv368cf:30146710
vsreu685:30147344
python383:30185418
vspor879:30202332
vspor708:30202333
vspor363:30204092
vslsvsres303:30308271
vserr242cf:30382550
pythontb:30283811
vsjup518:30340749
pythonptprofiler:30281270
vshan820:30294714
vstes263:30335439
vscorecescf:30445987
pythondataviewer:30285071
vscod805cf:30301675
binariesv615:30325510
bridge0708:30335490
bridge0723:30353136
cmake_vspar411:30581797
vsaa593:30376534
pythonvs932:30410667
cppdebug:30492333
vscaat:30438848
vsclangdc:30486549
c4g48928:30535728
dsvsc012:30540252
pynewext54:30695312
azure-dev_surveyone:30548225
vscccc:30610679
nodejswelcome1cf:30587006
282f8724:30602487
pyind779:30671433
f6dab269:30613381
pythonsymbol12:30671437
azdwalk:30687957
pythonms35:30701012
pythonfmttextcf:30716743

@karthiknadig
Copy link
Member

@armenzg Can you share the logs from Output > Python Language Server ? Code Actions come from language server.

@armenzg
Copy link
Author

armenzg commented Apr 19, 2023

Hi @karthiknadig thanks for the prompt response

Here's the output:

[Info  - 8:56:23 AM] (44971) Pylance language server 2023.4.20 (pyright 64a33975) starting
[Info  - 8:56:23 AM] (44971) Server root directory: /Users/armenzg/.vscode/extensions/ms-python.vscode-pylance-2023.4.20/dist
[Info  - 8:56:23 AM] (44971) Starting service instance "sentry"
[Info  - 8:56:23 AM] (44971) Auto-indent enabled
[Info  - 8:56:23 AM] (44971) Setting pythonPath for service "sentry": "/Users/armenzg/code/sentry/.venv/bin/python"
[Info  - 8:56:23 AM] (44971) Loading pyproject.toml file at /Users/armenzg/code/sentry/pyproject.toml
[Error - 8:56:23 AM] (44971) Pyproject file "/Users/armenzg/code/sentry/pyproject.toml" is missing "[tool.pyright]" section.
[Warn  - 8:56:23 AM] (44971) stubPath /Users/armenzg/code/sentry/typings is not a valid directory.
[Info  - 8:56:23 AM] (44971) Assuming Python version 3.8
[Info  - 8:56:24 AM] (44971) Found 4537 source files
[Info  - 8:56:24 AM] (44971) Background analysis(1) root directory: /Users/armenzg/.vscode/extensions/ms-python.vscode-pylance-2023.4.20/dist
[Info  - 8:56:24 AM] (44971) Background analysis(1) started
[Info  - 8:56:25 AM] (44971) Indexer background runner(2) root directory: /Users/armenzg/.vscode/extensions/ms-python.vscode-pylance-2023.4.20/dist (index)
[Info  - 8:56:25 AM] (44971) Indexing(2) started
[Info  - 8:56:26 AM] (44971) scanned(2) 241 files over 1 exec env
[Info  - 8:56:26 AM] (44971) [BG(1)] Long operation: getSemanticTokens full at /Users/armenzg/code/sentry/src/sentry/issues/escalating.py (2073ms)
[Warn  - 8:56:36 AM] (44971) Workspace indexing has hit its upper limit: 2000 files
[Info  - 8:56:36 AM] (44971) [IDX(2)] Long operation: index execution environment /Users/armenzg/code/sentry (3219ms)
[Info  - 8:56:36 AM] (44971) [IDX(2)] Long operation: index packages /Users/armenzg/code/sentry (3231ms)
[Info  - 8:56:36 AM] (44971) indexed(2) 169 files over 1 exec env
[Info  - 8:56:36 AM] (44971) Indexing finished(2).
[Info  - 9:15:34 AM] (44971) Found 4542 source files
[Info  - 9:32:42 AM] (44971) Found 4537 source files
[Info  - 9:32:46 AM] (44971) Found 4542 source files
[Info  - 12:19:14 PM] (44971) Found 4543 source files
[Info  - 12:19:18 PM] (44971) [BG(1)] Long operation: checking: /Users/armenzg/code/sentry/src/sentry/issues/escalating.py (2427ms)
[Info  - 12:19:18 PM] (44971) [BG(1)] Long operation: analyzing: /Users/armenzg/code/sentry/src/sentry/issues/escalating.py (2613ms)

@karthiknadig karthiknadig transferred this issue from microsoft/vscode-python Apr 19, 2023
@rchiodo
Copy link
Contributor

rchiodo commented Apr 20, 2023

Thanks for the issue.

The root cause of the problem is that it takes 3 seconds to analyze your file. That's a bit out of the ordinary. If we could get access to your source, we might be able to fix that problem.

Secondly, I'm not sure why VS code is asking for code actions on file save. We might be able to shortcut this problem. I'll look into if that's possible or not.

@armenzg
Copy link
Author

armenzg commented Apr 20, 2023

Hi @rchiodo This is the file I was editing.

The overall waiting is sometimes 10-30 seconds.

@rchiodo
Copy link
Contributor

rchiodo commented Apr 20, 2023

It seems there is a setting to control if code actions run on save or not. You must have this setting set:

  "editor.codeActionsOnSave": {
    "source.fixAll": true, <-- This one causes Pylance to have to analyze the file again before the save is finished.
    "source.organizeImports": true
  }

@armenzg
Copy link
Author

armenzg commented Apr 20, 2023

Okay. Let me add it to the user settings and report back.

@heejaechang
Copy link
Contributor

heejaechang commented Apr 20, 2023

@armenzg

image

if you click cancel, does it actually cancel codeActionsOnSave (https://code.visualstudio.com/updates/v1_23#_run-code-actions-on-save)? I am wondering whether cancellation is working as expected.

by the way, by default, we don't do anything on codeActionsOnSave, you must have set up something to run on save using python.analysis.fixAll

you can disable us participating in codeActionsOnSave using 2 ways. one is disabling codeActionsOnSave all together using editor.codeActionsOnSave. That will disable not just us but all extensions. the other one is disabling just us using python.analysis.fixAll (https://github.com/microsoft/pylance-release/blob/main/README.md#settings-and-customization)

@rchiodo
Copy link
Contributor

rchiodo commented Apr 20, 2023

I am reproing the issue using the source code provided. It's not quite as slow, but I can get the dialog to show up.

This setting has to be set for me to repro the problem though:

  "python.analysis.fixAll": [
    "source.unusedImports",
  ],

@rchiodo
Copy link
Contributor

rchiodo commented Apr 20, 2023

I believe we might be able to fix this problem by not cloning the service during a save. That seems to be what takes so long. It has to reparse everything every time you hit save.

At least that's what the log looks like when I repro (log output from just saving)
savelog.txt

@heejaechang
Copy link
Contributor

heejaechang commented Apr 20, 2023

that is only when you have 1 fix all in the setting. when there are multiple ones, you can't avoid cloning. we could optimize a case where there is only one fix all in the setting if we think that's worth it.

that said, the root cause of the issue, where executing one or multiple fix all taking long time is just nature of fix all. user needs to decide whether it is worth the perf hit. all fix all we have can be run manually through command palette fix all or code actions if user doesn't like to run it automatically when a file is saved.

any other extension's fix all will have same problem such as prettier or typescript's eslint fix all and etc.

@armenzg
Copy link
Author

armenzg commented Apr 21, 2023

Hi @ heejaechang
That's for your help. Find below my reply to your comment.

image

if you click cancel, does it actually cancel codeActionsOnSave (https://code.visualstudio.com/updates/v1_23#_run-code-actions-on-save)? I am wondering whether cancellation is working as expected.

I believe hitting the cancel button works. I will pay closer attention.

This is what I would notice:

  • I hit save
  • I switch to my terminal to run the tests
  • The tests unexpectedly fail since I just fixed them on VS Code
  • I switch back to VS Code and I notice that the prompt is there, thus, the code fixes have not been saved
  • I hit the cancel button and I go back to running the tests

I believe it's worked as mentioned above but I will check again.

by the way, by default, we don't do anything on codeActionsOnSave, you must have set up something to run on save using python.analysis.fixAll

I see this and this in our repo's workspace settings. Would that do it?

  "editor.codeActionsOnSave": {
    "source.fixAll.eslint": true
  },
  ...
  "[python]": {
    "editor.formatOnSave": true,
    "editor.codeActionsOnSave": {
      "source.organizeImports": true
    }
  },

you can disable us participating in codeActionsOnSave using 2 ways. one is disabling codeActionsOnSave all together using editor.codeActionsOnSave. That will disable not just us but all extensions. the other one is disabling just us using python.analysis.fixAll (https://github.com/microsoft/pylance-release/blob/main/README.md#settings-and-customization)

Like this? "python.analysis.fixAll": [], (No list of actions to be taken)

@armenzg
Copy link
Author

armenzg commented Apr 21, 2023

that is only when you have 1 fix all in the setting. when there are multiple ones, you can't avoid cloning. we could optimize a case where there is only one fix all in the setting if we think that's worth it.

that said, the root cause of the issue, where executing one or multiple fix all taking long time is just nature of fix all. user needs to decide whether it is worth the perf hit. all fix all we have can be run manually through command palette fix all or code actions if user doesn't like to run it automatically when a file is saved.

any other extension's fix all will have same problem such as prettier or typescript's eslint fix all and etc.

From a user's perspective, it's a bit hard to figure out what is the cause and what settings to use.
Unfortunately, this was impacting my development and I knew it would take a lot of time trying to figure it out.

At first, I let the issue go on for few weeks since it was intermittent.

Later on when I was doing heavier development I noticed it more frequently. I searched the web to determine what workarounds there could be. I was unaware of workspace settings having changed (assuming that's what started my problem) or if it was an updated version of VS code that started triggering this.

I tried resetting VS code's settings to discard the possibility of bad settings from using VS code for too many years. I removed all extensions and I started adding them back as needed. I then tried to disable the Jupyter extension (since that was the extension mentioned in the original prompt) and then the message will mention the Python extension, thus, deciding to file this issue.

It was also unclear what settings would be the right fix (e.g. I was not aware of python.analysis.fixAll and it was not suggested after I hit "Configure" in the prompt). This is all I would get:
image

@armenzg
Copy link
Author

armenzg commented Apr 21, 2023

For anyone following along.

Before this comment:
image

I had this setting from the workspace's setting:

  "editor.codeActionsOnSave": {
    "source.fixAll.eslint": true
  },
  ...
  "[python]": {
    "editor.formatOnSave": true,
    "editor.codeActionsOnSave": {
      "source.organizeImports": true
    }
  },

Yesterday I added the following as suggested and the issue still happened this morning:

  "editor.codeActionsOnSave": {
    "source.fixAll": true, <-- This one causes Pylance to have to analyze the file again before the save is finished.
    "source.organizeImports": true
  }

I now have:

  • Removed the "editor.codeActionsOnSave" section mentioned by @rchiodo
  • Kept the workspace settings
  • Added the following "python.analysis.fixAll": [],

@armenzg
Copy link
Author

armenzg commented Apr 21, 2023

Is this useful output?

2023-04-21 09:43:47.145 [info] UNRESPONSIVE extension host: starting to profile NOW
2023-04-21 09:43:53.265 [warning] UNRESPONSIVE extension host: 'ms-python.python' took 64.93929990255603% of 3248.832ms, saved PROFILE here: 'file:///var/folders/4p/5s4qn7950nx1j4_j5f748n5w0000gn/T/exthost-835837.cpuprofile'
2023-04-21 09:44:28.203 [error] Error: Aborted onWillSaveTextDocument-event after 1750ms
    at vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:1906:13638
2023-04-21 09:44:30.839 [info] UNRESPONSIVE extension host: starting to profile NOW
2023-04-21 09:44:34.059 [info] UNRESPONSIVE extension host: received responsive event and cancelling profiling session
2023-04-21 09:44:34.976 [warning] UNRESPONSIVE extension host: 'ms-python.python' took 70.24299447387308% of 2282.4ms, saved PROFILE here: 'file:///var/folders/4p/5s4qn7950nx1j4_j5f748n5w0000gn/T/exthost-3692a7.cpuprofile'
2023-04-21 09:45:06.633 [error] Error: Aborted onWillSaveTextDocument-event after 1750ms
    at vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/workbench/workbench.desktop.main.js:1906:13638
2023-04-21 09:45:08.759 [info] UNRESPONSIVE extension host: starting to profile NOW
2023-04-21 09:45:14.647 [warning] UNRESPONSIVE extension host: 'ms-python.python' took 69.13412892594997% of 3461.373ms, saved PROFILE here: 'file:///var/folders/4p/5s4qn7950nx1j4_j5f748n5w0000gn/T/exthost-b85259.cpuprofile'
2023-04-21 09:45:51.115 [info] UNRESPONSIVE extension host: starting to profile NOW
2023-04-21 09:45:55.910 [info] UNRESPONSIVE extension host: received responsive event and cancelling profiling session
2023-04-21 09:45:56.789 [warning] UNRESPONSIVE extension host: 'ms-python.python' took 69.23570817182755% of 3321.266ms, saved PROFILE here: 'file:///var/folders/4p/5s4qn7950nx1j4_j5f748n5w0000gn/T/exthost-8abcc6.cpuprofile'

@armenzg
Copy link
Author

armenzg commented Apr 21, 2023

I have verified that adding "python.analysis.fixAll": [], removes the problem.

Here's my config.

diff --git a/.vscode/settings.json b/.vscode/settings.json
index 4e6108da16..c7f254b2d3 100644
--- a/.vscode/settings.json
+++ b/.vscode/settings.json
@@ -59,9 +59,11 @@
   "[python]": {
     "editor.formatOnSave": true,
     "editor.codeActionsOnSave": {
-      "source.organizeImports": true
+      "source.organizeImports": true,
+      "source.fixAll": true
     }
   },
+  // "python.analysis.fixAll": [],

   "[html]": {
     "editor.formatOnSave": false```

@armenzg
Copy link
Author

armenzg commented Apr 21, 2023

I'm also upgrading to the M1 build rather than the Universal just in case there are some perf improvements.

Version: 1.77.3 (Universal)
Commit: 704ed70d4fd1c6bd6342c436f1ede30d1cff4710
Date: 2023-04-12T09:19:37.325Z
Electron: 19.1.11
Chromium: 102.0.5005.196
Node.js: 16.14.2
V8: 10.2.154.26-electron.0
OS: Darwin arm64 22.3.0
Sandboxed: No
Version: 1.77.3
Commit: 704ed70d4fd1c6bd6342c436f1ede30d1cff4710
Date: 2023-04-12T09:41:48.638Z
Electron: 19.1.11
Chromium: 102.0.5005.196
Node.js: 16.14.2
V8: 10.2.154.26-electron.0
OS: Darwin arm64 22.3.0
Sandboxed: No

@bschnurr
Copy link
Member

bschnurr commented Apr 21, 2023

Isn't the source of the problem 4542 source files and index hitting the limit of 2000?
from the logs

[Warn  - 8:56:36 AM] (44971) Workspace indexing has hit its upper limit: 2000 files
[Info  - 8:56:36 AM] (44971) [IDX(2)] Long operation: index execution environment /Users/armenzg/code/sentry (3219ms)
[Info  - 8:56:36 AM] (44971) [IDX(2)] Long operation: index packages /Users/armenzg/code/sentry (3231ms)
[Info  - 8:56:36 AM] (44971) indexed(2) 169 files over 1 exec env
[Info  - 8:56:36 AM] (44971) Indexing finished(2).
[Info  - 9:15:34 AM] (44971) Found 4542 source files

@rchiodo
Copy link
Contributor

rchiodo commented Apr 21, 2023

Isn't the source of the problem 4542 source files and index hitting the limit of 2000?

I feel like that's still our problem to solve. The real problem here is we reindex after every save because of the cloning of the service to handle the fix all command.

@armenzg
Copy link
Author

armenzg commented Apr 21, 2023

Yeah. I'm still hitting the issue.

Any other setting I should try?

@rchiodo
Copy link
Contributor

rchiodo commented Apr 21, 2023

I thought removing python.analysis.fixAll fixed the problem?

Otherwise, this would be expected to happen.

We do a bunch of work if python.analysis.fixAll set. The amount of work is really dependent upon the number of files in your workspace and environment.

We might be able to do less work, but there is not a way other than that setting to prevent the work from happening.

@armenzg
Copy link
Author

armenzg commented Apr 21, 2023

I thought so too.

Am I setting it correctly as this? "python.analysis.fixAll": []

It stills happens sporadically with this configuration:

diff --git a/.vscode/settings.json b/.vscode/settings.json
index 4e6108da16..d82e921adb 100644
--- a/.vscode/settings.json
+++ b/.vscode/settings.json
@@ -59,9 +59,11 @@
   "[python]": {
     "editor.formatOnSave": true,
     "editor.codeActionsOnSave": {
-      "source.organizeImports": true
+      "source.organizeImports": false,
+      "source.fixAll": false
     }
   },
+  "python.analysis.fixAll": [],

@heejaechang
Copy link
Contributor

heejaechang commented Apr 21, 2023

then, it might not even us. it could be some other extension running. turn off everything that is supposed to run automatically when save such as formatOnSave.

by the way, so I assume the cancellation didn't work? <= ignore this. I saw your reply up there.

@heejaechang
Copy link
Contributor

heejaechang commented Apr 21, 2023

hmm... we have this code

const workspace = (await this._ls.getWorkspaceForFile(filePath)) as PylanceWorkspace;
        if (!workspace || !workspace.fixAll || workspace.fixAll.length === 0) {
            // No fix all is configured.
            return;
        }

fixAll has value of python.analysis.fixAll, so unless there is a bug (I just checked and I couldn't see any issue), if there is no python.analysis.fixAll set, then it should be same as having python.analysis.fixAll: [] so not sure why having that setting affect anything.

The real problem here is we reindex after every save because of the cloning of the service to handle the fix all command.

cloning shouldn't affect re-indexing. that should be only affected by markDirty from actual file change itself. If it happens, then that sounds like a bug.

Warn - 8:56:36 AM] (44971) Workspace indexing has hit its upper limit: 2000 files

if we are hitting this from cloned service (hopefully, once we change how we use program, we just clone program rather than clone entire service to get cloned program), that sounds like a bug. the cloned service is supposed to be disabled all ide features including background thread.

@heejaechang
Copy link
Contributor

So, I just debugged our code and found that our cloneService doesn't create a bg, which means the indexer can't run for the cloned service since it only runs when there's a bg. Additionally, the cloned service is marked as not scan folder, so it can't be discovering files and taking a long time to do so.

That being said, I did find a bug. When we reach the 2000 file limit for the workspace, it appears that the limit only applies to how many files we process in one chunk. This means that if there are 4000 files, we will hit the 2000 file limit twice and end up processing all 4000 files. It seems like this occurs whenever a file is saved/edited because that's when the next 2000 chunk is processed.

In this particular case where there are 4xxx files, the process will run 3 times: 2 times for the 2000 files and once for the remaining files. After that, it will stop processing indices for workspace files.

I will fix this bug. However, it's worth noting that it shouldn't affect the fix all issue, as it should be unrelated. It's just that the repo is large enough to reveal another bug we weren't previously aware of.

@karthiknadig
Copy link
Member

@lobsterkatie You can look at the logs for Output > Black, it should give some clue on how long black actually took to format. The Black formatter extension does only one thing different from running black in terminal; it has the modules pre-loaded for formatting and does not have to spawn a new process to actually do formatting. The speed up comes from not having python run a separate process on each save.

That being said, black formatter extension does not contribute Code Actions, so if you are seeing a notification for this then please file a bug https://github.com/microsoft/vscode-black-formatter

@lobsterkatie
Copy link

@lobsterkatie those 2 are handled by microsoft/vscode-python . we only handle ones in python.analysis.fixall so you probably want to open an issue there for those 2 features.

Ah, right - I saw that the very similar issue Armen linked in the original issue description above was from the vscode-python repo, and failed to notice that this one was not! Apologies!

@lobsterkatie You can look at the logs for Output > Black, it should give some clue on how long black actually took to format. The Black formatter extension does only one thing different from running black in terminal; it has the modules pre-loaded for formatting and does not have to spawn a new process to actually do formatting. The speed up comes from not having python run a separate process on each save.

That being said, black formatter extension does not contribute Code Actions, so if you are seeing a notification for this then please file a bug microsoft/vscode-black-formatter

Thanks, that's helpful to know. And yeah, I do think that the popup I see mentioning the black formatter running is separate from the popup Armen mentioned about gathering code actions, but they come immediately in sequence when I save, and both contribute to the slowness. In any case, I can take that up in the vscode-python repo (now that I know that that's not here!).

Cheers!

@rchiodo
Copy link
Contributor

rchiodo commented Apr 26, 2023

@heejaechang and @erictraut I tried this with a deep copy of the program and the problem goes away in my repro. The save is almost instant then.

I'm going to look into have one source of truth for document parse trees.

@heejaechang
Copy link
Contributor

since plan is only sharing text and tree, but not binding info and type cache, once you did deep copy, you could remove all binding info using cleantree call and create new type eval and run fix all on it.

@rchiodo
Copy link
Contributor

rchiodo commented Apr 26, 2023

You mean don't create a 'DocumentManager' but instead keep it the same (with a deep copy) and then prune the deep copy?

@heejaechang
Copy link
Contributor

I meant just for prototyping.

@erictraut
Copy link
Contributor

@rchiodo, I propose we do this in stages. We can start with having one source of truth for the document text. That will address this bug. Then we can separate the bind information from the parse tree, which will allow us to treat the parse tree as immutable. Then we can share the parse trees without the need to do any deep cloning. Does that sound like a reasonable plan?

@rchiodo
Copy link
Contributor

rchiodo commented Apr 26, 2023

@rchiodo, I propose we do this in stages. We can start with having one source of truth for the document text. That will address this bug. Then we can separate the bind information from the parse tree, which will allow us to treat the parse tree as immutable. Then we can share the parse trees without the need to do any deep cloning. Does that sound like a reasonable plan?

Sounds good to me. I'm still verifying using Heejae's idea at the moment, but after that I'll create some sort of 'DocumentManager' that is the place to get SourceFiles? or maybe just parse results.

@rchiodo
Copy link
Contributor

rchiodo commented Apr 27, 2023

Cloning and rebinding is about the same time as the old algorithm (so parsing is not the slow part here).

Old code:

  • 2200 ms or so (average)

Clone with clear all bindings

  • 2100 ms.

Clone service, but keep same parse tree with same bindings (don't rebind or copy anything)

  • 800 ms.

Copying the different parse trees and rebinding takes just as long as creating them from scratch. So this will only really get us a gain if we bind less. (Difference between the old code and the clone is parsing, so parsing is only like 100ms)

Not sure it would be worth it to just share the parse information then.

What about a copy on write scheme were we only rebind when the edit is applied (just like when typing), but instead of rebinding the current source file, we copy it to another source file and bind that one?

@rchiodo
Copy link
Contributor

rchiodo commented Apr 27, 2023

Oh I guess the clone could be taking a portion of the time too. I didn't factor that time out.

@erictraut
Copy link
Contributor

Are you doing a shallow or deep clone of the parse tree? A shallow clone should be almost free. A deep clone won't be free, but my guess is that it's negligible.

I typically find that the time it takes to parse and bind files is similar. That is, if parsing all of the files in your project takes 1000ms, the total bind time will be roughly the same. The file load time varies greatly depending on whether it's in the OS's file system cache (and if not, how fast the storage device is).

Here's a typical breakdown.

image

If I understand your results correctly, you're saying that eliminating the file reads (which you get for free with the change I made yesterday) and eliminating parsing (which you get by cloning and cleaning the parse tree) drops the time by only 100ms (from 2200 to 2100ms), but eliminating the binding drops it by 1300ms (2100 to 800ms). That's not what I'd expect to see, so I'm somewhat suspicious of these results.

What about import resolution? Are you cloning that information as well, or are you recalculating that? Import resolution can be quite costly for some projects. I'm guessing that you're cloning it because the "clean parse tree" call doesn't clear it.

@rchiodo
Copy link
Contributor

rchiodo commented Apr 27, 2023

Yeah computing the deep clone only takes 10ms on average. It's negligible. (Just tracked it now).

Where is the import resolution data stored? I'm creating a new backgroundAnalysisProgram, a new program, sourceFileInfos are shallow copies, sourceFiles are deep copies with the parse tree recreated by deep coping it.

If it's on the sourceFileInfo, then it should save the import resolution information.

@rchiodo
Copy link
Contributor

rchiodo commented Apr 27, 2023

Perhaps it's the checker? There's no background thread for this clone of the service, so the checker might be taking CPU cycles?

@erictraut
Copy link
Contributor

The import info is stored in the parse tree (on nodes the correspond to import statements). Refer to the comments in analyzerNodeInfo.ts for more details.

The checker shouldn't be running in this use case, but the type evaluator will be invoked if you're requesting semantic information (types) for symbols in the file. As you know, the type evaluator will recursively evaluate types as needed to provide the requested answer. If the information it needs is not cached, it will need to evaluate it from scratch.

@rchiodo
Copy link
Contributor

rchiodo commented Apr 27, 2023

I think I need to do more analysis to determine which part is taking so long. It's too broad at the moment.

@heejaechang
Copy link
Contributor

we have a way to see how much time we spent on parsing, and binding accumulated. you can use that to see how much time is spent.

@heejaechang
Copy link
Contributor

sent PR that can track perf - https://github.com/microsoft/pyrx/pull/3443

@PylanceBot PylanceBot added the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label May 5, 2023
@rchiodo
Copy link
Contributor

rchiodo commented May 10, 2023

This issue has been fixed in prerelease version 2023.5.21, which we've just released. You can find the changelog here: CHANGELOG.md

@rchiodo rchiodo closed this as completed May 10, 2023
@armenzg
Copy link
Author

armenzg commented May 12, 2023

Thank you so much @rchiodo! I will update to the prerelease!

@flying-sheep
Copy link

flying-sheep commented Jul 6, 2023

Still unbearably slow in my code base: https://github.com/scverse/scanpy

Saving files takes multiple seconds. VS Code waits and eventually shows a popup “Getting code actions from ''Python"”.

When setting "python.languageServer": "None", saving works as expected.

Why will VS Code wait for Pylance to save things? I have only source.fixAll.ruff activated, so the Python extension shouldn’t be consulted when saving a file at all.

{
    "[python]": {
        "editor.formatOnSave": true,
        "editor.defaultFormatter": "ms-python.black-formatter",
        "editor.codeActionsOnSave": {
            "source.fixAll.ruff": true,
        },
    },
    "python.testing.pytestArgs": ["scanpy/tests"],
    "python.testing.unittestEnabled": false,
    "python.testing.pytestEnabled": true,
    "python.languageServer": "Pylance",
}

@rchiodo
Copy link
Contributor

rchiodo commented Jul 6, 2023

@flying-sheep, I'm opening a new issue for your problem. It might be the black formatter that is also taking a long time as you're formatting on every save.

@flying-sheep
Copy link

flying-sheep commented Jul 6, 2023

As said: it’s not black.

When setting "python.languageServer": "None", saving works as expected.

I’ll follow up there, thank you!

@flying-sheep
Copy link

flying-sheep commented Aug 8, 2023

It’s faster now, but there’s still a perceptible delay.

Thinking about this, it’s actually not relevant how slow or fast Pyright (parsing & analysis) is.

Pylance (the VS Code addon) should not delay saving a document for a single nanosecond if I don’t have VS Code configured to run “on save” actions from Pylance. Any kind of parsing and analysis should happen asynchronously without delaying the file being saved.

Could you please reopen this until Pylance doesn’t delay saving anymore? The issue described in the title here is not fixed:

Saving .py files delayed by Getting code actions from ''Python'

@rchiodo
Copy link
Contributor

rchiodo commented Aug 8, 2023

Pylance (the VS Code addon) should not delay saving a document for a single nanosecond if I don’t have VS Code configured to run “on save” actions from Pylance. Any kind of parsing and analysis should happen asynchronously without delaying the file being saved.

This request would be a VS code ask. They still ask us for code actions if there's anything indicated for codeActionsOnSave. This happens when a file is saved. The fix we made was to not run analysis at that point but we still get queried.

@flying-sheep
Copy link

OK, thank you for clarifying! So you did stop running analysis synchronously before returning the save actions, that’s great!

@rchiodo rchiodo mentioned this issue May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version P2
Projects
None yet
Development

No branches or pull requests

10 participants