CharlieFRuan commented on code in PR #16525:
URL: https://github.com/apache/tvm/pull/16525#discussion_r1486347194
##########
web/src/runtime.ts:
##########
@@ -1477,6 +1489,41 @@ export class Instance implements Disposable {
this.cacheMetadata = { ...this.cacheMetadata, ...(list["metadata"] as
Record<string, any>) };
}
+ /**
+ * Delete NDArray from cache url.
Review Comment:
Would be great if we could have more documentation on this function's
behavior. e.g. "given `cacheUrl`, we search up items to delete based on
`cacheUrl/ndarray-cache.json`", etc.
One thing worth pointing out is that, the `ndarray-cache.json` and tokenizer
files would still linger in the cache after this function. We only delete the
shard files.
##########
web/src/runtime.ts:
##########
@@ -1477,6 +1489,41 @@ export class Instance implements Disposable {
this.cacheMetadata = { ...this.cacheMetadata, ...(list["metadata"] as
Record<string, any>) };
}
+ /**
+ * Delete NDArray from cache url.
+ *
+ * @param ndarrayCacheUrl
+ * @param device
+ * @param cacheScope
+ */
+ async deleteNDArrayCache(
+ ndarrayCacheUrl: string,
+ device: DLDevice,
Review Comment:
device is not used here, perhaps we can remove
##########
web/src/runtime.ts:
##########
@@ -1477,6 +1489,41 @@ export class Instance implements Disposable {
this.cacheMetadata = { ...this.cacheMetadata, ...(list["metadata"] as
Record<string, any>) };
}
+ /**
+ * Delete NDArray from cache url.
+ *
+ * @param ndarrayCacheUrl
+ * @param device
+ * @param cacheScope
+ */
+ async deleteNDArrayCache(
+ ndarrayCacheUrl: string,
+ device: DLDevice,
+ cacheScope = "tvmjs"
+ ): Promise<any>{
+ const artifactCache = new ArtifactCache(cacheScope);
+ const jsonUrl = new URL("ndarray-cache.json", ndarrayCacheUrl).href;
+ const result = await artifactCache.fetchWithCache(jsonUrl);
+ let list;
+ if (result instanceof Response){
+ list = await result.json();
+ }
+ const arrayentry = list["records"] as Array<NDArrayShardEntry>;
+ for (let i = 0; i < arrayentry.length; ++i){
+ const dataUrl = new URL(arrayentry[i].dataPath, ndarrayCacheUrl).href;
+ try {
+ const result = await(await artifactCache.deleteInCache(dataUrl));
+ if (result === false){
+ return false;
+ }
Review Comment:
Should we end the deletion if one of the entry is not found? Or should we
ensure everything inside `ndarry-cache` is deleted? I prefer the latter -- let
me know what you think! If we go with the latter, this function does not need
to return a `boolean` but can be `void`. Whichever we go with, we need to make
sure the function's behavior is well defined, matching its docstring.
##########
web/src/runtime.ts:
##########
@@ -1477,6 +1489,41 @@ export class Instance implements Disposable {
this.cacheMetadata = { ...this.cacheMetadata, ...(list["metadata"] as
Record<string, any>) };
}
+ /**
+ * Delete NDArray from cache url.
+ *
+ * @param ndarrayCacheUrl
+ * @param device
+ * @param cacheScope
+ */
+ async deleteNDArrayCache(
Review Comment:
I feel like this function should be outside of `Instance`, just like
`hasNDArrayInCache()`, since it is more like a helper function. We can then
expose this function in
https://github.com/apache/tvm/blob/main/web/src/index.ts#L25, so that tvmjs
users can call this function.
##########
web/src/runtime.ts:
##########
@@ -1477,6 +1489,41 @@ export class Instance implements Disposable {
this.cacheMetadata = { ...this.cacheMetadata, ...(list["metadata"] as
Record<string, any>) };
}
+ /**
+ * Delete NDArray from cache url.
+ *
+ * @param ndarrayCacheUrl
+ * @param device
+ * @param cacheScope
+ */
+ async deleteNDArrayCache(
+ ndarrayCacheUrl: string,
+ device: DLDevice,
+ cacheScope = "tvmjs"
+ ): Promise<any>{
+ const artifactCache = new ArtifactCache(cacheScope);
+ const jsonUrl = new URL("ndarray-cache.json", ndarrayCacheUrl).href;
+ const result = await artifactCache.fetchWithCache(jsonUrl);
+ let list;
+ if (result instanceof Response){
+ list = await result.json();
+ }
+ const arrayentry = list["records"] as Array<NDArrayShardEntry>;
+ for (let i = 0; i < arrayentry.length; ++i){
+ const dataUrl = new URL(arrayentry[i].dataPath, ndarrayCacheUrl).href;
+ try {
+ const result = await(await artifactCache.deleteInCache(dataUrl));
+ if (result === false){
+ return false;
+ }
+ } catch (err){
+ this.env.logger("Error: Cannot fetch " + dataUrl + " err= " + err);
Review Comment:
Should be `Cannot delete` rather than `Cannot fetch`
--
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]
