segmented-object: TcpCubic ignores rtt=0

yoursunny · Jul 23, 2024 · 7b17e54 · 7b17e54
1 parent e0c16c4
commit 7b17e54
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 8 deletions.
diff --git a/pkg/segmented-object/src/fetch/congestion-avoidance.ts b/pkg/segmented-object/src/fetch/congestion-avoidance.ts
@@ -22,6 +22,7 @@ export abstract class CongestionAvoidance extends TypedEventTarget<EventMap> {
   public get cwnd() { return this.cwnd_; }
 
   protected updateCwnd(v: number) {
+    assert(Number.isFinite(v));
     assert(v >= 0);
     this.cwnd_ = v;
     this.dispatchTypedEvent("cwndupdate", new Event("cwndupdate"));

diff --git a/pkg/segmented-object/src/fetch/rtt-estimator.ts b/pkg/segmented-object/src/fetch/rtt-estimator.ts
@@ -19,6 +19,9 @@ const defaultParameters: Parameters = {
 /**
  * RTT estimator.
  * @see {@link https://datatracker.ietf.org/doc/html/rfc6298}
+ *
+ * While the algorithm is agnostic to time units, the default parameters assume milliseconds,
+ * following JavaScript convention.
  */
 export class RttEstimator {
   private params: Parameters;

diff --git a/pkg/segmented-object/src/fetch/tcp-cubic.ts b/pkg/segmented-object/src/fetch/tcp-cubic.ts
@@ -3,6 +3,8 @@ import { CongestionAvoidance } from "./congestion-avoidance";
 /**
  * TCP CUBIC algorithm.
  * @see {@link https://datatracker.ietf.org/doc/html/rfc8312}
+ *
+ * RTT time unit is milliseconds, following JavaScript convention.
  */
 export class TcpCubic extends CongestionAvoidance {
   private readonly c: number;
@@ -26,8 +28,9 @@ export class TcpCubic extends CongestionAvoidance {
   }
 
   public override increase(now: number, rtt: number) {
-    if (now < this.t0) {
-      // increase and decrease processed out-of-order, t would be negative
+    if (now < this.t0 || rtt < 0.001) {
+      // increase and decrease processed out-of-order, t would be negative, ignore
+      // rtt is less than 1us, probably due to performance.now() inaccuracy, ignore
       return;
     }
 
@@ -37,9 +40,8 @@ export class TcpCubic extends CongestionAvoidance {
       return;
     }
 
-    const t = (now - this.t0) / 1000;
-    rtt /= 1000;
-    const wCubic = this.c * (t - this.k) ** 3 + this.wMax;
+    const t = now - this.t0; // milliseconds
+    const wCubic = this.c * (t / 1000 - this.k) ** 3 + this.wMax;
     const wEst = this.wMax * this.betaCubic + this.alphaAimd * (t / rtt);
     if (wCubic < wEst) { // TCP friendly region
       this.updateCwnd(wEst);

diff --git a/pkg/segmented-object/tests/fetch-algo.t.ts b/pkg/segmented-object/tests/fetch-algo.t.ts
@@ -1,28 +1,39 @@
-import { expect, test } from "vitest";
+import { expect, test, vi } from "vitest";
 
 import { TcpCubic } from "..";
 
 test("TcpCubic", () => {
   const ca = new TcpCubic();
   let now = 1624060800000;
 
+  const cwndupdate = vi.fn<(evt: Event) => void>();
+  ca.addEventListener("cwndupdate", cwndupdate);
+
   // slow start
   for (let i = 0; i < 98; ++i) {
-    ca.increase(now, 100);
+    ca.increase(now, 99.5 + Math.random());
     now += 5;
   }
   expect(ca.cwnd).toBeCloseTo(100);
 
+  // ignore rtt=0 sample
+  ca.increase(now, 0);
+  now += 5;
+
   // enter congestion avoidance
   ca.decrease(now);
   expect(ca.cwnd).toBeCloseTo(70);
   now += 5;
 
+  // ignore rtt=0 sample
+  ca.increase(now, 0);
+  now += 5;
+
   // increase window
   const firstCwnd = ca.cwnd;
   let lastCwnd = firstCwnd;
   for (let i = 0; i < 1000; ++i) {
-    ca.increase(now, 100);
+    ca.increase(now, 99.5 + Math.random());
     const thisCwnd = ca.cwnd;
     expect(thisCwnd).toBeGreaterThanOrEqual(lastCwnd);
     lastCwnd = thisCwnd;
@@ -33,4 +44,7 @@ test("TcpCubic", () => {
   // decrease window
   ca.decrease(now);
   expect(ca.cwnd).toBeLessThan(lastCwnd);
+
+  // rtt=0 samples should not trigger cwndupdate
+  expect(cwndupdate).toHaveBeenCalledTimes(98 + 1 + 1000 + 1);
 });