Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Commit

Permalink
Merge pull request #313 from atom/ns-ro/avoid-building-lines
Browse files Browse the repository at this point in the history
Avoid construction of JS strings representing buffer lines on performance-sensitive code paths
  • Loading branch information
Nathan Sobo authored Jun 7, 2019
2 parents a6ac229 + 1f93f27 commit 06d1a50
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 51 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"mkdirp": "^0.5.1",
"pathwatcher": "8.0.2",
"serializable": "^1.0.3",
"superstring": "2.3.6",
"superstring": "2.4.0",
"underscore-plus": "^1.0.0"
},
"standard": {
Expand Down
35 changes: 17 additions & 18 deletions src/display-layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -542,12 +542,11 @@ class DisplayLayer {
}

getClipColumnDelta (bufferPosition, clipDirection) {
var {row: bufferRow, column: bufferColumn} = bufferPosition
var bufferLine = this.buffer.lineForRow(bufferRow)
var {row, column} = bufferPosition

// Treat paired unicode characters as atomic...
var previousCharacter = bufferLine[bufferColumn - 1]
var character = bufferLine[bufferColumn]
var character = this.buffer.getCharacterAtPosition(bufferPosition)
var previousCharacter = this.buffer.getCharacterAtPosition([row, column - 1])
if (previousCharacter && character && isCharacterPair(previousCharacter, character)) {
if (clipDirection === 'closest' || clipDirection === 'backward') {
return -1
Expand All @@ -560,27 +559,27 @@ class DisplayLayer {

if (!this.atomicSoftTabs) return 0

if (bufferColumn * this.ratioForCharacter(' ') > this.softWrapColumn) {
if (column * this.ratioForCharacter(' ') > this.softWrapColumn) {
return 0
}

for (let column = bufferColumn; column >= 0; column--) {
if (bufferLine[column] !== ' ') return 0
for (let position = {row, column}; position.column >= 0; position.column--) {
if (this.buffer.getCharacterAtPosition(position) !== ' ') return 0
}

var previousTabStop = bufferColumn - (bufferColumn % this.tabLength)
if (bufferColumn === previousTabStop) return 0
var previousTabStop = column - (column % this.tabLength)
if (column === previousTabStop) return 0
var nextTabStop = previousTabStop + this.tabLength

// If there is a non-whitespace character before the next tab stop,
// don't this whitespace as a soft tab
for (let column = bufferColumn; column < nextTabStop; column++) {
if (bufferLine[column] !== ' ') return 0
for (let position = {row, column}; position.column < nextTabStop; position.column++) {
if (this.buffer.getCharacterAtPosition(position) !== ' ') return 0
}

var clippedColumn
if (clipDirection === 'closest') {
if (bufferColumn - previousTabStop > this.tabLength / 2) {
if (column - previousTabStop > this.tabLength / 2) {
clippedColumn = nextTabStop
} else {
clippedColumn = previousTabStop
Expand All @@ -591,7 +590,7 @@ class DisplayLayer {
clippedColumn = nextTabStop
}

return clippedColumn - bufferColumn
return clippedColumn - column
}

getText (startRow, endRow) {
Expand Down Expand Up @@ -703,15 +702,15 @@ class DisplayLayer {
return length
}

findTrailingWhitespaceStartColumn (lineText) {
let column
for (column = lineText.length; column >= 0; column--) {
const previousCharacter = lineText[column - 1]
findTrailingWhitespaceStartColumn (bufferRow) {
let position
for (position = {row: bufferRow, column: this.buffer.lineLengthForRow(bufferRow) - 1}; position.column >= 0; position.column--) {
const previousCharacter = this.buffer.getCharacterAtPosition(position)
if (previousCharacter !== ' ' && previousCharacter !== '\t') {
break
}
}
return column
return position.column + 1
}

registerBuiltInScope (flags, className) {
Expand Down
70 changes: 38 additions & 32 deletions src/screen-line-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,14 @@ class ScreenLineBuilder {
this.requestedEndScreenRow = endScreenRow
this.displayLayer.populateSpatialIndexIfNeeded(this.displayLayer.buffer.getLineCount(), endScreenRow)

this.bufferRow = this.displayLayer.translateScreenPositionWithSpatialIndex(Point(startScreenRow, 0)).row
this.bufferRow = this.displayLayer.findBoundaryPrecedingBufferRow(this.bufferRow)
this.screenRow = this.displayLayer.translateBufferPositionWithSpatialIndex(Point(this.bufferRow, 0)).row
this.bufferPosition = {
row: this.displayLayer.findBoundaryPrecedingBufferRow(
this.displayLayer.translateScreenPositionWithSpatialIndex(Point(startScreenRow, 0)).row
),
column: 0
}

this.screenRow = this.displayLayer.translateBufferPositionWithSpatialIndex(Point(this.bufferPosition.row, 0)).row

const endBufferRow = this.displayLayer.translateScreenPositionWithSpatialIndex(Point(endScreenRow, Infinity)).row

Expand All @@ -37,7 +42,7 @@ class ScreenLineBuilder {
this.containingScopeIds = []
this.scopeIdsToReopen = []
this.screenLines = []
this.bufferColumn = 0
this.bufferPosition.column = 0
this.beginLine()

// Loop through all characters spanning the given screen row range, building
Expand All @@ -54,12 +59,12 @@ class ScreenLineBuilder {
if (nextHunk.newStart.row === this.screenRow) {
if (nextHunk.newEnd.row > nextHunk.newStart.row) {
this.screenRow++
this.bufferColumn = nextHunk.oldEnd.column
this.bufferPosition.column = nextHunk.oldEnd.column
hunkIndex++
continue screenRowLoop
} else {
this.bufferRow = nextHunk.oldEnd.row
this.bufferColumn = nextHunk.oldEnd.column
this.bufferPosition.row = nextHunk.oldEnd.row
this.bufferPosition.column = nextHunk.oldEnd.column
}
}

Expand All @@ -68,22 +73,23 @@ class ScreenLineBuilder {
}

this.screenRow++
this.bufferRow++
this.screenColumn = 0
this.bufferColumn = 0
this.bufferPosition.row++
this.bufferPosition.column = 0
continue
}

this.currentBuiltInClassNameFlags = 0
this.bufferLine = this.displayLayer.buffer.lineForRow(this.bufferRow)
if (this.bufferLine == null) break
this.trailingWhitespaceStartColumn = this.displayLayer.findTrailingWhitespaceStartColumn(this.bufferLine)
this.bufferLineLength = this.displayLayer.buffer.lineLengthForRow(this.bufferPosition.row)

if (this.bufferPosition.row > this.displayLayer.buffer.getLastRow()) break
this.trailingWhitespaceStartColumn = this.displayLayer.findTrailingWhitespaceStartColumn(this.bufferPosition.row)
this.inLeadingWhitespace = true
this.inTrailingWhitespace = false

if (!didSeekDecorationIterator || this.compareBufferPosition(decorationIterator.getPosition()) > 0) {
didSeekDecorationIterator = true
this.scopeIdsToReopen = decorationIterator.seek(Point(this.bufferRow, this.bufferColumn), endBufferRow)
this.scopeIdsToReopen = decorationIterator.seek(this.bufferPosition, endBufferRow)
}

var prevCachedScreenLine = this.displayLayer.cachedScreenLines[this.screenRow - 1]
Expand All @@ -94,10 +100,10 @@ class ScreenLineBuilder {

// This loop may visit multiple buffer rows if there are folds and
// multiple screen rows if there are soft wraps.
while (this.bufferColumn <= this.bufferLine.length) {
while (this.bufferPosition.column <= this.bufferLineLength) {
// Handle folds or soft wraps at the current position.
var nextHunk = hunks[hunkIndex]
while (nextHunk && nextHunk.oldStart.row === this.bufferRow && nextHunk.oldStart.column === this.bufferColumn) {
while (nextHunk && nextHunk.oldStart.row === this.bufferPosition.row && nextHunk.oldStart.column === this.bufferPosition.column) {
if (this.displayLayer.isSoftWrapHunk(nextHunk)) {
this.emitSoftWrap(nextHunk)
if (this.screenRow === endScreenRow) {
Expand All @@ -111,8 +117,8 @@ class ScreenLineBuilder {
nextHunk = hunks[hunkIndex]
}

var nextCharacter = this.bufferLine[this.bufferColumn]
if (this.bufferColumn >= this.trailingWhitespaceStartColumn) {
var nextCharacter = this.displayLayer.buffer.getCharacterAtPosition(this.bufferPosition)
if (this.bufferPosition.column >= this.trailingWhitespaceStartColumn) {
this.inTrailingWhitespace = true
this.inLeadingWhitespace = false
} else if (nextCharacter !== ' ' && nextCharacter !== '\t') {
Expand All @@ -131,7 +137,7 @@ class ScreenLineBuilder {
this.emitDecorationBoundaries(decorationIterator)

// Are we at the end of the line?
if (this.bufferColumn === this.bufferLine.length) {
if (this.bufferPosition.column === this.bufferLineLength) {
this.emitLineEnding()
break
}
Expand All @@ -150,7 +156,7 @@ class ScreenLineBuilder {
} else {
this.emitText(nextCharacter)
}
this.bufferColumn++
this.bufferPosition.column++
}
}

Expand Down Expand Up @@ -245,13 +251,13 @@ class ScreenLineBuilder {
this.emitText(this.displayLayer.foldCharacter)
this.emitCloseTag(this.getBuiltInScopeId(FOLD))

this.bufferRow = nextHunk.oldEnd.row
this.bufferColumn = nextHunk.oldEnd.column
this.bufferPosition.row = nextHunk.oldEnd.row
this.bufferPosition.column = nextHunk.oldEnd.column

this.scopeIdsToReopen = decorationIterator.seek(Point(this.bufferRow, this.bufferColumn), endBufferRow)
this.scopeIdsToReopen = decorationIterator.seek(this.bufferPosition, endBufferRow)

this.bufferLine = this.displayLayer.buffer.lineForRow(this.bufferRow)
this.trailingWhitespaceStartColumn = this.displayLayer.findTrailingWhitespaceStartColumn(this.bufferLine)
this.bufferLineLength = this.displayLayer.buffer.lineLengthForRow(this.bufferPosition.row)
this.trailingWhitespaceStartColumn = this.displayLayer.findTrailingWhitespaceStartColumn(this.bufferPosition.row)
}

emitSoftWrap (nextHunk) {
Expand All @@ -265,18 +271,18 @@ class ScreenLineBuilder {
emitLineEnding () {
this.emitCloseTag(this.getBuiltInScopeId(this.currentBuiltInClassNameFlags))

let lineEnding = this.displayLayer.buffer.lineEndingForRow(this.bufferRow)
let lineEnding = this.displayLayer.buffer.lineEndingForRow(this.bufferPosition.row)
const eolInvisible = this.displayLayer.eolInvisibles[lineEnding]
if (eolInvisible) {
let eolFlags = INVISIBLE_CHARACTER | LINE_ENDING
if (this.bufferLine.length === 0 && this.displayLayer.showIndentGuides) eolFlags |= INDENT_GUIDE
if (this.bufferLineLength === 0 && this.displayLayer.showIndentGuides) eolFlags |= INDENT_GUIDE
this.emitOpenTag(this.getBuiltInScopeId(eolFlags))
this.emitText(eolInvisible, false)
this.emitCloseTag(this.getBuiltInScopeId(eolFlags))
}

if (this.bufferLine.length === 0 && this.displayLayer.showIndentGuides) {
let whitespaceLength = this.displayLayer.leadingWhitespaceLengthForSurroundingLines(this.bufferRow)
if (this.bufferLineLength === 0 && this.displayLayer.showIndentGuides) {
let whitespaceLength = this.displayLayer.leadingWhitespaceLengthForSurroundingLines(this.bufferPosition.row)
this.emitIndentWhitespace(whitespaceLength)
}

Expand All @@ -286,8 +292,8 @@ class ScreenLineBuilder {
// the caller
if (this.currentScreenLineTags.length === 0) this.currentScreenLineTags.push(0)
this.emitNewline()
this.bufferRow++
this.bufferColumn = 0
this.bufferPosition.row++
this.bufferPosition.column = 0
}

emitNewline (softWrapIndent = -1) {
Expand Down Expand Up @@ -417,7 +423,7 @@ class ScreenLineBuilder {
}

compareBufferPosition (position) {
const rowComparison = this.bufferRow - position.row
return rowComparison === 0 ? (this.bufferColumn - position.column) : rowComparison
const rowComparison = this.bufferPosition.row - position.row
return rowComparison === 0 ? (this.bufferPosition.column - position.column) : rowComparison
}
}
4 changes: 4 additions & 0 deletions src/text-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,10 @@ class TextBuffer {
return this.cachedText
}

getCharacterAtPosition (position) {
return this.buffer.getCharacterAtPosition(Point.fromObject(position))
}

// Public: Get the text in a range.
//
// * `range` A {Range}
Expand Down

0 comments on commit 06d1a50

Please # to comment.