Merge pull request #739 from atom-minimap/decoration-management

Decoration Management Optimizations

Author: aminya

lib/mixins/decoration-management.js

@@ -31,41 +31,41 @@ export default class DecorationManagement {
      * @type {Object}
      * @access private
      */
-    this.decorationsById = {}
+    this.decorationsById = new Map()
     /**
      * The decorations stored in an array indexed with their marker id.
      * @type {Object}
      * @access private
      */
-    this.decorationsByMarkerId = {}
+    this.decorationsByMarkerId = new Map()
     /**
      * The subscriptions to the markers `did-change` event indexed using the
      * marker id.
      * @type {Object}
      * @access private
      */
-    this.decorationMarkerChangedSubscriptions = {}
+    this.decorationMarkerChangedSubscriptions = new Map()
     /**
      * The subscriptions to the markers `did-destroy` event indexed using the
      * marker id.
      * @type {Object}
      * @access private
      */
-    this.decorationMarkerDestroyedSubscriptions = {}
+    this.decorationMarkerDestroyedSubscriptions = new Map()
     /**
      * The subscriptions to the decorations `did-change-properties` event
      * indexed using the decoration id.
      * @type {Object}
      * @access private
      */
-    this.decorationUpdatedSubscriptions = {}
+    this.decorationUpdatedSubscriptions = new Map()
     /**
      * The subscriptions to the decorations `did-destroy` event indexed using
      * the decoration id.
      * @type {Object}
      * @access private
      */
-    this.decorationDestroyedSubscriptions = {}
+    this.decorationDestroyedSubscriptions = new Map()
   }
 
   /**
@@ -74,12 +74,7 @@ export default class DecorationManagement {
    * @return {Array<Decoration>} all the decorations in this `Minimap`
    */
   getDecorations () {
-    const decorations = this.decorationsById
-    const results = []
-
-    for (const id in decorations) { results.push(decorations[id]) }
-
-    return results
+    return this.decorationsById.values()
   }
 
   /**
@@ -173,7 +168,7 @@ export default class DecorationManagement {
    * @return {Decoration} the decoration with the given id
    */
   decorationForId (id) {
-    return this.decorationsById[id]
+    return this.decorationsById.get(id)
   }
 
   /**
@@ -192,9 +187,9 @@ export default class DecorationManagement {
 
     for (let i = 0, len = markers.length; i < len; i++) {
       const marker = markers[i]
-      const decorations = this.decorationsByMarkerId[marker.id]
+      const decorations = this.decorationsByMarkerId.get(marker.id)
 
-      if (decorations != null) {
+      if (decorations !== undefined) {
         decorationsByMarkerId[marker.id] = decorations
       }
     }
@@ -235,8 +230,10 @@ export default class DecorationManagement {
     }
 
     const cache = {}
-    for (const id in this.decorationsById) {
-      const decoration = this.decorationsById[id]
+
+    const decorations = this.decorationsById.values()
+    for (const decoration of decorations) {
+
       const range = decoration.marker.getScreenRange()
       const type = decoration.getProperties().type
 
@@ -341,22 +338,22 @@ export default class DecorationManagement {
       decorationParams.scope = `.minimap .${cls}`
     }
 
-    if (this.decorationMarkerDestroyedSubscriptions[id] == null) {
-      this.decorationMarkerDestroyedSubscriptions[id] =
+    if (!this.decorationMarkerDestroyedSubscriptions.has(id)) {
+      this.decorationMarkerDestroyedSubscriptions.set(id,
       marker.onDidDestroy(() => {
         this.removeAllDecorationsForMarker(marker)
-      })
+      }))
     }
 
-    if (this.decorationMarkerChangedSubscriptions[id] == null) {
-      this.decorationMarkerChangedSubscriptions[id] =
+    if (!this.decorationMarkerChangedSubscriptions.has(id)) {
+      this.decorationMarkerChangedSubscriptions.set(id,
       marker.onDidChange((event) => {
-        const decorations = this.decorationsByMarkerId[id]
+        const decorations = this.decorationsByMarkerId.get(id)
         const screenRange = marker.getScreenRange()
 
         this.invalidateDecorationForScreenRowsCache()
 
-        if (decorations != null) {
+        if (decorations !== undefined) {
           for (let i = 0, len = decorations.length; i < len; i++) {
             const decoration = decorations[i]
             this.emitter.emit('did-change-decoration', {
@@ -393,29 +390,29 @@ export default class DecorationManagement {
             end: end
           }, 0)
         }
-      })
+      }))
     }
 
     const decoration = new Decoration(marker, this, decorationParams)
 
-    if (this.decorationsByMarkerId[id] == null) {
-      this.decorationsByMarkerId[id] = []
+    if (!this.decorationsByMarkerId.has(id)) {
+      this.decorationsByMarkerId.set(id, [])
     }
 
-    this.decorationsByMarkerId[id].push(decoration)
-    this.decorationsById[decoration.id] = decoration
+    this.decorationsByMarkerId.get(id).push(decoration)
+    this.decorationsById.set(decoration.id, decoration)
 
-    if (this.decorationUpdatedSubscriptions[decoration.id] == null) {
-      this.decorationUpdatedSubscriptions[decoration.id] =
+    if (!this.decorationUpdatedSubscriptions.has(decoration.id)) {
+      this.decorationUpdatedSubscriptions.set(decoration.id,
       decoration.onDidChangeProperties((event) => {
         this.emitDecorationChanges(type, decoration)
-      })
+      }))
     }
 
-    this.decorationDestroyedSubscriptions[decoration.id] =
+    this.decorationDestroyedSubscriptions.set(decoration.id,
     decoration.onDidDestroy(() => {
       this.removeDecoration(decoration)
-    })
+    }))
 
     this.emitDecorationChanges(type, decoration)
     this.emitter.emit('did-add-decoration', {
@@ -527,19 +524,19 @@ export default class DecorationManagement {
     const marker = decoration.marker
     let subscription
 
-    delete this.decorationsById[decoration.id]
+    this.decorationsById.delete(decoration.id)
 
-    subscription = this.decorationUpdatedSubscriptions[decoration.id]
-    if (subscription != null) { subscription.dispose() }
+    subscription = this.decorationUpdatedSubscriptions.get(decoration.id)
+    if (subscription !== undefined) { subscription.dispose() }
 
-    subscription = this.decorationDestroyedSubscriptions[decoration.id]
-    if (subscription != null) { subscription.dispose() }
+    subscription = this.decorationDestroyedSubscriptions.get(decoration.id)
+    if (subscription !== undefined) { subscription.dispose() }
 
-    delete this.decorationUpdatedSubscriptions[decoration.id]
-    delete this.decorationDestroyedSubscriptions[decoration.id]
+    this.decorationUpdatedSubscriptions.delete(decoration.id)
+    this.decorationDestroyedSubscriptions.delete(decoration.id)
 
-    const decorations = this.decorationsByMarkerId[marker.id]
-    if (!decorations) { return }
+    const decorations = this.decorationsByMarkerId.get(marker.id)
+    if (decorations === undefined) { return }
 
     this.emitDecorationChanges(decoration.getProperties().type, decoration)
 
@@ -568,8 +565,8 @@ export default class DecorationManagement {
   removeAllDecorationsForMarker (marker) {
     if (marker == null) { return }
 
-    const decorations = this.decorationsByMarkerId[marker.id]
-    if (!decorations) { return }
+    const decorations = this.decorationsByMarkerId.get(marker.id)
+    if (decorations === undefined) { return }
 
     for (let i = 0, len = decorations.length; i < len; i++) {
       const decoration = decorations[i]
@@ -595,43 +592,48 @@ export default class DecorationManagement {
   removedAllMarkerDecorations (marker) {
     if (marker == null) { return }
 
-    this.decorationMarkerChangedSubscriptions[marker.id].dispose()
-    this.decorationMarkerDestroyedSubscriptions[marker.id].dispose()
+    this.decorationMarkerChangedSubscriptions.get(marker.id).dispose()
+    this.decorationMarkerDestroyedSubscriptions.get(marker.id).dispose()
 
-    delete this.decorationsByMarkerId[marker.id]
-    delete this.decorationMarkerChangedSubscriptions[marker.id]
-    delete this.decorationMarkerDestroyedSubscriptions[marker.id]
+    this.decorationsByMarkerId.delete(marker.id)
+    this.decorationMarkerChangedSubscriptions.delete(marker.id)
+    this.decorationMarkerDestroyedSubscriptions.delete(marker.id)
   }
 
   /**
    * Removes all the decorations that was created in the current `Minimap`.
    */
   removeAllDecorations () {
-    for (const id in this.decorationMarkerChangedSubscriptions) {
-      this.decorationMarkerChangedSubscriptions[id].dispose()
+    const decorationMarkerChangedSubscriptionsValues = this.decorationMarkerChangedSubscriptions.values()
+    for (const decoration of decorationMarkerChangedSubscriptionsValues) {
+      decoration.dispose()
     }
 
-    for (const id in this.decorationMarkerDestroyedSubscriptions) {
-      this.decorationMarkerDestroyedSubscriptions[id].dispose()
+    const decorationMarkerDestroyedSubscriptionsValues = this.decorationMarkerDestroyedSubscriptions.values()
+    for (const decoration of decorationMarkerDestroyedSubscriptionsValues) {
+      decoration.dispose()
     }
 
-    for (const id in this.decorationUpdatedSubscriptions) {
-      this.decorationUpdatedSubscriptions[id].dispose()
+    const decorationUpdatedSubscriptionsValues = this.decorationUpdatedSubscriptions.values()
+    for (const decoration of decorationUpdatedSubscriptionsValues) {
+      decoration.dispose()
     }
 
-    for (const id in this.decorationDestroyedSubscriptions) {
-      this.decorationDestroyedSubscriptions[id].dispose()
+    const decorationDestroyedSubscriptionsValues = this.decorationDestroyedSubscriptions.values()
+    for (const decoration of decorationDestroyedSubscriptionsValues) {
+      decoration.dispose()
     }
 
-    for (const id in this.decorationsById) {
-      this.decorationsById[id].destroy()
+    const decorationsByIdValues = this.decorationsById.values()
+    for (const decoration of decorationsByIdValues) {
+      decoration.destroy()
     }
 
-    this.decorationsById = {}
-    this.decorationsByMarkerId = {}
-    this.decorationMarkerChangedSubscriptions = {}
-    this.decorationMarkerDestroyedSubscriptions = {}
-    this.decorationUpdatedSubscriptions = {}
-    this.decorationDestroyedSubscriptions = {}
+    this.decorationsById.clear()
+    this.decorationsByMarkerId.clear()
+    this.decorationMarkerChangedSubscriptions.clear()
+    this.decorationMarkerDestroyedSubscriptions.clear()
+    this.decorationUpdatedSubscriptions.clear()
+    this.decorationDestroyedSubscriptions.clear()
   }
 }

spec/minimap-spec.js

@@ -336,7 +336,7 @@ describe('Minimap', () => {
     })
 
     it('creates a decoration for the given marker', () => {
-      expect(minimap.decorationsByMarkerId[marker.id]).toBeDefined()
+      expect(minimap.decorationsByMarkerId.get(marker.id)).toBeDefined()
     })
 
     it('creates a change corresponding to the marker range', () => {
@@ -367,7 +367,7 @@ describe('Minimap', () => {
       })
 
       it('removes the decoration from the render view', () => {
-        expect(minimap.decorationsByMarkerId[marker.id]).toBeUndefined()
+        expect(minimap.decorationsByMarkerId.get(marker.id)).toBeUndefined()
       })
 
       it('creates a change corresponding to the marker range', () => {
@@ -382,7 +382,7 @@ describe('Minimap', () => {
       })
 
       it('removes the decoration from the render view', () => {
-        expect(minimap.decorationsByMarkerId[marker.id]).toBeUndefined()
+        expect(minimap.decorationsByMarkerId.get(marker.id)).toBeUndefined()
       })
 
       it('creates a change corresponding to the marker range', () => {
@@ -397,7 +397,7 @@ describe('Minimap', () => {
       })
 
       it('removes the decoration from the render view', () => {
-        expect(minimap.decorationsByMarkerId[marker.id]).toBeUndefined()
+        expect(minimap.decorationsByMarkerId.get(marker.id)).toBeUndefined()
       })
 
       it('creates a change corresponding to the marker range', () => {
@@ -412,8 +412,8 @@ describe('Minimap', () => {
       })
 
       it('removes all the previously added decorations', () => {
-        expect(minimap.decorationsById).toEqual({})
-        expect(minimap.decorationsByMarkerId).toEqual({})
+        expect(minimap.decorationsById.size).toEqual(0)
+        expect(minimap.decorationsByMarkerId.size).toEqual(0)
       })
 
       it('prevents the creation of new decorations', () => {