Skip to content

Commit 0af0d50

Browse files
committed
fix bug in size-only throttling
1 parent cb04476 commit 0af0d50

File tree

9 files changed

+652
-404
lines changed

9 files changed

+652
-404
lines changed

vuu-ui/packages/vuu-data-remote/src/message-utils.ts

Lines changed: 42 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -91,47 +91,47 @@ export const createSchemaFromTableMetadata = ({
9191
* to satisfy client range request, so they were not sent
9292
* to client. Now we can send them all.
9393
*/
94-
export const gapBetweenLastRowSentToClient = (
95-
lastRowsReturnedToClient: [number, number],
96-
pendingUpdates: VuuRow[],
97-
clientRange: VuuRange,
98-
rowCount: number,
99-
): VuuRange | undefined => {
100-
const firstPendingUpdate = pendingUpdates.at(0);
101-
const lastPendingUpdate = pendingUpdates.at(-1);
94+
// export const gapBetweenLastRowSentToClient = (
95+
// lastRowsReturnedToClient: [number, number],
96+
// pendingUpdates: VuuRow[],
97+
// clientRange: VuuRange,
98+
// rowCount: number,
99+
// ): VuuRange | undefined => {
100+
// const firstPendingUpdate = pendingUpdates.at(0);
101+
// const lastPendingUpdate = pendingUpdates.at(-1);
102102

103-
if (firstPendingUpdate && lastPendingUpdate) {
104-
const maxTo = Math.min(rowCount, clientRange.to);
103+
// if (firstPendingUpdate && lastPendingUpdate) {
104+
// const maxTo = Math.min(rowCount, clientRange.to);
105105

106-
const [firstRowIndex, lastRowIndex] = lastRowsReturnedToClient;
107-
if (
108-
lastRowIndex < firstPendingUpdate.rowIndex - 1 &&
109-
clientRange.from < firstPendingUpdate.rowIndex
110-
) {
111-
return {
112-
from: Math.max(lastRowIndex + 1, clientRange.from),
113-
to: firstPendingUpdate.rowIndex,
114-
};
115-
} else if (
116-
firstRowIndex > lastPendingUpdate.rowIndex + 1 &&
117-
maxTo > lastPendingUpdate.rowIndex
118-
) {
119-
return {
120-
from: lastPendingUpdate.rowIndex + 1,
121-
to: Math.min(maxTo, firstRowIndex),
122-
};
123-
} else if (firstRowIndex === -1 && lastRowIndex === -1) {
124-
if (clientRange.from < firstPendingUpdate.rowIndex) {
125-
return {
126-
from: clientRange.from,
127-
to: firstPendingUpdate.rowIndex,
128-
};
129-
} else if (maxTo > lastPendingUpdate.rowIndex + 1) {
130-
return {
131-
from: lastPendingUpdate.rowIndex + 1,
132-
to: maxTo,
133-
};
134-
}
135-
}
136-
}
137-
};
106+
// const [firstRowIndex, lastRowIndex] = lastRowsReturnedToClient;
107+
// if (
108+
// lastRowIndex < firstPendingUpdate.rowIndex - 1 &&
109+
// clientRange.from < firstPendingUpdate.rowIndex
110+
// ) {
111+
// return {
112+
// from: Math.max(lastRowIndex + 1, clientRange.from),
113+
// to: firstPendingUpdate.rowIndex,
114+
// };
115+
// } else if (
116+
// firstRowIndex > lastPendingUpdate.rowIndex + 1 &&
117+
// maxTo > lastPendingUpdate.rowIndex
118+
// ) {
119+
// return {
120+
// from: lastPendingUpdate.rowIndex + 1,
121+
// to: Math.min(maxTo, firstRowIndex),
122+
// };
123+
// } else if (firstRowIndex === -1 && lastRowIndex === -1) {
124+
// if (clientRange.from < firstPendingUpdate.rowIndex) {
125+
// return {
126+
// from: clientRange.from,
127+
// to: firstPendingUpdate.rowIndex,
128+
// };
129+
// } else if (maxTo > lastPendingUpdate.rowIndex + 1) {
130+
// return {
131+
// from: lastPendingUpdate.rowIndex + 1,
132+
// to: maxTo,
133+
// };
134+
// }
135+
// }
136+
// }
137+
// };

vuu-ui/packages/vuu-data-remote/src/server-proxy/array-backed-moving-window.ts

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,9 @@ export class ArrayBackedMovingWindow {
5757
return this.#range;
5858
}
5959

60-
// TODO we should probably have a hasAllClientRowsWithinRange
6160
get hasAllRowsWithinRange(): boolean {
6261
return (
6362
this.rowsWithinRange === this.clientRange.to - this.clientRange.from ||
64-
// this.rowsWithinRange === this.range.to - this.range.from ||
6563
(this.rowCount > 0 &&
6664
this.clientRange.from + this.rowsWithinRange === this.rowCount)
6765
);
@@ -130,15 +128,15 @@ export class ArrayBackedMovingWindow {
130128
return this.clientRange.isWithin(index);
131129
}
132130

133-
// Returns [false] or [serverDataRequired, clientRows, holdingRows]
131+
// Returns [false] or [serverDataRequired, clientRows]
134132
setClientRange(from: number, to: number): RangeTuple {
135133
log.debug?.(`setClientRange ${from} - ${to}`);
136134

137135
const currentFrom = this.clientRange.from;
138136
const currentTo = Math.min(this.clientRange.to, this.rowCount);
139137

140138
if (from === currentFrom && to === currentTo) {
141-
return [false, EMPTY_ARRAY /*, EMPTY_ARRAY*/] as RangeTuple;
139+
return [false, EMPTY_ARRAY] as RangeTuple;
142140
}
143141

144142
const originalRange = this.clientRange.copy();
@@ -152,20 +150,30 @@ export class ArrayBackedMovingWindow {
152150
}
153151
}
154152

155-
let clientRows: readonly VuuRow[] = EMPTY_ARRAY;
153+
const clientRows: VuuRow[] = [];
156154
const offset = this.#range.from;
157155

158-
if (this.hasAllRowsWithinRange) {
159-
if (to > originalRange.to) {
160-
const start = Math.max(from, originalRange.to);
161-
clientRows = this.internalData.slice(start - offset, to - offset);
162-
} else {
163-
const end = Math.min(originalRange.from, to);
164-
clientRows = this.internalData.slice(from - offset, end - offset);
156+
// if (this.hasAllRowsWithinRange) {
157+
if (to > originalRange.to) {
158+
const start = Math.max(from, originalRange.to);
159+
for (let i = start - offset; i < to - offset; i++) {
160+
const row = this.internalData[i];
161+
if (row) {
162+
clientRows.push(row);
163+
}
164+
}
165+
} else {
166+
const end = Math.min(originalRange.from, to);
167+
for (let i = from - offset; i < end - offset; i++) {
168+
const row = this.internalData[i];
169+
if (row) {
170+
clientRows.push(row);
171+
}
165172
}
166-
} else if (this.rowsWithinRange > 0) {
167-
// console.log(`[ArrayBackedMovingWindow] has some client rows but not all`);
168173
}
174+
// } else if (this.rowsWithinRange > 0) {
175+
// // console.log(`[ArrayBackedMovingWindow] has some client rows but not all`);
176+
// }
169177

170178
const serverDataRequired = this.bufferBreakout(from, to);
171179
return [serverDataRequired, clientRows] as RangeTuple;

vuu-ui/packages/vuu-data-remote/src/server-proxy/viewport.ts

Lines changed: 10 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,7 @@ import {
4545
UnfreezeViewportRequest,
4646
} from "@vuu-ui/vuu-protocol-types";
4747
import { getFullRange, KeySet, logger, RangeMonitor } from "@vuu-ui/vuu-utils";
48-
import {
49-
gapBetweenLastRowSentToClient,
50-
getFirstAndLastRows,
51-
} from "../message-utils";
48+
import { getFirstAndLastRows } from "../message-utils";
5249
import { ArrayBackedMovingWindow } from "./array-backed-moving-window";
5350
import * as Message from "./messages";
5451

@@ -167,7 +164,6 @@ export class Viewport {
167164
private rowCountChanged = false;
168165
private lastUpdateStatus: LastUpdateStatus = NO_UPDATE_STATUS;
169166
private updateThrottleTimer: number | undefined = undefined;
170-
private lastRowsReturnedToClient: [number, number] = [-1, -1];
171167

172168
private rangeMonitor = new RangeMonitor("ViewPort");
173169

@@ -233,10 +229,6 @@ export class Viewport {
233229
// Records SIZE only updates
234230
private setLastSizeOnlyUpdateSize = (size: number) => {
235231
this.lastUpdateStatus.size = size;
236-
if (size === 0) {
237-
this.lastRowsReturnedToClient[0] = -1;
238-
this.lastRowsReturnedToClient[1] = -1;
239-
}
240232
};
241233
private setLastUpdate = (mode: DataUpdateMode) => {
242234
const { ts: lastTS, mode: lastMode } = this.lastUpdateStatus;
@@ -551,9 +543,6 @@ export class Viewport {
551543

552544
const toClient = this.isTree ? toClientRowTree : toClientRow;
553545
if (clientRows.length) {
554-
this.lastRowsReturnedToClient[0] = clientRows[0].rowIndex;
555-
this.lastRowsReturnedToClient[1] = clientRows.at(-1)?.rowIndex ?? -1;
556-
557546
return [
558547
serverRequest,
559548
clientRows.map((row) => {
@@ -711,8 +700,6 @@ export class Viewport {
711700

712701
if (!this.isTree && config.groupBy.length > 0) {
713702
this.dataWindow?.clear();
714-
this.lastRowsReturnedToClient[0] = -1;
715-
this.lastRowsReturnedToClient[1] = -1;
716703
}
717704

718705
return this.createRequest(
@@ -802,11 +789,11 @@ export class Viewport {
802789
`ignore a SIZE=0 message on disabled viewport (${rows.length} rows)`,
803790
);
804791
return;
805-
} else if (firstRow.updateType === "SIZE") {
792+
} /* else if (firstRow.updateType === "SIZE") {
806793
// record all size only updates, we may throttle them and always need
807794
// to know the value of last update received.
808795
this.setLastSizeOnlyUpdateSize(firstRow.vpSize);
809-
}
796+
}*/
810797
}
811798

812799
for (const row of rows) {
@@ -866,54 +853,14 @@ export class Viewport {
866853
this.updateThrottleTimer = undefined;
867854
}
868855

869-
if (
870-
this.pendingUpdates.length > 0 &&
871-
this.dataWindow.hasAllRowsWithinRange
872-
) {
856+
if (this.pendingUpdates.length > 0) {
873857
out = [];
874858
mode = "update";
875859

876-
const missingRows = gapBetweenLastRowSentToClient(
877-
this.lastRowsReturnedToClient,
878-
this.pendingUpdates,
879-
this.dataWindow.clientRange,
880-
this.dataWindow.rowCount,
881-
);
882-
if (missingRows) {
883-
for (let i = missingRows.from; i < missingRows.to; i++) {
884-
const row = this.dataWindow.getAtIndex(i);
885-
if (row) {
886-
out.push(toClient(row, keys));
887-
} else {
888-
console.warn("[Viewport] missing row not in data cache");
889-
}
890-
}
891-
for (const row of this.pendingUpdates) {
892-
out.push(toClient(row, keys));
893-
}
894-
895-
// for (const row of this.dataWindow.getData()) {
896-
// out.push(toClient(row, keys, selectedRows));
897-
// }
898-
out.sort(
899-
([idx1]: DataSourceRow, [idx2]: DataSourceRow) => idx1 - idx2,
900-
);
901-
} else {
902-
for (const row of this.pendingUpdates) {
903-
out.push(toClient(row, keys));
904-
}
860+
for (const row of this.pendingUpdates) {
861+
out.push(toClient(row, keys));
905862
}
906-
907-
this.lastRowsReturnedToClient[0] = out.at(0)?.[0] ?? -1;
908-
this.lastRowsReturnedToClient[1] = out.at(-1)?.[0] ?? -1;
909-
} else if (this.pendingUpdates.length > 0) {
910-
// We have updates, but local cache does not have all rows to fill client range.
911-
// That means we must be processing a full range refresh, but don't yet have all
912-
// the data to send to client. When remaining rows are received, we will forward
913-
// rows to client.
914-
// Reset the lastRowsReturnedToClient. otherwise we would skip these pending updates
915-
this.lastRowsReturnedToClient[0] = -1;
916-
this.lastRowsReturnedToClient[1] = -1;
863+
// }
917864
}
918865
this.pendingUpdates.length = 0;
919866
this.hasUpdates = false;
@@ -952,13 +899,14 @@ export class Viewport {
952899

953900
private throttleMessage = (mode: DataUpdateMode) => {
954901
if (this.shouldThrottleMessage(mode)) {
955-
info?.("throttling updates setTimeout to 2000");
902+
info?.("[Viewport] throttling updates setTimeout to 300");
903+
this.setLastSizeOnlyUpdateSize(this.dataWindow.rowCount);
956904
if (this.updateThrottleTimer === undefined) {
957905
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
958906
// @ts-ignore
959907
this.updateThrottleTimer = setTimeout(
960908
this.sendThrottledSizeMessage,
961-
2000,
909+
100,
962910
);
963911
}
964912
return true;

0 commit comments

Comments
 (0)