Merge pull request #12898 from atom/mb-defer-work-when-opening-files

Avoid forcing computation of all screen lines when opening a file

benchmarks/text-editor-large-file-construction.bench.js

@@ -1,19 +1,90 @@
 /** @babel */
 
-import fs from 'fs'
-import temp from 'temp'
 import {TextEditor, TextBuffer} from 'atom'
 
-export default function ({test}) {
-  const text = 'Lorem ipsum dolor sit amet\n'.repeat(test ? 10 : 500000)
-  const t0 = window.performance.now()
-  const buffer = new TextBuffer(text)
-  const editor = new TextEditor({buffer, largeFileMode: true})
-  editor.element.style.height = "600px"
-  document.body.appendChild(editor.element)
-  const t1 = window.performance.now()
-  editor.element.remove()
-  editor.destroy()
+const MIN_SIZE_IN_KB = 0 * 1024
+const MAX_SIZE_IN_KB = 10 * 1024
+const SIZE_STEP_IN_KB = 1024
+const LINE_TEXT = 'Lorem ipsum dolor sit amet\n'
+const TEXT = LINE_TEXT.repeat(Math.ceil(MAX_SIZE_IN_KB * 1024 / LINE_TEXT.length))
 
-  return [{name: 'Opening and rendering a large file', duration: t1 - t0}]
+export default async function ({test}) {
+  const data = []
+
+  const workspaceElement = atom.views.getView(atom.workspace)
+  document.body.appendChild(workspaceElement)
+
+  atom.packages.loadPackages()
+  await atom.packages.activate()
+
+  for (let pane of atom.workspace.getPanes()) {
+    pane.destroy()
+  }
+
+  for (let sizeInKB = MIN_SIZE_IN_KB; sizeInKB < MAX_SIZE_IN_KB; sizeInKB += SIZE_STEP_IN_KB) {
+    const text = TEXT.slice(0, sizeInKB * 1024)
+    console.log(text.length / 1024)
+
+    let t0 = window.performance.now()
+    const buffer = new TextBuffer(text)
+    const editor = new TextEditor({buffer, largeFileMode: true})
+    atom.workspace.getActivePane().activateItem(editor)
+    let t1 = window.performance.now()
+
+    data.push({
+      name: 'Opening a large file',
+      x: sizeInKB,
+      duration: t1 - t0
+    })
+
+    const tickDurations = []
+    for (let i = 0; i < 20; i++) {
+      await timeout(50)
+      t0 = window.performance.now()
+      await timeout(0)
+      t1 = window.performance.now()
+      tickDurations[i] = t1 - t0
+    }
+
+    data.push({
+      name: 'Max time event loop was blocked after opening a large file',
+      x: sizeInKB,
+      duration: Math.max(...tickDurations)
+    })
+
+    t0 = window.performance.now()
+    editor.setCursorScreenPosition(editor.element.screenPositionForPixelPosition({
+      top: 100,
+      left: 30
+    }))
+    t1 = window.performance.now()
+
+    data.push({
+      name: 'Clicking the editor after opening a large file',
+      x: sizeInKB,
+      duration: t1 - t0
+    })
+
+    t0 = window.performance.now()
+    editor.element.setScrollTop(editor.element.getScrollTop() + 100)
+    t1 = window.performance.now()
+
+    data.push({
+      name: 'Scrolling down after opening a large file',
+      x: sizeInKB,
+      duration: t1 - t0
+    })
+
+    editor.destroy()
+    buffer.destroy()
+    await timeout(10000)
+  }
+
+  workspaceElement.remove()
+
+  return data
+}
+
+function timeout (duration) {
+  return new Promise((resolve) => setTimeout(resolve, duration))
 }

package.json

@@ -60,7 +60,7 @@
     "sinon": "1.17.4",
     "source-map-support": "^0.3.2",
     "temp": "0.8.1",
-    "text-buffer": "9.3.0",
+    "text-buffer": "9.4.0",
     "typescript-simple": "1.0.0",
     "underscore-plus": "^1.6.6",
     "winreg": "^1.2.1",

spec/lines-yardstick-spec.coffee

@@ -40,11 +40,13 @@ describe "LinesYardstick", ->
 
       mockLineNodesProvider =
         lineNodesById: {}
+
         lineIdForScreenRow: (screenRow) ->
-          editor.screenLineForScreenRow(screenRow).id
+          editor.screenLineForScreenRow(screenRow)?.id
 
         lineNodeForScreenRow: (screenRow) ->
-          @lineNodesById[@lineIdForScreenRow(screenRow)] ?= buildLineNode(screenRow)
+          if id = @lineIdForScreenRow(screenRow)
+            @lineNodesById[id] ?= buildLineNode(screenRow)
 
         textNodesForScreenRow: (screenRow) ->
           lineNode = @lineNodeForScreenRow(screenRow)

src/lines-yardstick.coffee

@@ -13,19 +13,21 @@ class LinesYardstick
   measuredRowForPixelPosition: (pixelPosition) ->
     targetTop = pixelPosition.top
     row = Math.floor(targetTop / @model.getLineHeightInPixels())
-    row if 0 <= row <= @model.getLastScreenRow()
+    row if 0 <= row
 
   screenPositionForPixelPosition: (pixelPosition) ->
     targetTop = pixelPosition.top
-    targetLeft = pixelPosition.left
-    row = @lineTopIndex.rowForPixelPosition(targetTop)
-    targetLeft = 0 if targetTop < 0 or targetLeft < 0
-    targetLeft = Infinity if row > @model.getLastScreenRow()
-    row = Math.min(row, @model.getLastScreenRow())
-    row = Math.max(0, row)
-
+    row = Math.max(0, @lineTopIndex.rowForPixelPosition(targetTop))
     lineNode = @lineNodesProvider.lineNodeForScreenRow(row)
-    return Point(row, 0) unless lineNode
+    unless lineNode
+      lastScreenRow = @model.getLastScreenRow()
+      if row > lastScreenRow
+        return Point(lastScreenRow, @model.lineLengthForScreenRow(lastScreenRow))
+      else
+        return Point(row, 0)
+
+    targetLeft = pixelPosition.left
+    targetLeft = 0 if targetTop < 0 or targetLeft < 0
 
     textNodes = @lineNodesProvider.textNodesForScreenRow(row)
     lineOffset = lineNode.getBoundingClientRect().left

src/text-editor-presenter.coffee

@@ -138,6 +138,11 @@ class TextEditorPresenter
     @shouldUpdateDecorations = true
 
   observeModel: ->
+    @disposables.add @model.displayLayer.onDidReset =>
+      @spliceBlockDecorationsInRange(0, Infinity, Infinity)
+      @shouldUpdateDecorations = true
+      @emitDidUpdateState()
+
     @disposables.add @model.displayLayer.onDidChangeSync (changes) =>
       for change in changes
         startRow = change.start.row
@@ -292,24 +297,21 @@ class TextEditorPresenter
   tileForRow: (row) ->
     row - (row % @tileSize)
 
-  constrainRow: (row) ->
-    Math.max(0, Math.min(row, @model.getScreenLineCount()))
-
   getStartTileRow: ->
-    @constrainRow(@tileForRow(@startRow ? 0))
+    @tileForRow(@startRow ? 0)
 
   getEndTileRow: ->
-    @constrainRow(@tileForRow(@endRow ? 0))
+    @tileForRow(@endRow ? 0)
 
   isValidScreenRow: (screenRow) ->
-    screenRow >= 0 and screenRow < @model.getScreenLineCount()
+    screenRow >= 0 and screenRow < @model.getApproximateScreenLineCount()
 
   getScreenRowsToRender: ->
     startRow = @getStartTileRow()
-    endRow = @constrainRow(@getEndTileRow() + @tileSize)
+    endRow = @getEndTileRow() + @tileSize
 
     screenRows = [startRow...endRow]
-    longestScreenRow = @model.getLongestScreenRow()
+    longestScreenRow = @model.getApproximateLongestScreenRow()
     if longestScreenRow?
       screenRows.push(longestScreenRow)
     if @screenRowsToMeasure?
@@ -355,7 +357,7 @@ class TextEditorPresenter
     zIndex = 0
 
     for tileStartRow in [@tileForRow(endRow)..@tileForRow(startRow)] by -@tileSize
-      tileEndRow = @constrainRow(tileStartRow + @tileSize)
+      tileEndRow = tileStartRow + @tileSize
       rowsWithinTile = []
 
       while screenRowIndex >= 0
@@ -390,7 +392,7 @@ class TextEditorPresenter
       visibleTiles[tileStartRow] = true
       zIndex++
 
-    if @mouseWheelScreenRow? and 0 <= @mouseWheelScreenRow < @model.getScreenLineCount()
+    if @mouseWheelScreenRow? and 0 <= @mouseWheelScreenRow < @model.getApproximateScreenLineCount()
       mouseWheelTile = @tileForRow(@mouseWheelScreenRow)
 
       unless visibleTiles[mouseWheelTile]?
@@ -409,8 +411,7 @@ class TextEditorPresenter
     visibleLineIds = {}
     for screenRow in screenRows
       line = @linesByScreenRow.get(screenRow)
-      unless line?
-        throw new Error("No line exists for row #{screenRow}. Last screen row: #{@model.getLastScreenRow()}")
+      continue unless line?
 
       visibleLineIds[line.id] = true
       precedingBlockDecorations = @precedingBlockDecorationsByScreenRow[screenRow] ? []
@@ -598,7 +599,9 @@ class TextEditorPresenter
     visibleLineNumberIds = {}
 
     for screenRow in screenRows when @isRowRendered(screenRow)
-      lineId = @linesByScreenRow.get(screenRow).id
+      line = @linesByScreenRow.get(screenRow)
+      continue unless line?
+      lineId = line.id
       {bufferRow, softWrappedAtStart: softWrapped} = @displayLayer.softWrapDescriptorForScreenRow(screenRow)
       foldable = not softWrapped and @model.isFoldableAtBufferRow(bufferRow)
       decorationClasses = @lineNumberDecorationClassesForRow(screenRow)
@@ -625,7 +628,7 @@ class TextEditorPresenter
     return unless @scrollTop? and @lineHeight? and @height?
 
     @endRow = Math.min(
-      @model.getScreenLineCount(),
+      @model.getApproximateScreenLineCount(),
       @lineTopIndex.rowForPixelPosition(@scrollTop + @height + @lineHeight - 1) + 1
     )
 
@@ -659,7 +662,7 @@ class TextEditorPresenter
   updateVerticalDimensions: ->
     if @lineHeight?
       oldContentHeight = @contentHeight
-      @contentHeight = Math.round(@lineTopIndex.pixelPositionAfterBlocksForRow(@model.getScreenLineCount()))
+      @contentHeight = Math.round(@lineTopIndex.pixelPositionAfterBlocksForRow(@model.getApproximateScreenLineCount()))
 
     if @contentHeight isnt oldContentHeight
       @updateHeight()
@@ -669,7 +672,7 @@ class TextEditorPresenter
   updateHorizontalDimensions: ->
     if @baseCharacterWidth?
       oldContentWidth = @contentWidth
-      rightmostPosition = @model.getRightmostScreenPosition()
+      rightmostPosition = @model.getApproximateRightmostScreenPosition()
       @contentWidth = @pixelPositionForScreenPosition(rightmostPosition).left
       @contentWidth += @scrollLeft
       @contentWidth += 1 unless @model.isSoftWrapped() # account for cursor width
@@ -1530,7 +1533,7 @@ class TextEditorPresenter
     [@startRow, @endRow]
 
   isRowRendered: (row) ->
-    @getStartTileRow() <= row < @constrainRow(@getEndTileRow() + @tileSize)
+    @getStartTileRow() <= row < @getEndTileRow() + @tileSize
 
   isOpenTagCode: (tagCode) ->
     @displayLayer.isOpenTagCode(tagCode)

src/text-editor.coffee

@@ -2,7 +2,7 @@ _ = require 'underscore-plus'
 path = require 'path'
 fs = require 'fs-plus'
 Grim = require 'grim'
-{CompositeDisposable, Emitter} = require 'event-kit'
+{CompositeDisposable, Disposable, Emitter} = require 'event-kit'
 {Point, Range} = TextBuffer = require 'text-buffer'
 LanguageMode = require './language-mode'
 DecorationManager = require './decoration-manager'
@@ -184,6 +184,10 @@ class TextEditor extends Model
     else
       @displayLayer = @buffer.addDisplayLayer(displayLayerParams)
 
+    @backgroundWorkHandle = requestIdleCallback(@doBackgroundWork)
+    @disposables.add new Disposable =>
+      cancelIdleCallback(@backgroundWorkHandle) if @backgroundWorkHandle?
+
     @displayLayer.setTextDecorationLayer(@tokenizedBuffer)
     @defaultMarkerLayer = @displayLayer.addMarkerLayer()
     @selectionsMarkerLayer ?= @addMarkerLayer(maintainHistory: true, persistent: true)
@@ -210,6 +214,13 @@ class TextEditor extends Model
       priority: 0
       visible: lineNumberGutterVisible
 
+  doBackgroundWork: (deadline) =>
+    if @displayLayer.doBackgroundWork(deadline)
+      @presenter?.updateVerticalDimensions()
+      @backgroundWorkHandle = requestIdleCallback(@doBackgroundWork)
+    else
+      @backgroundWorkHandle = null
+
   update: (params) ->
     displayLayerParams = {}
 
@@ -394,6 +405,9 @@ class TextEditor extends Model
     @disposables.add @displayLayer.onDidChangeSync (e) =>
       @mergeIntersectingSelections()
       @emitter.emit 'did-change', e
+    @disposables.add @displayLayer.onDidReset =>
+      @mergeIntersectingSelections()
+      @emitter.emit 'did-change', {}
 
   destroyed: ->
     @disposables.dispose()
@@ -910,6 +924,8 @@ class TextEditor extends Model
   # editor. This accounts for folds.
   getScreenLineCount: -> @displayLayer.getScreenLineCount()
 
+  getApproximateScreenLineCount: -> @displayLayer.getApproximateScreenLineCount()
+
   # Essential: Returns a {Number} representing the last zero-indexed buffer row
   # number of the editor.
   getLastBufferRow: -> @buffer.getLastRow()
@@ -956,7 +972,6 @@ class TextEditor extends Model
     tokens
 
   screenLineForScreenRow: (screenRow) ->
-    return if screenRow < 0 or screenRow > @getLastScreenRow()
     @displayLayer.getScreenLines(screenRow, screenRow + 1)[0]
 
   bufferRowForScreenRow: (screenRow) ->
@@ -974,10 +989,14 @@ class TextEditor extends Model
 
   getRightmostScreenPosition: -> @displayLayer.getRightmostScreenPosition()
 
+  getApproximateRightmostScreenPosition: -> @displayLayer.getApproximateRightmostScreenPosition()
+
   getMaxScreenLineLength: -> @getRightmostScreenPosition().column
 
   getLongestScreenRow: -> @getRightmostScreenPosition().row
 
+  getApproximateLongestScreenRow: -> @getApproximateRightmostScreenPosition().row
+
   lineLengthForScreenRow: (screenRow) -> @displayLayer.lineLengthForScreenRow(screenRow)
 
   # Returns the range for the given buffer row.