Skip to content

Commit 87ce1a8

Browse files
authored
Always test for a fence with gl2.clientWaitSync(sync, 0, 0) (tensorflow#1529)
Fixes tensorflow/tfjs#1145 The smallest repro is calling .data() 3 times without waiting: ```ts tf.ENV.set('WEBGL_CPU_FORWARD', false) tf.ENV.set('WEBGL_SIZE_UPLOAD_UNIFORM', 0); tf.square(3).data(); tf.square(3).data(); tf.square(3).data(); ``` There are 3 fences that are in parallel. And on the next tick the binary search tests only the middle fence and sees that it is done, and assumes the first fence is done too. But since we never explicitly tested fence[0].isDone(), Chrome gives a warning. The warnings started showing up more often when Layers started doing parallel reads of the scalar loss values: tensorflow/tfjs-layers#398 I changed the binary search to linear search so we also test all fences up to the one that is not done.
1 parent 3a77069 commit 87ce1a8

File tree

2 files changed

+29
-34
lines changed

2 files changed

+29
-34
lines changed

src/kernels/webgl/gpgpu_context.ts

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -520,9 +520,8 @@ export class GPGPUContext {
520520
private itemsToPoll: PollItem[] = [];
521521

522522
pollItems(): void {
523-
// Find the last query that has finished using binary search.
524-
// All other queries before it are also done.
525-
const index = binSearchLastTrue(this.itemsToPoll.map(x => x.isDoneFn));
523+
// Find the last query that has finished.
524+
const index = linearSearchLastTrue(this.itemsToPoll.map(x => x.isDoneFn));
526525
for (let i = 0; i <= index; ++i) {
527526
const {resolveFn} = this.itemsToPoll[i];
528527
resolveFn();
@@ -616,22 +615,18 @@ type PollItem = {
616615
};
617616

618617
/**
619-
* Finds the index of the last true element using binary search where
620-
* evaluation of an entry is expensive.
618+
* Finds the index of the last true element using linear search.
619+
* Note: We can't do binary search because Chrome expects us to explicitly
620+
* test all fences before download:
621+
* https://github.com/tensorflow/tfjs/issues/1145
621622
*/
622-
export function binSearchLastTrue(arr: Array<() => boolean>): number {
623-
let start = 0;
624-
let end = arr.length - 1;
625-
let best = -1;
626-
while (start <= end) {
627-
const mid = (start + end) >> 1;
628-
const isDone = arr[mid]();
629-
if (isDone) {
630-
best = mid;
631-
start = mid + 1;
632-
} else {
633-
end = mid - 1;
623+
export function linearSearchLastTrue(arr: Array<() => boolean>): number {
624+
let i = 0;
625+
for (; i < arr.length; ++i) {
626+
const isDone = arr[i]();
627+
if (!isDone) {
628+
break;
634629
}
635630
}
636-
return best;
631+
return i - 1;
637632
}

src/kernels/webgl/gpgpu_context_test.ts

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import {describeWithFlags} from '../../jasmine_util';
1919
import {expectArraysClose, expectNumbersClose} from '../../test_util';
2020
import {getGlslDifferences} from './glsl_version';
21-
import {binSearchLastTrue, GPGPUContext} from './gpgpu_context';
21+
import {GPGPUContext, linearSearchLastTrue} from './gpgpu_context';
2222
import * as tex_util from './tex_util';
2323

2424
const DOWNLOAD_FLOAT_ENVS = {
@@ -316,88 +316,88 @@ describeWithFlags('GPGPUContext', DOWNLOAD_FLOAT_ENVS, () => {
316316
});
317317
});
318318

319-
describe('gpgpu_context binSearchLastTrue', () => {
319+
describe('gpgpu_context linearSearchLastTrue', () => {
320320
it('[false]', () => {
321321
const a: boolean[] = [false];
322322
const arr = a.map(x => () => x);
323-
expect(binSearchLastTrue(arr)).toBe(-1);
323+
expect(linearSearchLastTrue(arr)).toBe(-1);
324324
});
325325

326326
it('[true]', () => {
327327
const a: boolean[] = [true];
328328
const arr = a.map(x => () => x);
329-
expect(binSearchLastTrue(arr)).toBe(0);
329+
expect(linearSearchLastTrue(arr)).toBe(0);
330330
});
331331

332332
it('[false, false]', () => {
333333
const a: boolean[] = [false, false];
334334
const arr = a.map(x => () => x);
335-
expect(binSearchLastTrue(arr)).toBe(-1);
335+
expect(linearSearchLastTrue(arr)).toBe(-1);
336336
});
337337

338338
it('[true, false]', () => {
339339
const a: boolean[] = [true, false];
340340
const arr = a.map(x => () => x);
341-
expect(binSearchLastTrue(arr)).toBe(0);
341+
expect(linearSearchLastTrue(arr)).toBe(0);
342342
});
343343

344344
it('[true, true]', () => {
345345
const a: boolean[] = [true, true];
346346
const arr = a.map(x => () => x);
347-
expect(binSearchLastTrue(arr)).toBe(1);
347+
expect(linearSearchLastTrue(arr)).toBe(1);
348348
});
349349

350350
it('[false, false, false]', () => {
351351
const a: boolean[] = [false, false, false];
352352
const arr = a.map(x => () => x);
353-
expect(binSearchLastTrue(arr)).toBe(-1);
353+
expect(linearSearchLastTrue(arr)).toBe(-1);
354354
});
355355

356356
it('[true, false, false]', () => {
357357
const a: boolean[] = [true, false, false];
358358
const arr = a.map(x => () => x);
359-
expect(binSearchLastTrue(arr)).toBe(0);
359+
expect(linearSearchLastTrue(arr)).toBe(0);
360360
});
361361

362362
it('[true, true, false]', () => {
363363
const a: boolean[] = [true, true, false];
364364
const arr = a.map(x => () => x);
365-
expect(binSearchLastTrue(arr)).toBe(1);
365+
expect(linearSearchLastTrue(arr)).toBe(1);
366366
});
367367

368368
it('[true, true, true]', () => {
369369
const a: boolean[] = [true, true, true];
370370
const arr = a.map(x => () => x);
371-
expect(binSearchLastTrue(arr)).toBe(2);
371+
expect(linearSearchLastTrue(arr)).toBe(2);
372372
});
373373

374374
it('[false, false, false, false]', () => {
375375
const a: boolean[] = [false, false, false, false];
376376
const arr = a.map(x => () => x);
377-
expect(binSearchLastTrue(arr)).toBe(-1);
377+
expect(linearSearchLastTrue(arr)).toBe(-1);
378378
});
379379

380380
it('[true, false, false, false]', () => {
381381
const a: boolean[] = [true, false, false, false];
382382
const arr = a.map(x => () => x);
383-
expect(binSearchLastTrue(arr)).toBe(0);
383+
expect(linearSearchLastTrue(arr)).toBe(0);
384384
});
385385

386386
it('[true, true, false, false]', () => {
387387
const a: boolean[] = [true, true, false, false];
388388
const arr = a.map(x => () => x);
389-
expect(binSearchLastTrue(arr)).toBe(1);
389+
expect(linearSearchLastTrue(arr)).toBe(1);
390390
});
391391

392392
it('[true, true, true, false]', () => {
393393
const a: boolean[] = [true, true, true, false];
394394
const arr = a.map(x => () => x);
395-
expect(binSearchLastTrue(arr)).toBe(2);
395+
expect(linearSearchLastTrue(arr)).toBe(2);
396396
});
397397

398398
it('[true, true, true, true]', () => {
399399
const a: boolean[] = [true, true, true, true];
400400
const arr = a.map(x => () => x);
401-
expect(binSearchLastTrue(arr)).toBe(3);
401+
expect(linearSearchLastTrue(arr)).toBe(3);
402402
});
403403
});

0 commit comments

Comments
 (0)