monaco-languageclient icon indicating copy to clipboard operation
monaco-languageclient copied to clipboard

Any way to throttle or debounce calls to language server?

Open valstu opened this issue 2 years ago • 10 comments

I'm using clangd as language server with websocket to jsonrpc forwarding. My use case is quite specific and it would be ok in this case if the calls to language server wouldn't happen on each keypress. Is there any way to throttle or debounce calls to language server?

valstu avatar Dec 16 '21 14:12 valstu

There is many calls to language server after each keystroke (didContentChange, semanticTokens, codeActions...)

Why do you want to prevent the client from sending too many requests? what is your specific use case?

CGNonofr avatar Dec 16 '21 19:12 CGNonofr

Yeah, I would prefer if the client would make the calls after user stopped typing. I'm building web based ide and multiple clients will connect to same language server. Idea is that by throttling the calls we could reduce the server load.

valstu avatar Dec 16 '21 21:12 valstu

I dont think there is anything you can do to change the default behavior (except some hack or fork of monaco-editor?).

What you can do is disabling feature that send requests after each key stroke.

Another solution I can think of is to "intercept" requests and respond with an error while the user is typing

You'll still have the didContentChange notification sent though.

CGNonofr avatar Dec 16 '21 23:12 CGNonofr

Another solution I can think of is to "intercept" requests and respond with an error while the user is typing

If you change the implementation of monaco-languages.ts by injecting debounce detection in the providers and returning null or [] or whatever you could achieve what you want I think.

You'll still have the didContentChange notification sent though.

Yeah you definitely still need this. You could try to merge the changes and send one big event to the server I guess.

rcjsuen avatar Dec 17 '21 02:12 rcjsuen

I just came here to see if there was a configuration parameter for debounce and was pleasantly surprised to see it was the most recent issue.

What appears to be happening (and I suspect similar for everyone else) is that the language client is asking for a completion before the textDocument/didChange notifications are completed.

image

Since these are send without an id, it appears that there isn't a way to know when they are finished processing, so even with the suggested fix below I'm not sure it would help.

The language server spec shows the option of being able to replace the entire contents of the file instead of ranges: https://microsoft.github.io/language-server-protocol/specifications/specification-3-17/#textDocumentContentChangeEvent

However I don't think we need to do this. We already have a contentChanges array we could make a private variable on the class so it can accumulate the changes between debounces: https://github.com/TypeFox/monaco-languageclient/blob/2065d485d59209e7118e2e55ea1208b8a2148037/client/src/monaco-workspace.ts#L46-L67

I'm not great at javascript but my thoughts are something to the effect of

const documentChanges = new Map<string, any>();
function debounce(func, timeout = 300) {
  let timer;
  return (...args) => {
    clearTimeout(timer);
    timer = setTimeout(() => { func.apply(this, args); }, timeout);
  };
}
protected sendChanges() {
  documentChanges.forEach((change) => 
  this.onDidChangeTextDocumentEmitter.fire(change)
  );
}
protected onDidChangeContent(uri: string, model: monaco.editor.IModel, event: monaco.editor.IModelContentChangedEvent) {
  const textDocument = this.setModel(uri, model);
  let existingChanges = this.documentChanges.get(uri);
  let contentChanges = [];
  if (existingChanges !== undefined)
    contentChanges = existingChanges.contentChanges;

  const contentChanges = [];
  for (const change of event.changes) {
    const range = this.m2p.asRange(change.range);
    const rangeLength = change.rangeLength;
    const text = change.text;
    contentChanges.push({ range, rangeLength, text });
  }


  this.documentChanges.set(uri, {
    textDocument,
    contentChanges
  });
  processChanges();
}

const processChanges = debounce(() => sendChanges());

dkattan avatar Jan 12 '22 12:01 dkattan

What appears to be happening (and I suspect similar for everyone else) is that the language client is asking for a completion before the textDocument/didChange notifications are completed.

@dkattan What language server are you trying to integrate with?

Since Visual Studio Code itself is so tightly connected with Monaco, I wonder if the same -32801 errors are being sent back to the VS Code extension for the same language server. 🤔

rcjsuen avatar Jan 12 '22 12:01 rcjsuen

I have had success creating a custom MessageWriter (which is passed into the MessageConnection). It's not really in a shareable state at the moment (but has been in heavy use for many years without issue), but I can outline the general idea...

When a textDocument/didChange event is given to the message writer, debounce or throttle the processing of it taking the following into consideration:

  1. Is the change from the same document
  2. Is the change on the same line
  3. Is the change in a direct sibling of the last change

If all the above are true, merge the new event into the old event, and effectively discard the new event

Once the throttle or debounce time clears, send the merged event.

If the event is not a textDocument/didChange, queue it up and send it after any debounced textDocument/didChange events you may have to keep the order the same.

richardmward avatar Jan 12 '22 15:01 richardmward

What appears to be happening (and I suspect similar for everyone else) is that the language client is asking for a completion before the textDocument/didChange notifications are completed.

@dkattan What language server are you trying to integrate with?

Since Visual Studio Code itself is so tightly connected with Monaco, I wonder if the same -32801 errors are being sent back to the VS Code extension for the same language server. 🤔

