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

Support background analysis #13

Open
zeronone opened this issue Aug 8, 2020 · 18 comments
Open

Support background analysis #13

zeronone opened this issue Aug 8, 2020 · 18 comments

Comments

@zeronone
Copy link
Collaborator

zeronone commented Aug 8, 2020

@dam5h
Copy link

dam5h commented Oct 25, 2020

I am having a performance issue with pyright on a particular file that has a long analysis time (over 4 secs), which can freeze emacs. By this I mean that I can type characters and they actually don't appear on the screen until the analysis is completed. Sounds like having analysis cancellation may solve this issue for me.

@seagle0128
Copy link
Collaborator

seagle0128 commented Oct 26, 2020

@dam5h
Copy link

dam5h commented Oct 26, 2020

This is interesting, if I am seeing [FG] in my lsp-log does that mean that background is not enabled? I assumed this meant Foreground, but that was just an assumption. I definitely experience emacs freezing up during these long analysis times. The link you posted is for pyright, and the issue is marked as closed, what does this mean regarding whether or not lsp-pyright can support background analysis with cancelations? Thanks!

[FG] Long operation: checking: tests/conftest.py (4206ms)
[FG] Long operation: analyzing: tests/conftest.py (4266ms)
[FG] Long operation: checking: tests/conftest.py (3960ms)
[FG] Long operation: analyzing: tests/conftest.py (3997ms)
[FG] Long operation: checking: tests/conftest.py (4775ms)
[FG] Long operation: analyzing: tests/conftest.py (4883ms)
[FG] Long operation: checking: tests/conftest.py (3994ms)
[FG] Long operation: analyzing: tests/conftest.py (3994ms)
[FG] Long operation: checking: tests/conftest.py (3665ms)

@zeronone
Copy link
Collaborator Author

zeronone commented Oct 27, 2020

@dam5h Can you check the performance difference with VSCode pyright/pylance?

what does this mean regarding whether or not lsp-pyright can support background analysis with cancelations?

I think not supported yet, @seagle0128 any specific reasons for closing?

@dam5h
Copy link

dam5h commented Oct 27, 2020

@zeronone , I don't actually have VS Code installed, but I had an exchange with on Gitter with @msfterictraut (main pyright developer) and he made the point that even if the performance is bad on a particular file's analysis, it won't impact the responsiveness of the editor if it supports cancellation. I can handle a long analysis time much better if I can at least continue typing while waiting, so it's more the editor hanging up rather than analysis performance that I am concerned with here.

https://gitter.im/microsoft-pyright/community?at=5f95abac7be0d67d279778e1

@seagle0128 seagle0128 reopened this Oct 27, 2020
@seagle0128
Copy link
Collaborator

@zeronone sorry, closed it by mistake somehow. Reopen it now.

@seagle0128
Copy link
Collaborator

@dam5h @zeronone so do you know how to send cancellation messages to server?

@braineo
Copy link

braineo commented Nov 1, 2020

Looks like createBackgroundAnalysis is only called here?

https://github.com/microsoft/pyright/blob/da39a2a78a442d2a6b29d664b32475c361c853f1/packages/pyright-internal/src/commands/commandController.ts#L39

            case Commands.createTypeStub: {
                return this._createStub.execute(cmdParams, token);
            }

