Optimize getMarks(AtRange), getActiveMarksAtRange, getInsertMarksAtRange, remove the dependency of getCharacters (#1808)

* Rewrite getCharacters(AtRange); rewrite getMarks

* Single concat for getMarks

* getActiveMarksAtRange

* getMarksAtRange and getInsertMakrsAtRange

* fix typo

* import getTextsBetweenPositionsAsArray to run getMarks

* Fix eslint error

* Restore getTextsAtRange

* Remove getMarksAtCollapsedRange; Add annotations

* Annotation

* Fix endTexts

* Fix typos

* Fix long line

* Fix getTextsAtRange List; Fix getTextsAtRangeAsArray

* Explain early return for character.marks short cut

* Styling

* Styling

* Styling

packages/slate/src/models/node.js

@@ -10,7 +10,6 @@ import Inline from './inline'
 import Range from './range'
 import Text from './text'
 import generateKey from '../utils/generate-key'
-import isIndexInRange from '../utils/is-index-in-range'
 import memoize from '../utils/memoize'
 
 /**
@@ -469,22 +468,7 @@ class Node {
    */
 
   getCharacters() {
-    const array = this.getCharactersAsArray()
+    return this.getTexts().flatMap(t => t.characters)
-    return new List(array)
-  }
-
-  /**
-   * Get all of the characters for every text node as an array
-   *
-   * @return {Array}
-   */
-
-  getCharactersAsArray() {
-    return this.nodes.reduce((arr, node) => {
-      return node.object == 'text'
-        ? arr.concat(node.characters.toArray())
-        : arr.concat(node.getCharactersAsArray())
-    }, [])
   }
 
   /**
@@ -495,28 +479,23 @@ class Node {
    */
 
   getCharactersAtRange(range) {
-    const array = this.getCharactersAtRangeAsArray(range)
-    return new List(array)
-  }
-
-  /**
-   * Get a list of the characters in a `range` as an array.
-   *
-   * @param {Range} range
-   * @return {Array}
-   */
-
-  getCharactersAtRangeAsArray(range) {
     range = range.normalize(this)
-    if (range.isUnset) return []
+    if (range.isUnset) return List()
+    const { startKey, endKey, startOffset, endOffset } = range
+    if (startKey === endKey) {
+      const endText = this.getDescendant(endKey)
+      return endText.characters.slice(startOffset, endOffset)
+    }
 
-    return this.getTextsAtRange(range).reduce((arr, text) => {
+    return this.getTextsAtRange(range).flatMap(t => {
-      const chars = text.characters
+      if (t.key === startKey) {
-        .filter((char, i) => isIndexInRange(i, text, range))
+        return t.characters.slice(startOffset)
-        .toArray()
+      }
-
+      if (t.key === endKey) {
-      return arr.concat(chars)
+        return t.characters.slice(0, endOffset)
-    }, [])
+      }
+      return t.characters
+    })
   }
 
   /**
@@ -1024,9 +1003,13 @@ class Node {
    */
 
   getMarksAsArray() {
-    return this.nodes.reduce((marks, node) => {
+    // PERF: use only one concat rather than multiple concat
-      return marks.concat(node.getMarksAsArray())
+    // becuase one concat is faster
-    }, [])
+    const result = []
+    this.nodes.forEach(node => {
+      result.push(node.getMarksAsArray())
+    })
+    return Array.prototype.concat.apply([], result)
   }
 
   /**
@@ -1037,8 +1020,7 @@ class Node {
    */
 
   getMarksAtRange(range) {
-    const array = this.getMarksAtRangeAsArray(range)
+    return new Set(this.getOrderedMarksAtRange(range))
-    return new Set(array)
   }
 
   /**
@@ -1049,8 +1031,18 @@ class Node {
    */
 
   getInsertMarksAtRange(range) {
-    const array = this.getInsertMarksAtRangeAsArray(range)
+    range = range.normalize(this)
-    return new Set(array)
+    if (range.isUnset) return Set()
+    if (range.isCollapsed) {
+      // PERF: range is not cachable, use key and offset as proxies for cache
+      return this.getMarksAtPosition(range.startKey, range.startOffset)
+    }
+
+    const text = this.getDescendant(range.startKey)
+    const char = text.characters.get(range.startOffset)
+    if (!char) return Set()
+
+    return char.marks
   }
 
   /**
@@ -1061,8 +1053,54 @@ class Node {
    */
 
   getOrderedMarksAtRange(range) {
-    const array = this.getMarksAtRangeAsArray(range)
+    range = range.normalize(this)
-    return new OrderedSet(array)
+    if (range.isUnset) return OrderedSet()
+    if (range.isCollapsed) {
+      // PERF: range is not cachable, use key and offset as proxies for cache
+      return this.getMarksAtPosition(range.startKey, range.startOffset)
+    }
+
+    const { startKey, startOffset, endKey, endOffset } = range
+    return this.getOrderedMarksBetweenPositions(
+      startKey,
+      startOffset,
+      endKey,
+      endOffset
+    )
+  }
+
+  /**
+   * Get a set of the marks in a `range`.
+   * PERF: arguments use key and offset for utilizing cache
+   *
+   * @param {string} startKey
+   * @param {number} startOffset
+   * @param {string} endKey
+   * @param {number} endOffset
+   * @returns {OrderedSet<Mark>}
+   */
+
+  getOrderedMarksBetweenPositions(startKey, startOffset, endKey, endOffset) {
+    if (startKey === endKey) {
+      const startText = this.getDescendant(startKey)
+      return startText.getMarksBetweenOffsets(startOffset, endOffset)
+    }
+
+    const texts = this.getTextsBetweenPositionsAsArray(startKey, endKey)
+
+    return OrderedSet().withMutations(result => {
+      texts.forEach(text => {
+        if (text.key === startKey) {
+          result.union(
+            text.getMarksBetweenOffsets(startOffset, text.text.length)
+          )
+        } else if (text.key === endKey) {
+          result.union(text.getMarksBetweenOffsets(0, endOffset))
+        } else {
+          result.union(text.getMarks())
+        }
+      })
+    })
   }
 
   /**
@@ -1073,108 +1111,81 @@ class Node {
    */
 
   getActiveMarksAtRange(range) {
-    const array = this.getActiveMarksAtRangeAsArray(range)
-    return new Set(array)
-  }
-
-  /**
-   * Get a set of the marks in a `range`, by unioning.
-   *
-   * @param {Range} range
-   * @return {Array}
-   */
-
-  getMarksAtRangeAsArray(range) {
     range = range.normalize(this)
-    if (range.isUnset) return []
+    if (range.isUnset) return Set()
-    if (range.isCollapsed) return this.getMarksAtCollapsedRangeAsArray(range)
+    if (range.isCollapsed) {
-
+      const { startKey, startOffset } = range
-    return this.getCharactersAtRange(range).reduce((memo, char) => {
+      return this.getMarksAtPosition(startKey, startOffset).toSet()
-      if (char) {
-        char.marks.toArray().forEach(c => memo.push(c))
-      }
-      return memo
-    }, [])
-  }
-
-  /**
-   * Get a set of the marks in a `range` for insertion behavior.
-   *
-   * @param {Range} range
-   * @return {Array}
-   */
-
-  getInsertMarksAtRangeAsArray(range) {
-    range = range.normalize(this)
-    if (range.isUnset) return []
-    if (range.isCollapsed) return this.getMarksAtCollapsedRangeAsArray(range)
-
-    const text = this.getDescendant(range.startKey)
-    const char = text.characters.get(range.startOffset)
-    if (!char) return []
-
-    return char.marks.toArray()
-  }
-
-  /**
-   * Get a set of marks in a `range`, by treating it as collapsed.
-   *
-   * @param {Range} range
-   * @return {Array}
-   */
-
-  getMarksAtCollapsedRangeAsArray(range) {
-    if (range.isUnset) return []
-
-    const { startKey, startOffset } = range
-
-    if (startOffset == 0) {
-      const previous = this.getPreviousText(startKey)
-      if (!previous || previous.text.length == 0) return []
-      if (
-        this.getClosestBlock(startKey) !== this.getClosestBlock(previous.key)
-      ) {
-        return []
-      }
-      const char = previous.characters.get(previous.text.length - 1)
-      if (!char) return []
-
-      return char.marks.toArray()
     }
 
-    const text = this.getDescendant(startKey)
+    let { startKey, endKey, startOffset, endOffset } = range
-    const char = text.characters.get(startOffset - 1)
+    let startText = this.getDescendant(startKey)
-    if (!char) return []
 
-    return char.marks.toArray()
+    if (startKey !== endKey) {
+      while (startKey !== endKey && endOffset === 0) {
+        const endText = this.getPreviousText(endKey)
+        endKey = endText.key
+        endOffset = endText.text.length
+      }
+
+      while (startKey !== endKey && startOffset === startText.text.length) {
+        startText = this.getNextText(startKey)
+        startKey = startText.key
+        startOffset = 0
+      }
+    }
+
+    if (startKey === endKey) {
+      return startText.getActiveMarksBetweenOffsets(startOffset, endOffset)
+    }
+
+    const startMarks = startText.getActiveMarksBetweenOffsets(
+      startOffset,
+      startText.text.length
+    )
+    if (startMarks.size === 0) return Set()
+    const endText = this.getDescendant(endKey)
+    const endMarks = endText.getActiveMarksBetweenOffsets(0, endOffset)
+    let marks = startMarks.intersect(endMarks)
+    // If marks is already empty, the active marks is empty
+    if (marks.size === 0) return marks
+
+    let text = this.getNextText(startKey)
+    while (text.key !== endKey) {
+      if (text.text.length !== 0) {
+        marks = marks.intersect(text.getActiveMarks())
+        if (marks.size === 0) return Set()
+      }
+      text = this.getNextText(text.key)
+    }
+    return marks
   }
 
   /**
-   * Get a set of marks in a `range`, by intersecting.
+   * Get a set of marks in a `position`, the equivalent of a collapsed range
    *
-   * @param {Range} range
+   * @param {string} key
-   * @return {Array}
+   * @param {number} offset
+   * @return {OrderedSet}
    */
 
-  getActiveMarksAtRangeAsArray(range) {
+  getMarksAtPosition(key, offset) {
-    range = range.normalize(this)
+    if (offset == 0) {
-    if (range.isUnset) return []
+      const previous = this.getPreviousText(key)
-    if (range.isCollapsed) return this.getMarksAtCollapsedRangeAsArray(range)
+      if (!previous || previous.text.length == 0) return OrderedSet()
+      if (this.getClosestBlock(key) !== this.getClosestBlock(previous.key)) {
+        return OrderedSet()
+      }
 
-    // Otherwise, get a set of the marks for each character in the range.
+      const char = previous.characters.last()
-    const chars = this.getCharactersAtRange(range)
+      if (!char) return OrderedSet()
-    const first = chars.first()
+      return new OrderedSet(char.marks)
-    if (!first) return []
+    }
 
-    let memo = first.marks
+    const text = this.getDescendant(key)
-
+    const char = text.characters.get(offset - 1)
-    chars.slice(1).forEach(char => {
+    if (!char) return OrderedSet()
-      const marks = char ? char.marks : []
+    return new OrderedSet(char.marks)
-      memo = memo.intersect(marks)
-      return memo.size != 0
-    })
-
-    return memo.toArray()
   }
 
   /**
@@ -1622,8 +1633,33 @@ class Node {
    */
 
   getTextsAtRange(range) {
-    const array = this.getTextsAtRangeAsArray(range)
+    range = range.normalize(this)
-    return new List(array)
+    if (range.isUnset) return List()
+    const { startKey, endKey } = range
+    return new List(this.getTextsBetweenPositionsAsArray(startKey, endKey))
+  }
+
+  /**
+   * Get all of the text nodes in a `range` as an array.
+   * PERF: use key in arguments for cache
+   *
+   * @param {string} startKey
+   * @param {string} endKey
+   * @returns {Array}
+   */
+
+  getTextsBetweenPositionsAsArray(startKey, endKey) {
+    const startText = this.getDescendant(startKey)
+
+    // PERF: the most common case is when the range is in a single text node,
+    // where we can avoid a lot of iterating of the tree.
+    if (startKey == endKey) return [startText]
+
+    const endText = this.getDescendant(endKey)
+    const texts = this.getTextsAsArray()
+    const start = texts.indexOf(startText)
+    const end = texts.indexOf(endText, start)
+    return texts.slice(start, end + 1)
   }
 
   /**
@@ -1636,19 +1672,8 @@ class Node {
   getTextsAtRangeAsArray(range) {
     range = range.normalize(this)
     if (range.isUnset) return []
-
     const { startKey, endKey } = range
-    const startText = this.getDescendant(startKey)
+    return this.getTextsBetweenPositionsAsArray(startKey, endKey)
-
-    // PERF: the most common case is when the range is in a single text node,
-    // where we can avoid a lot of iterating of the tree.
-    if (startKey == endKey) return [startText]
-
-    const endText = this.getDescendant(endKey)
-    const texts = this.getTextsAsArray()
-    const start = texts.indexOf(startText)
-    const end = texts.indexOf(endText)
-    return texts.slice(start, end + 1)
   }
 
   /**
@@ -2050,13 +2075,10 @@ function assertKey(arg) {
 
 memoize(Node.prototype, [
   'areDescendantsSorted',
-  'getActiveMarksAtRangeAsArray',
   'getAncestors',
   'getBlocksAsArray',
   'getBlocksAtRangeAsArray',
   'getBlocksByTypeAsArray',
-  'getCharactersAtRangeAsArray',
-  'getCharactersAsArray',
   'getChild',
   'getClosestBlock',
   'getClosestInline',
@@ -2076,8 +2098,9 @@ memoize(Node.prototype, [
   'getInlinesAtRangeAsArray',
   'getInlinesByTypeAsArray',
   'getMarksAsArray',
-  'getMarksAtRangeAsArray',
+  'getMarksAtPosition',
-  'getInsertMarksAtRangeAsArray',
+  'getOrderedMarksBetweenPositions',
+  'getInsertMarksAtRange',
   'getKeysAsArray',
   'getLastText',
   'getMarksByTypeAsArray',
@@ -2098,7 +2121,7 @@ memoize(Node.prototype, [
   'getTextAtOffset',
   'getTextDirection',
   'getTextsAsArray',
-  'getTextsAtRangeAsArray',
+  'getTextsBetweenPositionsAsArray',
   'isLeafBlock',
   'isLeafInline',
   'validate',

packages/slate/src/models/text.js

@@ -296,6 +296,66 @@ class Text extends Record(DEFAULTS) {
     return leaves
   }
 
+  /**
+   * Get all of the active marks on between two offsets
+   *
+   * @return {Set<Mark>}
+   */
+
+  getActiveMarksBetweenOffsets(startOffset, endOffset) {
+    if (startOffset === 0 && endOffset === this.characters.size) {
+      return this.getActiveMarks()
+    }
+    const startCharacter = this.characters.get(startOffset)
+    if (!startCharacter) return Set()
+    const result = startCharacter.marks
+    if (result.size === 0) return result
+    return result.withMutations(x => {
+      for (let index = startOffset + 1; index < endOffset; index++) {
+        const c = this.characters.get(index)
+        x.intersect(c.marks)
+        if (x.size === 0) return
+      }
+    })
+  }
+
+  /**
+   * Get all of the active marks on the text
+   *
+   * @return {Set<Mark>}
+   */
+
+  getActiveMarks() {
+    if (this.characters.size === 0) return Set()
+    const result = this.characters.first().marks
+    if (result.size === 0) return result
+
+    return result.withMutations(x => {
+      this.characters.forEach(c => {
+        x.intersect(c.marks)
+        if (x.size === 0) return false
+      })
+    })
+  }
+
+  /**
+   * Get all of the marks on between two offsets
+   *
+   * @return {OrderedSet<Mark>}
+   */
+
+  getMarksBetweenOffsets(startOffset, endOffset) {
+    if (startOffset === 0 && endOffset === this.characters.size) {
+      return this.getMarks()
+    }
+    return new OrderedSet().withMutations(result => {
+      for (let index = startOffset; index < endOffset; index++) {
+        const c = this.characters.get(index)
+        result.union(c.marks)
+      }
+    })
+  }
+
   /**
    * Get all of the marks on the text.
    *
@@ -314,9 +374,19 @@ class Text extends Record(DEFAULTS) {
    */
 
   getMarksAsArray() {
-    return this.characters.reduce((array, char) => {
+    if (this.characters.size === 0) return []
-      return array.concat(char.marks.toArray())
+    const first = this.characters.first().marks
-    }, [])
+    let previousMark = first
+    const result = []
+    this.characters.forEach(c => {
+      // If the character marks is the same with the
+      // previous characters, we do not need to
+      // add the marks twice
+      if (c.marks === previousMark) return true
+      previousMark = c.marks
+      result.push(previousMark.toArray())
+    })
+    return Array.prototype.concat.apply(first.toArray(), result)
   }
 
   /**
@@ -519,14 +589,13 @@ Text.prototype[MODEL_TYPES.TEXT] = true
  * Memoize read methods.
  */
 
-memoize(Text.prototype, ['getMarks', 'getMarksAsArray'], {
-  takesArguments: false,
-})
-
 memoize(Text.prototype, [
   'getDecoratedCharacters',
   'getDecorations',
   'getLeaves',
+  'getActiveMarks',
+  'getMarks',
+  'getMarksAsArray',
   'getMarksAtIndex',
   'validate',
 ])

packages/slate/src/utils/is-index-in-range.js

@@ -1,30 +0,0 @@
-/**
- * Check if an `index` of a `text` node is in a `range`.
- *
- * @param {Number} index
- * @param {Text} text
- * @param {Range} range
- * @return {Boolean}
- */
-
-function isIndexInRange(index, text, range) {
-  const { startKey, startOffset, endKey, endOffset } = range
-
-  if (text.key == startKey && text.key == endKey) {
-    return startOffset <= index && index < endOffset
-  } else if (text.key == startKey) {
-    return startOffset <= index
-  } else if (text.key == endKey) {
-    return index < endOffset
-  } else {
-    return true
-  }
-}
-
-/**
- * Export.
- *
- * @type {Function}
- */
-
-export default isIndexInRange