I'm using the PowerShell language server in PowerShell Editor Services. This could very well be PSES not keeping up with the changes. It isn't causing any usability problems except for console logs in the browser that the user doesn't see.

dkattan avatar Jan 12 '22 21:01 dkattan

@richardmward

This is actually really interesting, could you share your implementation? How does the merging of events actually works?

john-schmitz avatar Mar 24 '22 18:03 john-schmitz

What I have done (right or wrong!) is:

When creating your message connectioncreateMessageConnection(reader, writer) pass in a custom writer so that we have access to the write(message) method. On each call to that i pass it into a throttle class, copied below.

DISCLAIMER: This was written many years ago, and I'm neither an expert in JS or language servers. It's just something I pieced together with a lot of trial and error. It is used on a regular basis and doesn't seem to cause a lot of issues, but I provide it purely as a guide to working out for youselves!

export default class WebsocketMessageWriterThrottle {
  constructor(sendMessage, throttle, debounce) {
    this.sendMessage = sendMessage;
    this.throttle = throttle;
    this.debounce = debounce;

    this.pendingDidChange = null;
    this.queuedMessages = [];
    this.lastMessageMethod = null;
    this.lastMessageTime = null;
    this.didChangeTimeout = null;
  }

  processMessage(message) {
    // We process 'did change' methods differently to try to group some of the changes together.
    if (message.method === "textDocument/didChange") {
      this._processDidChange(message);
    } else {
      // If there is a pending did change, queue this up first to keep the message order correct.
      if (this.pendingDidChange) {
        clearTimeout(this.didChangeTimeout);
        this._queueMessage(this.pendingDidChange);
        this.pendingDidChange = null;
      }
      this._queueMessage(message);
    }
  }

  _queueMessage(message) {
    this.queuedMessages.push(message);
    if (this.queuedMessages.length === 1) {
      this._sendQueuedMessage();
    }
  }

  _sendQueuedMessage() {
    const now = Date.now();
    const throttle = this.lastMessageMethod === "textDocument/didChange" ? this.throttle : 0;

    // If it is the first message, or the last message was sent long enough ago, send straight away.
    if (this.lastMessageTime == null || this.lastMessageTime + throttle < now) {
      const message = this.queuedMessages.shift();
      this.sendMessage(message);
      this.lastMessageTime = now;
      this.lastMessageMethod = message.method;
      if (this.queuedMessages.length > 0) {
        this._sendQueuedMessage();
      }
    } else {
      // If there was a message within the throttle window, queue it up again after that time has passed.
      setTimeout(() => {
        this._sendQueuedMessage();
      }, now - this.lastMessageTime + throttle);
    }
  }

  _processDidChange(message) {
    clearTimeout(this.didChangeTimeout);

    if (this.pendingDidChange) {
      // Attempt to merge the updates.
      const updateable = this._updatePendingDidChange(message);

      // If the pending change isn't updateable, queue the existing message and start a new pending change.
      if (!updateable) {
        this._queueMessage(this.pendingDidChange);
        this.pendingDidChange = message;
      }
    } else {
      // If there isn't a pending change, set it as the new message
      this.pendingDidChange = message;
    }

    // Debounce queuing the pending changes
    this.didChangeTimeout = setTimeout(() => {
      this._queueMessage(this.pendingDidChange);
      this.pendingDidChange = null;
    }, this.debounce);
  }

  _updatePendingDidChange(message) {
    // If it isn't the same document, then we can't update it.
    if (message.params.textDocument.uri !== this.pendingDidChange.params.textDocument.uri) {
      return false;
    }

    // It'll get very confusing trying to handle arrays of changes in either side, so we'll only consider this approach
    // if there is exactly one on each side, which is what usually happens.
    if (message.params.contentChanges.length !== 1 || this.pendingDidChange.params.contentChanges.length !== 1) {
      return false;
    }

    const lastChange = this.pendingDidChange.params.contentChanges[0];
    const currentChange = message.params.contentChanges[0];

    // If the change is not on the same line, we won't update it
    if (lastChange.range.start.line !== currentChange.range.start.line) {
      return false;
    }

    // Work out where we expect the cursor to be
    let cursorOffset = lastChange.text.length;
    if (currentChange.text.length === 0) {
      cursorOffset = lastChange.text.length - currentChange.rangeLength;
    }

    // If the cursor isn't where we expect it to be one the line, we won't update it
    if (lastChange.range.start.character + cursorOffset !== currentChange.range.start.character) {
      return false;
    }

    // If the text length is zero, it's a delete.
    if (currentChange.text.length === 0) {
      // If the old text length is zero, we are deleting old content so we need to alter the last change start position.
      if (lastChange.text.length === 0) {
        lastChange.range.start.character -= currentChange.rangeLength;
        lastChange.rangeLength += currentChange.rangeLength;
      } else {
        // Otherwise we are deleting newly typed content, so just change the replacement text.
        lastChange.text = lastChange.text.substr(0, lastChange.text.length - currentChange.rangeLength || 0);
      }
    } else {
      // If adding new text, simply append it. No need to change the ranges.
      lastChange.text += currentChange.text;
    }

    return true;
  }
}

richardmward avatar Mar 31 '22 14:03 richardmward

I am converting this into a discussion. It is not an issue, but should not be closed IMO.

kaisalmen avatar Jan 06 '23 11:01 kaisalmen