Slows down the editor if too many errors

[aminya]

When there are too many errors in an editor, the linter slows down the editor. The scrolling gets sluggish, and entering text gets very slow.

This happens when you open a file that has to have lf line-ending on a Windows machine.

Reproduce steps on Windows:

1. git clone https://github.com/iyonaga/minimap-lens.git
2. open the project in atom with eslint plugin
3. open lib/minimap-lens.js and try to scroll or edit something. Because of line ending differences, the editor gets very slow.

[aminya]

I found out the root of this problem. It might be related to linter rather linter-UI.

We need to skip rendering/processing the new messages that are the same as those that are already cached.

I wrote this function which filters the messages based on the given propertyNames. We can be sure that two messages are identical if these properties: ['description', 'excerpt', 'severity', 'linterName', 'location'] are the same. Their keys may be different but they are the same thing.

outdated

// This fast function returns the new messages by comparing them against the cache.
// This prevents re-rendering the already rendered messages
export function newMessages(input: Array<Message>, cache: Array<Message>, propertyNames: Array<string>) {
  // inputs check
  if (
    input === undefined ||
    cache === undefined ||
    input.length === undefined ||
    cache.length === undefined
  ) {
    return null
  }
  // comparison
  const areSame = Array(input.length).fill(true)
  for (let i = 0, len = input.length; i < len; i++) {
    for (const propertyName of propertyNames) {
      if (input[i][propertyName] !== cache[i][propertyName]) {
        areSame[i] = false
		break
      }
    }
  }
  return input.filter((m, i) => !areSame[i])
}

For example, in LinterUI.render function, the following will remove the repeated messages.

linter-ui-default/lib/main.js

Line 75 in f663a1f

render(difference: MessagesPatch) {

    if (this.messages) {
      difference.added = newMessages(difference.added, this.messagesCache, ['description', 'excerpt', 'severity', 'linterName', 'location'])
      if (difference.added  == null) {
        return
      }
    }

@steelbrain Is this already implemented in Linter? or it just does not work properly?

Edit: The issue is in the linter that does not skip passing the identical messages to linterUI

[aminya]

We should transfer this issue to linter repository. Could you do that? @steelbrain

[steelbrain]

@aminya Linter and Linter UI APIs already work based on diffs, if no lint messages change, nothing is re-rendered. As for the slow behavior when there are a lot of errors, I've been thinking, I don't think it's a good idea to have a 100k errors in a linter provider. The Atom editor cannot handle too many decorations and any change to the buffer would be slow as it'll have to recalculate all the decorations and their markers. And this goes beyond just linter, if you have that many messages, you're better off tweaking your lint config to adapt to what you have and slowly make your way to sanity. CLIs are better suited for these tasks than GUIs. So not supporting that many messages is intentional.

I'm thinking of enforcing a hard-limit on how many messages a lint-provider can provide, it'll be set at 5k (negotiable), so it doesn't break any existing sane-implementations and doesn't make the users suffer if the provider doesn't cap it themselves.

Edit: Another way to think about this: If you have incorrect line-endings in a file, any errors beyond the first few are useless, the user gets it, they have bad line endings. Drowning them with one error per line will only make the user agitated.

[aminya]

@steelbrain

My problem is that the linter passes repeated messages and added every time I save an editor. Even though they are already rendered. You can observe this by:

linter-ui-default/lib/main.js

Line 75 in f663a1f

render(difference: MessagesPatch) {

  render(difference: MessagesPatch) {
	console.log(difference.added)
	console.log(difference.messages)

Open an editor and type a simple space, and see that the passed difference is the same as previous ones. Linter does not skip passing already rendered messages.

Another possibility is that maybe the messageKey function in the linter is too conservative in making keys for messages, and makes new keys even when everything for the messages are the same.

P.S:

• the hard-limit can be useful in rare cases.
• about line-ending specifically, I think it should be fixed in linter-eslint. Unless there is a way for Linter to detect line-ending errors.