gemini-code-assist[bot] commented on code in PR #19673:
URL: https://github.com/apache/tvm/pull/19673#discussion_r3370380378
##########
web/src/opfs_store.ts:
##########
@@ -43,94 +51,193 @@ interface OPFSStorageManager {
getDirectory?: () => Promise<OPFSDirectoryHandle>;
}
-interface OPFSStoreMetadata {
+interface OPFSStoreRecord {
url: string;
+ nbytes: number;
contentType?: string;
}
+interface OPFSStoredEntry {
+ payloadHandle: OPFSFileHandle;
+ record: OPFSStoreRecord;
+}
+
+interface OPFSSyncAccessHandle {
+ getSize(): number;
+ read(buffer: BufferSource, options?: { at?: number }): number;
+ write(buffer: BufferSource, options?: { at?: number }): number;
+ truncate(size: number): void;
+ flush(): void;
+ close(): void;
+}
+
+type OPFSGlobalScope = typeof globalThis & {
+ DedicatedWorkerGlobalScope?: new () => object;
+ FileSystemFileHandle?: {
+ prototype?: {
+ createSyncAccessHandle?: unknown;
+ };
+ };
+};
+
const HASH_ALGORITHM = "SHA-256";
const OPFS_STORE_ROOT_DIRECTORY = "tvmjs-opfs-store";
export class OPFSStore {
private readonly scope: string;
+ private readonly requestedAccessMode: OPFSAccessMode;
+ private accessMode: OPFSEffectiveAccessMode;
private directoryPromise?: Promise<OPFSDirectoryHandle>;
- constructor(scope: string) {
+ constructor(scope: string, accessMode: OPFSAccessMode = "async") {
this.scope = scope;
+ this.requestedAccessMode = accessMode;
+ this.accessMode = OPFSStore.resolveAccessMode(accessMode);
}
static isAvailable(): boolean {
const storage = OPFSStore.getStorageManager();
return storage !== undefined && typeof storage.getDirectory === "function";
}
+ private static resolveAccessMode(
+ accessMode: OPFSAccessMode,
+ ): OPFSEffectiveAccessMode {
+ if (accessMode !== "auto") {
+ return accessMode;
+ }
+ return OPFSStore.isDedicatedWorkerWithSyncAccessHandle()
+ ? "sync"
+ : "async";
+ }
+
async has(url: string): Promise<boolean> {
- return (await this.read(url)) !== undefined;
+ try {
+ const entry = await this.getStoredEntry(url);
+ if (entry === undefined) {
+ return false;
+ }
+ return this.hasExpectedPayloadSize(entry);
+ } catch (err) {
+ if (this.handleCacheMissStateError(err)) {
+ return false;
+ }
+ throw err;
+ }
}
async read(url: string): Promise<Response | undefined> {
- const directory = await this.getScopedDirectory();
- const baseName = await this.hashUrl(url);
- const dataHandle = await this.getFileHandleIfExists(
- directory,
- `${baseName}.bin`,
- false,
- );
- if (dataHandle === undefined) {
- return undefined;
+ try {
+ const entry = await this.getStoredEntry(url);
+ if (entry === undefined) {
+ return undefined;
+ }
+ if (this.accessMode === "async") {
+ const blob = await entry.payloadHandle.getFile();
+ if (blob.size !== entry.record.nbytes) {
+ return undefined;
+ }
+ return new Response(blob, this.getResponseInit(entry.record));
+ }
+ const payload = await this.readPayload(entry.payloadHandle);
+ if (payload.byteLength !== entry.record.nbytes) {
Review Comment:
When overwriting an existing cache entry, if the payload write fails or is
interrupted, the old `.meta.json` file will still exist and point to the
now-corrupted or truncated `.bin` file. This would cause subsequent reads to
incorrectly treat the corrupted file as a valid cache hit.
To prevent this, we should delete the old `.meta.json` file before starting
to write the new payload, ensuring that any failure during the payload write
results in a clean cache miss.
```typescript
await this.removeEntryIfExists(directory, `${baseName}.meta.json`);
const payloadHandle = await directory.getFileHandle(`${baseName}.bin`, {
create: true,
});
```
##########
web/src/opfs_store.ts:
##########
@@ -43,94 +51,193 @@ interface OPFSStorageManager {
getDirectory?: () => Promise<OPFSDirectoryHandle>;
}
-interface OPFSStoreMetadata {
+interface OPFSStoreRecord {
url: string;
+ nbytes: number;
contentType?: string;
}
+interface OPFSStoredEntry {
+ payloadHandle: OPFSFileHandle;
+ record: OPFSStoreRecord;
+}
+
+interface OPFSSyncAccessHandle {
+ getSize(): number;
+ read(buffer: BufferSource, options?: { at?: number }): number;
+ write(buffer: BufferSource, options?: { at?: number }): number;
+ truncate(size: number): void;
+ flush(): void;
+ close(): void;
+}
+
+type OPFSGlobalScope = typeof globalThis & {
+ DedicatedWorkerGlobalScope?: new () => object;
+ FileSystemFileHandle?: {
+ prototype?: {
+ createSyncAccessHandle?: unknown;
+ };
+ };
+};
+
const HASH_ALGORITHM = "SHA-256";
const OPFS_STORE_ROOT_DIRECTORY = "tvmjs-opfs-store";
export class OPFSStore {
private readonly scope: string;
+ private readonly requestedAccessMode: OPFSAccessMode;
+ private accessMode: OPFSEffectiveAccessMode;
private directoryPromise?: Promise<OPFSDirectoryHandle>;
- constructor(scope: string) {
+ constructor(scope: string, accessMode: OPFSAccessMode = "async") {
this.scope = scope;
+ this.requestedAccessMode = accessMode;
+ this.accessMode = OPFSStore.resolveAccessMode(accessMode);
}
static isAvailable(): boolean {
const storage = OPFSStore.getStorageManager();
return storage !== undefined && typeof storage.getDirectory === "function";
}
+ private static resolveAccessMode(
+ accessMode: OPFSAccessMode,
+ ): OPFSEffectiveAccessMode {
+ if (accessMode !== "auto") {
+ return accessMode;
+ }
+ return OPFSStore.isDedicatedWorkerWithSyncAccessHandle()
+ ? "sync"
+ : "async";
+ }
+
async has(url: string): Promise<boolean> {
- return (await this.read(url)) !== undefined;
+ try {
+ const entry = await this.getStoredEntry(url);
+ if (entry === undefined) {
+ return false;
+ }
+ return this.hasExpectedPayloadSize(entry);
+ } catch (err) {
+ if (this.handleCacheMissStateError(err)) {
+ return false;
+ }
+ throw err;
+ }
}
async read(url: string): Promise<Response | undefined> {
- const directory = await this.getScopedDirectory();
- const baseName = await this.hashUrl(url);
- const dataHandle = await this.getFileHandleIfExists(
- directory,
- `${baseName}.bin`,
- false,
- );
- if (dataHandle === undefined) {
- return undefined;
+ try {
+ const entry = await this.getStoredEntry(url);
+ if (entry === undefined) {
+ return undefined;
+ }
+ if (this.accessMode === "async") {
+ const blob = await entry.payloadHandle.getFile();
+ if (blob.size !== entry.record.nbytes) {
+ return undefined;
+ }
+ return new Response(blob, this.getResponseInit(entry.record));
+ }
+ const payload = await this.readPayload(entry.payloadHandle);
+ if (payload.byteLength !== entry.record.nbytes) {
+ return undefined;
+ }
+ return new Response(payload, this.getResponseInit(entry.record));
+ } catch (err) {
+ if (this.handleCacheMissStateError(err)) {
+ return undefined;
+ }
+ throw err;
}
- const dataBlob = await dataHandle.getFile();
- const metadataHandle = await this.getFileHandleIfExists(
- directory,
- `${baseName}.meta.json`,
- false,
- );
- let metadata: OPFSStoreMetadata | undefined = undefined;
- if (metadataHandle !== undefined) {
- metadata = await this.readMetadata(metadataHandle);
- if (metadata?.url !== undefined && metadata.url !== url) {
- throw new Error("OPFSStore: metadata URL does not match key URL.");
+ }
+
+ async readArrayBuffer(url: string): Promise<ArrayBuffer | undefined> {
+ try {
+ const entry = await this.getStoredEntry(url);
+ if (entry === undefined) {
+ return undefined;
+ }
+ const payload = await this.readPayload(entry.payloadHandle);
+ return payload.byteLength === entry.record.nbytes ? payload : undefined;
+ } catch (err) {
+ if (this.handleCacheMissStateError(err)) {
+ return undefined;
}
+ throw err;
}
- const headers =
- metadata?.contentType !== undefined
- ? { "content-type": metadata.contentType }
- : undefined;
- return new Response(dataBlob, headers ? { headers } : undefined);
}
async write(url: string, response: Response): Promise<void> {
- const directory = await this.getScopedDirectory();
- const baseName = await this.hashUrl(url);
- const dataHandle = await directory.getFileHandle(`${baseName}.bin`, {
- create: true,
- });
- const metadataHandle = await directory.getFileHandle(
- `${baseName}.meta.json`,
- { create: true },
- );
- const metadata: OPFSStoreMetadata = {
- url,
- contentType: response.headers.get("content-type") ?? undefined,
- };
- const writable = await dataHandle.createWritable();
- if (response.body !== null) {
- await response.body.pipeTo(writable);
- } else {
- await writable.write(await response.arrayBuffer());
- await writable.close();
+ try {
+ const directory = await this.getScopedDirectory();
+ const baseName = await this.hashUrl(url);
+ await this.removeEntryIfExists(
+ directory,
+ this.getRecordFilename(baseName),
+ );
+ const payloadHandle = await directory.getFileHandle(
+ this.getPayloadFilename(baseName),
+ { create: true },
+ );
+ const nbytes = await this.writePayload(payloadHandle, response);
+ const recordHandle = await directory.getFileHandle(
+ this.getRecordFilename(baseName),
+ { create: true },
+ );
Review Comment:
If the `.meta.json` file is corrupted or partially written (e.g., due to an
interrupted write or disk issues), `readMetadata` will throw a `SyntaxError`
(from `JSON.parse`) or another validation error. Since `readMetadata` only
catches `NotFoundError`, this error will propagate and crash the entire cache
read/has operation.
Wrapping the `readMetadata` call in a `try-catch` block inside
`getExistingEntry` ensures that any corrupted metadata is gracefully treated as
a cache miss.
```typescript
let metadata: OPFSStoreMetadata | undefined;
try {
metadata = await this.readMetadata(metadataHandle);
} catch {
return undefined;
}
if (metadata === undefined) {
return undefined;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
