Fix sorting (#77)

packages/common/src/SortedCollection.ts

@@ -0,0 +1,185 @@
+import { Maybe, Opaque } from './helpers';
+
+export type Compare<T> = (a: T, b: T) => number;
+
+/**
+ * Insert an item into a sorted array using binary search.
+ *
+ * @type   {T}                item    type
+ * @param  {T}                item    to be inserted
+ * @param  {Array<T>}         array   to be modified
+ * @param  {(a, b) => number} compare function
+ *
+ * @return {number}                   insertion index
+ */
+export function sortedInsert<T>(item: T, into: Array<T>, compare: Compare<T>): number {
+  if (into.length === 0) {
+    into.push(item);
+
+    return 0;
+  }
+
+  let min = 0;
+  let max = into.length - 1;
+
+  while (min !== max) {
+    const guess = (min + max) / 2 | 0;
+
+    if (compare(item, into[guess]) < 0) {
+      max = Math.max(min, guess - 1);
+    } else {
+      min = Math.min(max, guess + 1);
+    }
+  }
+
+  const insert = compare(item, into[min]) <= 0 ? min : min + 1;
+
+  into.splice(insert, 0, item);
+
+  return insert;
+}
+
+/**
+ * Find an index of an element within a sorted array. This should be substantially
+ * faster than `indexOf` for large arrays.
+ *
+ * @type  {T}                item    type
+ * @param {T}                item    to find
+ * @param {Array<T>}         array   to look through
+ * @param {(a, b) => number} compare function
+ *
+ * @return {number}                  index of the element, `-1` if not found
+ */
+export function sortedIndexOf<T>(item:T, within: Array<T>, compare: Compare<T>): number {
+  if (within.length === 0) {
+    return -1;
+  }
+
+  let min = 0;
+  let max = within.length - 1;
+
+  while (min !== max) {
+    let guess = (min + max) / 2 | 0;
+    const other = within[guess];
+
+    if (item === other) {
+      return guess;
+    }
+
+    const result = compare(item, other);
+
+    if (result < 0) {
+      max = Math.max(min, guess - 1);
+    } else if (result > 0) {
+      min = Math.min(max, guess + 1);
+    } else {
+      // Equal sort value, but different reference, do value search from min
+      return within.indexOf(item, min);
+    }
+  }
+
+  if (item === within[min]) {
+    return min;
+  }
+
+  return -1;
+}
+
+export namespace SortedCollection {
+  export type StateRef = Opaque<number, 'SortedCollection.StateRef'>;
+}
+
+export class SortedCollection<Id, Item extends { id: Id }> {
+  private readonly map = new Map<Id, Item>();
+  private readonly compare: Compare<Item>;
+
+  private list = Array<Item>();
+  private changeRef = 0;
+
+  constructor(compare: Compare<Item>) {
+    this.compare = compare;
+  }
+
+  public ref(): SortedCollection.StateRef {
+    return this.changeRef as SortedCollection.StateRef;
+  }
+
+  public add(item: Item) {
+    this.map.set(item.id, item);
+    sortedInsert(item, this.list, this.compare);
+
+    this.changeRef += 1;
+  }
+
+  public remove(id: Id) {
+    const item = this.map.get(id);
+
+    if (!item) {
+      return;
+    }
+
+    const index = sortedIndexOf(item, this.list, this.compare);
+    this.list.splice(index, 1);
+    this.map.delete(id);
+
+    this.changeRef += 1;
+  }
+
+  public get(id: Id): Maybe<Item> {
+    return this.map.get(id);
+  }
+
+  public sorted(): Array<Item> {
+    return this.list;
+  }
+
+  public mut(id: Id, mutator: (item: Item) => void) {
+    const item = this.map.get(id);
+
+    if (!item) {
+      return;
+    }
+
+    mutator(item);
+  }
+
+  public mutAndSort(id: Id, mutator: (item: Item) => void) {
+    const item = this.map.get(id);
+
+    if (!item) {
+      return;
+    }
+
+    const index = sortedIndexOf(item, this.list, this.compare);
+
+    mutator(item);
+
+    this.list.splice(index, 1);
+
+    const newIndex = sortedInsert(item, this.list, this.compare);
+
+    if (newIndex !== index) {
+      this.changeRef += 1;
+    }
+  }
+
+  public mutEach(mutator: (item: Item) => void) {
+    this.list.forEach(mutator);
+  }
+
+  public mutEachAndSort(mutator: (item: Item) => void) {
+    this.list.forEach(mutator);
+    this.list.sort(this.compare);
+  }
+
+  public clear() {
+    this.map.clear();
+    this.list = [];
+
+    this.changeRef += 1;
+  }
+
+  public hasChangedSince(ref: SortedCollection.StateRef): boolean {
+    return this.changeRef > ref;
+  }
+}

packages/common/src/helpers.ts

@@ -112,81 +112,3 @@ export class NumStats<T extends number> {
     return this.index < this.history ? this.stack.slice(0, this.index) : this.stack;
   }
 }
-
-/**
- * Insert an item into a sorted array using binary search.
- *
- * @type   {T}                item    type
- * @param  {T}                item    to be inserted
- * @param  {Array<T>}         array   to be modified
- * @param  {(a, b) => number} compare function
- *
- * @return {number}                   insertion index
- */
-export function sortedInsert<T>(item: T, into: Array<T>, compare: (a: T, b: T) => number): number {
-  if (into.length === 0) {
-    into.push(item);
-
-    return 0;
-  }
-
-  let min = 0;
-  let max = into.length - 1;
-
-  while (min !== max) {
-    const guess = (min + max) / 2 | 0;
-
-    if (compare(item, into[guess]) < 0) {
-      max = Math.max(min, guess - 1);
-    } else {
-      min = Math.min(max, guess + 1);
-    }
-  }
-
-  let insert = compare(item, into[min]) <= 0 ? min : min + 1;
-
-  into.splice(insert, 0, item);
-
-  return insert;
-}
-
-/**
- * Find an index of an element within a sorted array. This should be substantially
- * faster than `indexOf` for large arrays.
- *
- * @type  {T}                item    type
- * @param {T}                item    to find
- * @param {Array<T>}         array   to look through
- * @param {(a, b) => number} compare function
- *
- * @return {number}                  index of the element, `-1` if not found
- */
-export function sortedIndexOf<T>(item:T, within: Array<T>, compare: (a: T, b: T) => number): number {
-  if (within.length === 0) {
-    return -1;
-  }
-
-  let min = 0;
-  let max = within.length - 1;
-
-  while (min !== max) {
-    const guess = (min + max) / 2 | 0;
-    const other = within[guess];
-
-    if (item === other) {
-      return guess;
-    }
-
-    if (compare(item, other) < 0) {
-      max = Math.max(min, guess - 1);
-    } else {
-      min = Math.min(max, guess + 1);
-    }
-  }
-
-  if (item === within[min]) {
-    return min;
-  }
-
-  return -1;
-}

packages/common/src/index.ts

@@ -1,6 +1,7 @@
 export * from './helpers';
 export * from './id';
 export * from './stringify';
+export * from './SortedCollection';
 
 import * as Types from './types';
 import * as FeedMessage from './feed';

packages/common/test/index.js

@@ -74,21 +74,20 @@ test('sortedInsert indexes', (assert) => {
 
 test('sortedIndexOf', (assert) => {
   const { sortedIndexOf } = common;
-  const cmp = (a, b) => a - b;
+  const cmp = (a, b) => a.value - b.value;
+  const array = [];
 
-  assert.equals(sortedIndexOf(1, [1,2,3,4,5,6,7,8,9], cmp), 0, 'Found 1');
+  for (let i = 1; i <= 1000; i++) {
-  assert.equals(sortedIndexOf(2, [1,2,3,4,5,6,7,8,9], cmp), 1, 'Found 2');
+    array.push({ value: i >> 1 });
-  assert.equals(sortedIndexOf(3, [1,2,3,4,5,6,7,8,9], cmp), 2, 'Found 3');
+  }
-  assert.equals(sortedIndexOf(4, [1,2,3,4,5,6,7,8,9], cmp), 3, 'Found 4');
-  assert.equals(sortedIndexOf(5, [1,2,3,4,5,6,7,8,9], cmp), 4, 'Found 5');
-  assert.equals(sortedIndexOf(6, [1,2,3,4,5,6,7,8,9], cmp), 5, 'Found 6');
-  assert.equals(sortedIndexOf(7, [1,2,3,4,5,6,7,8,9], cmp), 6, 'Found 7');
-  assert.equals(sortedIndexOf(8, [1,2,3,4,5,6,7,8,9], cmp), 7, 'Found 8');
-  assert.equals(sortedIndexOf(9, [1,2,3,4,5,6,7,8,9], cmp), 8, 'Found 9');
 
-  assert.equals(sortedIndexOf(0, [1,2,3,4,5,6,7,8,9], cmp), -1, 'No 0');
+  for (let i = 0; i < 50; i++) {
-  assert.equals(sortedIndexOf(10, [1,2,3,4,5,6,7,8,9], cmp), -1, 'No 10');
+    let index = Math.random() * 1000 | 0;
-  assert.equals(sortedIndexOf(5.5, [1,2,3,4,5,6,7,8,9], cmp), -1, 'No 5.5');
+
+    item = array[index];
+
+    assert.equals(sortedIndexOf(item, array, cmp), array.indexOf(item), `Correct for ${item.value}`);
+  }
 
   assert.end();
 });

packages/frontend/src/App.tsx

@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { Types } from '@dotstats/common';
+import { Types, SortedCollection } from '@dotstats/common';
 import { Chains, Chain, Ago, OfflineIndicator } from './components';
 import { Connection } from './Connection';
 import { PersistentObject, PersistentSet } from './persist';
@@ -36,13 +36,11 @@ export default class App extends React.Component<{}, State> {
     );
 
     this.pins = new PersistentSet<Types.NodeName>('pinned_names', (pins) => {
-      const { nodes, sortedNodes } = this.state;
+      const { nodes } = this.state;
 
-      for (const node of nodes.values()) {
+      nodes.mutEachAndSort((node) => node.setPinned(pins.has(node.name)));
-        node.setPinned(pins.has(node.name));
-      }
 
-      this.setState({ nodes, pins, sortedNodes: sortedNodes.sort(Node.compare) });
+      this.setState({ nodes, pins });
     });
 
     this.state = {
@@ -53,8 +51,7 @@ export default class App extends React.Component<{}, State> {
       timeDiff: 0 as Types.Milliseconds,
       subscribed: null,
       chains: new Map(),
-      nodes: new Map(),
+      nodes: new SortedCollection(Node.compare),
-      sortedNodes: [],
       settings: this.settings.raw(),
       pins: this.pins.get(),
     };

packages/frontend/src/Connection.ts

@@ -1,5 +1,4 @@
 import { VERSION, timestamp, FeedMessage, Types, Maybe, sleep } from '@dotstats/common';
-import { sortedInsert, sortedIndexOf } from '@dotstats/common';
 import { State, Update, Node } from './state';
 import { PersistentSet } from './persist';
 import { getHashData, setHashData } from './utils';
@@ -83,10 +82,7 @@ export class Connection {
 
   public handleMessages = (messages: FeedMessage.Message[]) => {
     const { nodes, chains } = this.state;
-    let { sortedNodes } = this.state;
+    const ref = nodes.ref();
-
-    // TODO: boolean flags are code smell, find a cleaner way to do this
-    let dirty = false;
 
     for (const message of messages) {
       switch (message.action) {
@@ -107,7 +103,7 @@ export class Connection {
         case Actions.BestBlock: {
           const [best, blockTimestamp, blockAverage] = message.payload;
 
-          nodes.forEach((node) => node.newBestBlock());
+          nodes.mutEach((node) => node.newBestBlock());
 
           this.state = this.update({ best, blockTimestamp, blockAverage });
 
@@ -119,90 +115,47 @@ export class Connection {
           const pinned = this.pins.has(nodeDetails[0]);
           const node = new Node(pinned, id, nodeDetails, nodeStats, nodeHardware, blockDetails, location);
 
-          nodes.set(id, node);
+          nodes.add(node);
-          sortedInsert(node, sortedNodes, Node.compare);
-
-          if (nodes.size !== sortedNodes.length) {
-            console.error('Node count in sorted array is wrong!');
-            sortedNodes = Array.from(nodes.values()).sort(Node.compare);
-          }
-
-          dirty = true;
 
           break;
         }
 
         case Actions.RemovedNode: {
           const id = message.payload;
-          const node = nodes.get(id);
 
-          if (node) {
+          nodes.remove(id);
-            nodes.delete(id);
-            const index = sortedIndexOf(node, sortedNodes, Node.compare);
-            sortedNodes.splice(index, 1);
-
-            if (nodes.size !== sortedNodes.length) {
-              console.error('Node count in sorted array is wrong!');
-              sortedNodes = Array.from(nodes.values()).sort(Node.compare);
-            }
-          }
-
-          dirty = true;
 
           break;
         }
 
         case Actions.LocatedNode: {
           const [id, lat, lon, city] = message.payload;
-          const node = nodes.get(id);
 
-          if (!node) {
+          nodes.mut(id, (node) => node.updateLocation([lat, lon, city]));
-            break;
-          }
-
-          node.updateLocation([lat, lon, city]);
 
           break;
         }
 
         case Actions.ImportedBlock: {
           const [id, blockDetails] = message.payload;
-          const node = nodes.get(id);
 
-          if (!node) {
+          nodes.mutAndSort(id, (node) => node.updateBlock(blockDetails));
-            break;
-          }
-
-          node.updateBlock(blockDetails);
-          sortedNodes = sortedNodes.sort(Node.compare);
-
-          dirty = true;
 
           break;
         }
 
         case Actions.NodeStats: {
           const [id, nodeStats] = message.payload;
-          const node = nodes.get(id);
 
-          if (!node) {
+          nodes.mut(id, (node) => node.updateStats(nodeStats));
-            break;
-          }
-
-          node.updateStats(nodeStats);
 
           break;
         }
 
         case Actions.NodeHardware: {
           const [id, nodeHardware] = message.payload;
-          const node = nodes.get(id);
 
-          if (!node) {
+          nodes.mut(id, (node) => node.updateHardware(nodeHardware));
-            return;
-          }
-
-          node.updateHardware(nodeHardware);
 
           break;
         }
@@ -219,7 +172,7 @@ export class Connection {
           const [label, nodeCount] = message.payload;
           chains.set(label, nodeCount);
 
-          dirty = true;
+          this.state = this.update({ chains });
 
           break;
         }
@@ -229,22 +182,16 @@ export class Connection {
 
           if (this.state.subscribed === message.payload) {
             nodes.clear();
-            sortedNodes = [];
+            this.state = this.update({ subscribed: null, nodes, chains });
-            this.state = this.update({ subscribed: null, nodes, chains, sortedNodes });
           }
 
-          dirty = true;
-
           break;
         }
 
         case Actions.SubscribedTo: {
           nodes.clear();
-          sortedNodes = [];
 
-          this.state = this.update({ subscribed: message.payload, nodes, sortedNodes });
+          this.state = this.update({ subscribed: message.payload, nodes });
-
-          dirty = true;
 
           break;
         }
@@ -252,11 +199,9 @@ export class Connection {
         case Actions.UnsubscribedFrom: {
           if (this.state.subscribed === message.payload) {
             nodes.clear();
-            sortedNodes = [];
-            this.state = this.update({ subscribed: null, nodes, sortedNodes });
-          }
 
-          dirty = true;
+            this.state = this.update({ subscribed: null, nodes });
+          }
 
           break;
         }
@@ -273,8 +218,8 @@ export class Connection {
       }
     }
 
-    if (dirty) {
+    if (nodes.hasChangedSince(ref)) {
-      this.state = this.update({ nodes, chains, sortedNodes });
+      this.state = this.update({ nodes });
     }
 
     this.autoSubscribe();
@@ -283,9 +228,13 @@ export class Connection {
   private bindSocket() {
     this.ping();
 
+    if (this.state) {
+      const { nodes } = this.state;
+      nodes.clear();
+    }
+
     this.state = this.update({
       status: 'online',
-      nodes: new Map()
     });
 
     if (this.state.subscribed) {

packages/frontend/src/components/Chain/Chain.tsx

@@ -201,7 +201,7 @@ export class Chain extends React.Component<Chain.Props, Chain.State> {
   }
 
   private nodes(): NodeState[] {
-    return this.props.appState.sortedNodes;
+    return this.props.appState.nodes.sorted();
   }
 
   private pixelPosition(lat: Types.Latitude, lon: Types.Longitude): Node.Location.Position {

packages/frontend/src/components/Sparkline.css

@@ -1,6 +1,6 @@
 .Sparkline {
-  fill: currentcolor; /* rgba(255,255,255,0.5); */
+  fill: currentcolor;
-  fill-opacity: 0.5;
+  fill-opacity: 0.35;
   stroke: currentcolor;
   margin: 0 -1px -3px -1px;
 }

packages/frontend/src/state.ts

@@ -1,4 +1,4 @@
-import { Types, Maybe } from '@dotstats/common';
+import { Types, Maybe, SortedCollection } from '@dotstats/common';
 
 export class Node {
   public static compare(a: Node, b: Node): number {
@@ -166,8 +166,7 @@ export interface State {
   timeDiff: Types.Milliseconds;
   subscribed: Maybe<Types.ChainLabel>;
   chains: Map<Types.ChainLabel, Types.NodeCount>;
-  nodes: Map<Types.NodeId, Node>;
+  nodes: SortedCollection<Types.NodeId, Node>;
-  sortedNodes: Node[];
   settings: Readonly<State.Settings>;
   pins: Readonly<Set<Types.NodeName>>;
 }

packages/frontend/test/Connection.spec.ts

@@ -3,7 +3,7 @@ const { shallow, mount } = Enzyme;
 
 import { Server } from 'mock-socket';
 
-import { Types, FeedMessage, timestamp, VERSION } from '../../common';
+import { Types, FeedMessage, timestamp, VERSION, SortedCollection } from '../../common';
 
 import { Node, Update, State } from '../src/state';
 import { Connection } from '../src/Connection';
@@ -54,8 +54,7 @@ describe('Connection.ts', () => {
       timeDiff: 0 as Types.Milliseconds,
       subscribed: null,
       chains: new Map(),
-      nodes: new Map(),
+      nodes: new SortedCollection(Node.compare),
-      sortedNodes: [],
       settings,
       pins: new Set()
     } as State;
@@ -133,13 +132,7 @@ describe('Connection.ts', () => {
       expect(state.status).toBe('online');
       expect(state.nodes).toBeDefined();
 
-      const nodes = [];
+      const firstNode = state.nodes.sorted()[0];
-
-      for (const node of state.nodes.values()) {
-        nodes.push(node);
-      }
-
-      const firstNode = nodes[0];
 
       expect(firstNode.id).toBe(1);
       expect(firstNode.name).toBe('Sample Node');
@@ -172,13 +165,7 @@ describe('Connection.ts', () => {
 
       expect(update).toHaveBeenCalled();
 
-      const nodes = [];
+      const firstNode = state.nodes.sorted()[0];
-
-      for (const node of state.nodes.values()) {
-        nodes.push(node);
-      }
-
-      const firstNode = nodes[0];
 
       expect(firstNode.lat).toEqual(30.828)
       expect(firstNode.lon).toEqual(101.4111)
@@ -193,13 +180,7 @@ describe('Connection.ts', () => {
         }
       ] as any as FeedMessage.Message[]);
 
-      const nodes = [];
+      const firstNode = state.nodes.sorted()[0];
-
-      for (const node of state.nodes.values()) {
-        nodes.push(node);
-      }
-
-      const firstNode = nodes[0];
 
       expect(firstNode.blockTimestamp).toBe(time);
     })
@@ -216,15 +197,7 @@ describe('Connection.ts', () => {
         }
       ] as any as FeedMessage.Message[]);
 
-      expect(state.sortedNodes).toBeDefined();
+      const firstSortedNode = state.nodes.sorted()[0];
-
-      const sortedNodes = [];
-
-      for (const node of state.sortedNodes) {
-        sortedNodes.push(node);
-      }
-
-      const firstSortedNode = sortedNodes[0];
 
       expect(firstSortedNode.pinned).toBeFalsy();
       expect(firstSortedNode).toMatchObject({
@@ -261,7 +234,7 @@ describe('Connection.ts', () => {
       ] as any as FeedMessage.Message[]);
 
       expect(update).toHaveBeenCalled();
-      expect(state.nodes.keys()).toMatchObject({});
+      expect(state.nodes.sorted()).toEqual([]);
     })
   })