Skip to content

Commit

Permalink
Add AdapterLimitsGPUTest that sets all limits to the adapter's (#3466)
Browse files Browse the repository at this point in the history
Note: I'm not sure how I refactored device_pool.ts is good
but, the old code created one initial device with no descriptor
the first time in acquire. It then created another of the type
actually wanted by the user. That no longer works becauase the
adapter is passed in and so if the first requestDevice succeeds,
then the adapter has been used and can't be used for the second
one.

Also, assuming this change is approved,  I can (and should?) probably
refactor LimitsTest to use this vs how it's doing it now. That
would end up using cached devices.
  • Loading branch information
greggman authored Mar 8, 2024
1 parent 3e45aee commit 0a68bf7
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 22 deletions.
4 changes: 2 additions & 2 deletions src/webgpu/api/operation/rendering/draw.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import {
TypedArrayBufferView,
TypedArrayBufferViewConstructor,
} from '../../../../common/util/util.js';
import { GPUTest, TextureTestMixin } from '../../../gpu_test.js';
import { AdapterLimitsGPUTest, TextureTestMixin } from '../../../gpu_test.js';
import { PerPixelComparison } from '../../../util/texture/texture_ok.js';

class DrawTest extends TextureTestMixin(GPUTest) {
class DrawTest extends TextureTestMixin(AdapterLimitsGPUTest) {
checkTriangleDraw(opts: {
firstIndex: number | undefined;
count: number;
Expand Down
90 changes: 82 additions & 8 deletions src/webgpu/gpu_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,47 @@ export function initUncanonicalizedDeviceDescriptor(
}
}

/**
* Gets the adapter limits as a standard JavaScript object.
*/
function getAdapterLimitsAsDeviceRequiredLimits(adapter: GPUAdapter) {
const requiredLimits: Record<string, GPUSize64> = {};
const adapterLimits = adapter.limits as unknown as Record<string, GPUSize64>;
for (const key in adapter.limits) {
requiredLimits[key] = adapterLimits[key];
}
return requiredLimits;
}

/**
* Conditionally applies adapter limits to device descriptor
* but does not overwrite existing requested limits.
*/
function conditionallyApplyAdapterLimitsToDeviceDescriptor(
adapter: GPUAdapter,
useAdapterLimits: boolean,
descriptor: UncanonicalizedDeviceDescriptor | undefined
): UncanonicalizedDeviceDescriptor {
return {
...(descriptor || {}),
requiredLimits: {
...(useAdapterLimits && getAdapterLimitsAsDeviceRequiredLimits(adapter)),
...(descriptor?.requiredLimits || {}),
},
};
}

export class GPUTestSubcaseBatchState extends SubcaseBatchState {
/** Provider for default device. */
private provider: Promise<DeviceProvider> | undefined;
/** Provider for mismatched device. */
private mismatchedProvider: Promise<DeviceProvider> | undefined;
/** True if device should be created with adapter limits */
private useAdapterLimits = false;

constructor(recorder: TestCaseRecorder, params: TestParams) {
super(recorder, params);
}

override async postInit(): Promise<void> {
// Skip all subcases if there's no device.
Expand All @@ -123,6 +159,14 @@ export class GPUTestSubcaseBatchState extends SubcaseBatchState {
return this.provider;
}

useAdapterLimitsForDevice() {
assert(
this.provider === undefined,
'useAdapterLimits must be called before getting the device'
);
this.useAdapterLimits = true;
}

get isCompatibility() {
return globalTestConfig.compatibility;
}
Expand All @@ -140,10 +184,18 @@ export class GPUTestSubcaseBatchState extends SubcaseBatchState {
*/
selectDeviceOrSkipTestCase(descriptor: DeviceSelectionDescriptor): void {
assert(this.provider === undefined, "Can't selectDeviceOrSkipTestCase() multiple times");
this.provider = devicePool.acquire(
this.recorder,
initUncanonicalizedDeviceDescriptor(descriptor)
);
this.provider = devicePool
.requestAdapter(this.recorder)
.then(adapter =>
devicePool.acquire(
adapter,
conditionallyApplyAdapterLimitsToDeviceDescriptor(
adapter,
this.useAdapterLimits,
initUncanonicalizedDeviceDescriptor(descriptor)
)
)
);
// Suppress uncaught promise rejection (we'll catch it later).
this.provider.catch(() => {});
}
Expand Down Expand Up @@ -201,10 +253,18 @@ export class GPUTestSubcaseBatchState extends SubcaseBatchState {
"Can't selectMismatchedDeviceOrSkipTestCase() multiple times"
);

this.mismatchedProvider = mismatchedDevicePool.acquire(
this.recorder,
initUncanonicalizedDeviceDescriptor(descriptor)
);
this.mismatchedProvider = mismatchedDevicePool
.requestAdapter(this.recorder)
.then(adapter =>
mismatchedDevicePool.acquire(
adapter,
conditionallyApplyAdapterLimitsToDeviceDescriptor(
adapter,
this.useAdapterLimits,
initUncanonicalizedDeviceDescriptor(descriptor)
)
)
);
// Suppress uncaught promise rejection (we'll catch it later).
this.mismatchedProvider.catch(() => {});
}
Expand Down Expand Up @@ -1203,6 +1263,20 @@ export class GPUTest extends GPUTestBase {
}
}

/**
* A version of GPUTest that requires the adapter limits.
*/
export class AdapterLimitsGPUTest extends GPUTest {
public static override MakeSharedState(
recorder: TestCaseRecorder,
params: TestParams
): GPUTestSubcaseBatchState {
const state = new GPUTestSubcaseBatchState(recorder, params);
state.useAdapterLimitsForDevice();
return state;
}
}

/**
* Texture expectation mixin can be applied on top of GPUTest to add texture
* related expectation helpers.
Expand Down
32 changes: 20 additions & 12 deletions src/webgpu/util/device_pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,27 @@ export class TestOOMedShouldAttemptGC extends Error {}
export class DevicePool {
private holders: 'uninitialized' | 'failed' | DescriptorToHolderMap = 'uninitialized';

async requestAdapter(recorder: TestCaseRecorder) {
const gpu = getGPU(recorder);
const adapter = await gpu.requestAdapter();
assert(adapter !== null, 'requestAdapter returned null');
return adapter;
}

/** Acquire a device from the pool and begin the error scopes. */
async acquire(
recorder: TestCaseRecorder,
adapter: GPUAdapter,
descriptor?: UncanonicalizedDeviceDescriptor
): Promise<DeviceProvider> {
let errorMessage = '';
let holder;
if (this.holders === 'uninitialized') {
this.holders = new DescriptorToHolderMap();
}

let errorMessage = '';
if (this.holders !== 'failed') {
try {
await this.holders.getOrCreate(recorder, undefined);
holder = await this.holders.getOrCreate(adapter, descriptor);
} catch (ex) {
this.holders = 'failed';
if (ex instanceof Error) {
Expand All @@ -46,9 +57,7 @@ export class DevicePool {
this.holders !== 'failed',
`WebGPU device failed to initialize${errorMessage}; not retrying`
);

const holder = await this.holders.getOrCreate(recorder, descriptor);

assert(!!holder);
assert(holder.state === 'free', 'Device was in use on DevicePool.acquire');
holder.state = 'acquired';
holder.beginTestScope();
Expand Down Expand Up @@ -138,7 +147,7 @@ class DescriptorToHolderMap {
* Throws SkipTestCase if devices with this descriptor are unsupported.
*/
async getOrCreate(
recorder: TestCaseRecorder,
adapter: GPUAdapter,
uncanonicalizedDescriptor: UncanonicalizedDeviceDescriptor | undefined
): Promise<DeviceHolder> {
const [descriptor, key] = canonicalizeDescriptor(uncanonicalizedDescriptor);
Expand All @@ -163,7 +172,7 @@ class DescriptorToHolderMap {
// No existing item was found; add a new one.
let value;
try {
value = await DeviceHolder.create(recorder, descriptor);
value = await DeviceHolder.create(adapter, descriptor);
} catch (ex) {
if (ex instanceof FeaturesNotSupported) {
this.unsupported.add(key);
Expand Down Expand Up @@ -298,15 +307,14 @@ class DeviceHolder implements DeviceProvider {
// Gets a device and creates a DeviceHolder.
// If the device is lost, DeviceHolder.lost gets set.
static async create(
recorder: TestCaseRecorder,
adapter: GPUAdapter,
descriptor: CanonicalDeviceDescriptor | undefined
): Promise<DeviceHolder> {
const gpu = getGPU(recorder);
const adapter = await gpu.requestAdapter();
assert(adapter !== null, 'requestAdapter returned null');
assert(adapter !== null, 'requestAdapter is null');
if (!supportsFeature(adapter, descriptor)) {
throw new FeaturesNotSupported('One or more features are not supported');
}

const device = await adapter.requestDevice(descriptor);
assert(device !== null, 'requestDevice returned null');

Expand Down

0 comments on commit 0a68bf7

Please sign in to comment.