1819269 - disableEmptyLines could be faster

Status: RESOLVED FIXED

(DevTools :: Debugger, task)

Description

[Nicolas Chevobbe [:nchevobbe]]

The function was showing up in a profile (not in big ways, but looks like it could be improved)

https://searchfox.org/mozilla-central/rev/aa3ccd258b64abfd4c5ce56c1f512bc7f65b844c/devtools/client/debugger/src/components/Editor/EmptyLines.js#53-74

disableEmptyLines() {
  const { breakableLines, selectedSource, editor } = this.props;

  editor.codeMirror.operation(() => {
    editor.codeMirror.eachLine(lineHandle => {
      const line = fromEditorLine(
        selectedSource.id,
        editor.codeMirror.getLineNumber(lineHandle)
      );

      if (breakableLines.has(line)) {
        editor.codeMirror.removeLineClass(
          lineHandle,
          "wrapClass",
          "empty-line"
        );
      } else {
        editor.codeMirror.addLineClass(lineHandle, "wrapClass", "empty-line");
      }
    });
  });
}

2 immediate things are visible:

1. fromEditorLine is checking if the source is wasm. And so here we check it for every line, while this information is unique per file

https://searchfox.org/mozilla-central/rev/aa3ccd258b64abfd4c5ce56c1f512bc7f65b844c/devtools/client/debugger/src/utils/editor/index.js#73-79

export function fromEditorLine(sourceId, line) {
  if (isWasm(sourceId)) {
    return lineToWasmOffset(sourceId, line) || 0;
  }

  return line + 1;
}

2. Using codeMirror.eachLine means that we need to call codeMirror.getLineNumber, for each line. Since codeMirror.(add|remove)LineClass can take a line index instead of a lineHandle, we could have a simple for loop instead.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1819269 - [devtools] Make disableEmptyLines faster. r=#devtools-reviewers

• Replace codeMirror.eachLine with a simple for loop
• Change fromEditorLine signature so it takes a new parameter indicating
  if the source is wasm. This allows us to only compute this information once
  per file, and not for each line of a file.

Depends on D171186

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52980b1f16ff
[devtools] Make disableEmptyLines faster. r=jdescottes

Comment 3

[Atila Butkovits]

Backed out for causing Bug 1819531.

Backout link: https://hg.mozilla.org/integration/autoland/rev/6833a092ebd6bea1be81a71cb87a8d55b917e804

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7edfdbd5daf0
[devtools] Make disableEmptyLines faster. r=jdescottes

Comment 5

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/7edfdbd5daf0