So seems with a createTypeStub and non-empty callingFile, server will run a background analysis

    async execute(cmdParams: ExecuteCommandParams, token: CancellationToken): Promise<any> {
        if (cmdParams.arguments && cmdParams.arguments.length >= 2) {
            const service = await this._createTypeStubService(callingFile);

@zeronone
Copy link
Collaborator Author

zeronone commented Nov 3, 2020

Just to share my findings, if I understood it correctly, VSCode by default sends an additional argument to pyright that enables some sort of file-based background job cancellation strategy (probably when the file is changed, it cancels the jobs for that file). one, two

In our case we don't send any such argument, so the getCancellationFolderName() yields false. three

However looking at the code, it seems to also support message-based cancellation strategy, however it can't be used due to the additional check in createBackgroundAnalysis function four. I don't know why it is not used. Perhaps we can use the message strategy. I think this issue should be discussed upstream.

@seagle0128
Copy link
Collaborator

seagle0128 commented Nov 3, 2020

I think option 1 is the simple and reasonable method. Do you know how to send the addtional arg in elisp?

@erictraut
Copy link

Message-based cancellation won't work because this language server is written in TypeScript (javascript) and doesn't run on a separate thread from message handling. If you send a cancellation via message, the message won't be received or processed until after the operation is complete. A file-based cancellation mechanism works because changes can be polled from the inner loop of a long analysis operation.

@zeronone
Copy link
Collaborator Author

zeronone commented Nov 4, 2020

@erictraut Thanks a lot for your input, and the great tool. I guess then we can't go with the message-based strategy. Can you point to the documentation for the file-based cancellation strategy? I couldn't find any :( I assume we should create a directory where each file acts as token for a background job; and if the file is deleted the background job is also cancelled. Is it correct? If correct, I am wondering when the files should be created/deleted and what are the contents of the files.

@seagle0128 Sending the additional argument should be easy, but we also need to support the cancellation strategy. I don't know if lsp-mode has such feature yet.

@braineo
Copy link

braineo commented Nov 4, 2020

So we have two things going on here

  1. Run analysis in background
  2. Cancellation strategy

for cancellation, the pyright process needs another argument as discussed. and vscode extension code is here. Maybe need to mimic the behavior as

  1. when vsc extension is activated, cancellationStrategy is instantiated

https://github.com/microsoft/pyright/blob/7bc2bd8ab05fc4967120d1b6b2a0a1834da2b783/packages/vscode-pyright/src/extension.ts#L64

cancellationStrategy = new FileBasedCancellationStrategy();
  1. it pick up a random name, https://github.com/microsoft/pyright/blob/7bc2bd8ab05fc4967120d1b6b2a0a1834da2b783/packages/vscode-pyright/src/cancellationUtils.ts#L77
export class FileBasedCancellationStrategy implements CancellationStrategy, Disposable {
    constructor() {
        const folderName = randomBytes(21).toString('hex');
        this._sender = new FileCancellationSenderStrategy(folderName);
    }
  1. which is an argument sent to construct the language server process
    getCommandLineArguments(): string[] {
        return [`--cancellationReceive=file:${this._sender.folderName}`];
    }

and this sender sends cancellation by simply writing empty string to this file with name from getCancellationFilePath, typically takes a cancellation id. Assuming we already know from response message format, we know which request to cancel.

class FileCancellationSenderStrategy implements CancellationSenderStrategy {
    sendCancellation(_: MessageConnection, id: CancellationId): void {
        const file = getCancellationFilePath(this.folderName, id);
        tryRun(() => fs.writeFileSync(file, '', { flag: 'w' }));
    }

@koustuvsinha
Copy link

+1 this feature would be very useful addition to lsp-pyright. As of now, in my MBP the CPU usage is very high with this, and lsp-log shows [FG] Long operation: checking <file .py> (~4000ms)

@tshu-w
Copy link

tshu-w commented May 26, 2021

ping for this feature, I keep having this problem so much that I can't use it.

[FG] Long operation: checking: file.py (9989ms)
[FG] Long operation: analyzing: file.py (9989ms)

@tshu-w
Copy link

tshu-w commented Jun 9, 2021

I found that turning off useLibraryCodeForTypes and manually generating stubs files by pyright --createstub packagename avoids this problem.
microsoft/pyright#1970

@falcomomo
Copy link

I'm getting this a lot - 4s for the long analysis operations.
However, I'm not sure if it's caused by some of my own project imports, or due to some other module imports.

Did anyone find a workaround for this, or did anyone manage to get an implementation for the file based cancellation?

@tshu-w
Copy link

tshu-w commented Sep 27, 2021

I think it should be reported to upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants