fix: make circular buffer not recreate an array on each overflow

Author: Bilb

ts/data/data.ts

@@ -392,7 +392,7 @@ async function removeMessage(id: string): Promise<void> {
 
 /**
  * Note: this method will not clean up external files, just delete from SQL.
- * File are cleaned up on app start if they are not linked to any messages
+ * Files are cleaned up on app start if they are not linked to any messages
  *
  */
 async function removeMessagesByIds(ids: Array<string>): Promise<void> {

ts/session/apis/open_group_api/sogsv3/sogsApiV3.ts

@@ -326,7 +326,7 @@ const handleMessagesResponseV4 = async (
             /*
              * When a message is deleted from the server, we get the deleted event as a data: null on the message itself
              * and an update on its reactions.
-             * But, because we just deleted that message, we can skip trying to udpate its reactions: it's not in the DB anymore.
+             * But, because we just deleted that message, we can skip trying to update its reactions: it's not in the DB anymore.
              */
             if (sogsRollingDeletions.hasMessageDeletedId(conversationId, messageWithReaction.id)) {
               continue;

ts/session/apis/open_group_api/sogsv3/sogsRollingDeletions.ts

@@ -13,7 +13,7 @@ const addMessageDeletedId = (conversationId: string, messageDeletedId: number) =
   if (!ringBuffer) {
     return;
   }
-  ringBuffer.add(messageDeletedId);
+  ringBuffer.insert(messageDeletedId);
 };
 
 const hasMessageDeletedId = (conversationId: string, messageDeletedId: number) => {

ts/session/utils/RingBuffer.ts

@@ -4,6 +4,8 @@
  *
  */
 export class RingBuffer<T> {
+  private newest = -1;
+  private oldest = 0;
   private buffer: Array<T> = [];
   private readonly capacity: number;
 
@@ -16,25 +18,59 @@ export class RingBuffer<T> {
   }
 
   public getLength(): number {
-    return this.buffer.length;
+    if (this.isEmpty()) {
+      return 0;
+    }
+
+    // When only one item was added, newest = 0 and oldest = 0.
+    // When more than one item was added, but less than capacity, newest = nbItemsAdded & oldest = 0.
+    // As soon as we overflow, oldest is incremented to oldest+1 and newest rolls back to 0,
+    // so this test fails here and we have to extract the length based on the two parts instead.
+    if (this.newest >= this.oldest) {
+      return this.newest + 1;
+    }
+    const firstPart = this.capacity - this.oldest;
+    const secondPart = this.newest + 1;
+    return firstPart + secondPart;
   }
 
-  public add(item: T) {
-    this.buffer.push(item);
-    this.crop();
+  public insert(item: T) {
+    // see comments in `getLength()`
+    this.newest = (this.newest + 1) % this.capacity;
+    if (this.buffer.length >= this.capacity) {
+      this.oldest = (this.oldest + 1) % this.capacity;
+    }
+    this.buffer[this.newest] = item;
   }
 
   public has(item: T) {
-    return this.buffer.includes(item);
+    // no items at all
+    if (this.isEmpty()) {
+      return false;
+    }
+    return this.toArray().includes(item);
+  }
+
+  public isEmpty() {
+    return this.newest === -1;
   }
 
   public clear() {
     this.buffer = [];
+    this.newest = -1;
+    this.oldest = 0;
   }
 
-  private crop() {
-    while (this.buffer.length > this.capacity) {
-      this.buffer.shift();
+  public toArray(): Array<T> {
+    if (this.isEmpty()) {
+      return [];
+    }
+
+    if (this.newest >= this.oldest) {
+      return this.buffer.slice(0, this.newest + 1);
     }
+    const firstPart = this.buffer.slice(this.oldest, this.capacity);
+    const secondPart = this.buffer.slice(0, this.newest + 1);
+    return [...firstPart, ...secondPart];
   }
 }

ts/test/session/unit/utils/RingBuffer_test.ts

@@ -10,43 +10,80 @@ describe('RingBuffer Utils', () => {
     const ring = new RingBuffer<number>(5000);
     expect(ring.getCapacity()).to.equal(5000);
     expect(ring.getLength()).to.equal(0);
-    expect(ring.has(0)).to.equal(false, '4 should not be there');
+    expect(ring.has(0)).to.equal(false, '0 should not be there');
   });
 
   describe('length & capacity are right', () => {
+    it('length is right 0', () => {
+      const ring = new RingBuffer<number>(4);
+      expect(ring.getLength()).to.equal(0);
+    });
+
     it('length is right 1', () => {
       const ring = new RingBuffer<number>(4);
-      ring.add(0);
+      ring.insert(0);
       expect(ring.getLength()).to.equal(1);
     });
 
     it('length is right 4', () => {
       const ring = new RingBuffer<number>(4);
-      ring.add(0);
-      ring.add(1);
-      ring.add(2);
-      ring.add(3);
+      ring.insert(0);
+      ring.insert(1);
+      ring.insert(2);
+      ring.insert(3);
       expect(ring.getLength()).to.equal(4);
     });
 
     it('capacity does not get exceeded', () => {
       const ring = new RingBuffer<number>(4);
-      ring.add(0);
-      ring.add(1);
-      ring.add(2);
-      ring.add(3);
-      ring.add(4);
+      ring.insert(0);
+      ring.insert(1);
+      ring.insert(2);
+      ring.insert(3);
+      ring.insert(4);
       expect(ring.getLength()).to.equal(4);
     });
   });
 
+  describe('isEmpty is correct', () => {
+    it('no items', () => {
+      const ring = new RingBuffer<number>(4);
+      expect(ring.isEmpty()).to.equal(true, 'no items isEmpty should be true');
+    });
+
+    it('length is right 1', () => {
+      const ring = new RingBuffer<number>(4);
+      ring.insert(0);
+      expect(ring.isEmpty()).to.equal(false, '1 item isEmpty should be false');
+    });
+
+    it('length is right 4', () => {
+      const ring = new RingBuffer<number>(4);
+      ring.insert(0);
+      ring.insert(1);
+      ring.insert(2);
+      ring.insert(3);
+      expect(ring.isEmpty()).to.equal(false, '4 items isEmpty should be false');
+    });
+
+    it('more than capacity', () => {
+      const ring = new RingBuffer<number>(4);
+      ring.insert(0);
+      ring.insert(1);
+      ring.insert(2);
+      ring.insert(3);
+      ring.insert(4);
+      expect(ring.isEmpty()).to.equal(false, '5 item isEmpty should be false');
+    });
+  });
+
   it('items are removed in order 1', () => {
     const ring = new RingBuffer<number>(4);
-    ring.add(0);
-    ring.add(1);
-    ring.add(2);
-    ring.add(3);
-    ring.add(4);
+    ring.insert(0);
+    ring.insert(1);
+    ring.insert(2);
+    ring.insert(3);
+    ring.insert(4);
     expect(ring.has(0)).to.equal(false, '0 should not be there anymore');
     expect(ring.has(1)).to.equal(true, '1 should still be there');
     expect(ring.has(2)).to.equal(true, '2 should still be there');
@@ -56,11 +93,11 @@ describe('RingBuffer Utils', () => {
 
   it('two times the same items can exist', () => {
     const ring = new RingBuffer<number>(4);
-    ring.add(0);
-    ring.add(1);
-    ring.add(2);
-    ring.add(1);
-    ring.add(4);
+    ring.insert(0);
+    ring.insert(1);
+    ring.insert(2);
+    ring.insert(1);
+    ring.insert(4);
     expect(ring.has(0)).to.equal(false, '0 should not be there anymore');
     expect(ring.has(1)).to.equal(true, '1 should still be there');
     expect(ring.has(2)).to.equal(true, '2 should still be there');
@@ -70,14 +107,14 @@ describe('RingBuffer Utils', () => {
 
   it('items are removed in order completely', () => {
     const ring = new RingBuffer<number>(4);
-    ring.add(0);
-    ring.add(1);
-    ring.add(2);
-    ring.add(3);
-    ring.add(10);
-    ring.add(20);
-    ring.add(30);
-    ring.add(40);
+    ring.insert(0);
+    ring.insert(1);
+    ring.insert(2);
+    ring.insert(3);
+    ring.insert(10);
+    ring.insert(20);
+    ring.insert(30);
+    ring.insert(40);
     expect(ring.has(0)).to.equal(false, '0 should not be there anymore');
     expect(ring.has(1)).to.equal(false, '1 should not be there');
     expect(ring.has(2)).to.equal(false, '2 should not be there');
@@ -92,15 +129,96 @@ describe('RingBuffer Utils', () => {
 
   it('clear empties the list but keeps the capacity', () => {
     const ring = new RingBuffer<number>(4);
-    ring.add(0);
-    ring.add(1);
-    ring.add(2);
-    ring.add(1);
+    ring.insert(0);
+    ring.insert(1);
+    ring.insert(2);
+    ring.insert(1);
     expect(ring.getLength()).to.equal(4);
     expect(ring.getCapacity()).to.equal(4);
     ring.clear();
     expect(ring.getCapacity()).to.equal(4);
 
     expect(ring.getLength()).to.equal(0);
   });
+
+  describe('toArray', () => {
+    it('empty buffer', () => {
+      const ring = new RingBuffer<number>(4);
+      expect(ring.toArray()).to.deep.eq([]);
+    });
+
+    it('with 1', () => {
+      const ring = new RingBuffer<number>(4);
+      ring.insert(0);
+
+      expect(ring.toArray()).to.deep.eq([0]);
+    });
+
+    it('with 4', () => {
+      const ring = new RingBuffer<number>(4);
+      ring.insert(0);
+      ring.insert(1);
+      ring.insert(2);
+      ring.insert(3);
+
+      expect(ring.toArray()).to.deep.eq([0, 1, 2, 3]);
+    });
+
+    it('with 5', () => {
+      const ring = new RingBuffer<number>(4);
+      ring.insert(0);
+      ring.insert(1);
+      ring.insert(2);
+      ring.insert(3);
+      ring.insert(4);
+
+      expect(ring.toArray()).to.deep.eq([1, 2, 3, 4]);
+    });
+
+    it('more than 2 full laps erasing data', () => {
+      const ring = new RingBuffer<number>(4);
+      ring.insert(0);
+      ring.insert(1);
+      ring.insert(2);
+      ring.insert(3);
+      ring.insert(4); // first lap first item
+      ring.insert(5);
+      ring.insert(6); // first item in toArray should be this one
+      ring.insert(7);
+      ring.insert(8); // second lap first item
+      ring.insert(9);
+
+      expect(ring.toArray()).to.deep.eq([6, 7, 8, 9]);
+    });
+  });
+
+  describe('clear', () => {
+    it('empty buffer', () => {
+      const ring = new RingBuffer<number>(4);
+      ring.clear();
+      expect(ring.getCapacity()).to.deep.eq(4);
+      expect(ring.getLength()).to.deep.eq(0);
+    });
+
+    it('with 1', () => {
+      const ring = new RingBuffer<number>(4);
+      ring.insert(0);
+      ring.clear();
+      expect(ring.getCapacity()).to.deep.eq(4);
+      expect(ring.getLength()).to.deep.eq(0);
+    });
+
+    it('with 5', () => {
+      const ring = new RingBuffer<number>(4);
+      ring.insert(0);
+      ring.insert(1);
+      ring.insert(2);
+      ring.insert(3);
+      ring.insert(4);
+
+      ring.clear();
+      expect(ring.getCapacity()).to.deep.eq(4);
+      expect(ring.getLength()).to.deep.eq(0);
+    });
+  });
 });