You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "DiegoCao (via GitHub)" <gi...@apache.org> on 2024/02/05 18:03:15 UTC
[PR] [WEB] [MINOR] Move ArtifactCache to Interface to support future different cache typ… [tvm]
DiegoCao opened a new pull request, #16525:
URL: https://github.com/apache/tvm/pull/16525
- Move old artifact cache as a interface template to support future IndexDB cache fetch
- Add deleteInCache api in cache template
- Fix one test path error
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WEB] Move ArtifactCache to Interface, Support Cache delete and Batch Delete, Remove typo [tvm]
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen merged PR #16525:
URL: https://github.com/apache/tvm/pull/16525
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WEB] [MINOR] Move ArtifactCache to Interface to support future different cache typ… [tvm]
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on PR #16525:
URL: https://github.com/apache/tvm/pull/16525#issuecomment-1930215569
please fix the ci errors
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WEB] Move ArtifactCache to Interface, Support Cache delete and Batch Delete, Remove typo [tvm]
Posted by "CharlieFRuan (via GitHub)" <gi...@apache.org>.
CharlieFRuan commented on PR #16525:
URL: https://github.com/apache/tvm/pull/16525#issuecomment-1938045055
Will take a look tomorrow; thank you @DiegoCao!
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WEB] Move ArtifactCache to Interface, Support Cache delete and Batch Delete, Remove typo [tvm]
Posted by "DiegoCao (via GitHub)" <gi...@apache.org>.
DiegoCao commented on code in PR #16525:
URL: https://github.com/apache/tvm/pull/16525#discussion_r1488496964
##########
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:
I also prefer the latter one. Will change the function to void
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WEB] Move ArtifactCache to Interface, Support Cache delete and Batch Delete, Remove typo [tvm]
Posted by "CharlieFRuan (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WEB] Move ArtifactCache to Interface, Support Cache delete and Batch Delete, Remove typo [tvm]
Posted by "CharlieFRuan (via GitHub)" <gi...@apache.org>.
CharlieFRuan commented on PR #16525:
URL: https://github.com/apache/tvm/pull/16525#issuecomment-1951413801
Thank you! I think this is ready to be merged cc @tqchen
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WEB] Move ArtifactCache to Interface, Support Cache delete and Batch Delete, Remove typo [tvm]
Posted by "CharlieFRuan (via GitHub)" <gi...@apache.org>.
CharlieFRuan commented on code in PR #16525:
URL: https://github.com/apache/tvm/pull/16525#discussion_r1493816633
##########
web/src/runtime.ts:
##########
@@ -2432,3 +2444,28 @@ export async function hasNDArrayInCache(
list = list["records"] as Array<NDArrayShardEntry>;
return await artifactCache.hasAllKeys(list.map(key => new URL(key.dataPath, ndarrayCacheUrl).href));
}
+
+/**
+ * Given cacheUrl, search up items to delete based on cacheUrl/ndarray-cache.json
+ *
+ * @param ndarrayCacheUrl
+ * @param cacheScope
+ */
+export async function deleteNDArrayCache(
+ ndarrayCacheUrl: string,
+ cacheScope = "tvmjs"
+) {
+ 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>;
+ const processShard = async (i: number) => {
+ const dataUrl = new URL(arrayentry[i].dataPath, ndarrayCacheUrl).href;
+ await(await artifactCache.deleteInCache(dataUrl));
Review Comment:
Only one `await` is needed
##########
web/tvm_home:
##########
@@ -0,0 +1 @@
+/ssd1/hangruic/tvm/
Review Comment:
Please remove this change
##########
web/src/runtime.ts:
##########
@@ -2432,3 +2444,28 @@ export async function hasNDArrayInCache(
list = list["records"] as Array<NDArrayShardEntry>;
return await artifactCache.hasAllKeys(list.map(key => new URL(key.dataPath, ndarrayCacheUrl).href));
}
+
+/**
+ * Given cacheUrl, search up items to delete based on cacheUrl/ndarray-cache.json
+ *
+ * @param ndarrayCacheUrl
+ * @param cacheScope
+ */
+export async function deleteNDArrayCache(
+ ndarrayCacheUrl: string,
Review Comment:
We should replace `ndarrayCacheUrl` to `cacheUrl` to match the docstring and the semantics.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WEB] Move ArtifactCache to Interface, Support Cache delete and Batch Delete, Remove typo [tvm]
Posted by "DiegoCao (via GitHub)" <gi...@apache.org>.
DiegoCao commented on code in PR #16525:
URL: https://github.com/apache/tvm/pull/16525#discussion_r1488503849
##########
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:
Received, also plan to move the fetchNDArrayInCache to the export
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WEB] Move ArtifactCache to Interface, Support Cache delete and Batch Delete, Remove typo [tvm]
Posted by "DiegoCao (via GitHub)" <gi...@apache.org>.
DiegoCao commented on PR #16525:
URL: https://github.com/apache/tvm/pull/16525#issuecomment-1951411477
All issues are resolved
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WEB] [MINOR] Move ArtifactCache to Interface to support future different cache typ… [tvm]
Posted by "DiegoCao (via GitHub)" <gi...@apache.org>.
DiegoCao commented on PR #16525:
URL: https://github.com/apache/tvm/pull/16525#issuecomment-1930758652
Will re-open after supporting batch delete weights
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org