Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Web] Support web indexDB cache for larger model storage #16733

Merged
merged 7 commits into from
Apr 8, 2024
Merged

[Web] Support web indexDB cache for larger model storage #16733

merged 7 commits into from
Apr 8, 2024

Conversation

DiegoCao
Copy link
Contributor

@DiegoCao DiegoCao commented Mar 18, 2024 •
edited
Loading

Support IndexDB Cache, including the following:

  • Implement ArtifactIndexedDBCache to cache model with IndexedDB.
  • Add storetype to addToCache() and fetchWithCache() such that users need to specify either "json" or "arraybuffer" when using the APIs, instead of calling .json() or .arraybuffer() on their own outside of the function call.
  • Move all cache-related code to artifact_cache.ts.
  • Remove cacheOnly for callback in fetchNDArrayCache(), use loading instead (since we separated download and loading).

One change to use the IndexDBCache is, as there's no json() response when calling fetchCacheData, thinking about removing the .json() when calling fetchCacheData. @CharlieFRuan

@DiegoCao DiegoCao marked this pull request as ready for review March 20, 2024 16:17
@DiegoCao DiegoCao changed the title Support web indexDB cache for larger model storage [Web] Support web indexDB cache for larger model storage Mar 20, 2024
CharlieFRuan
CharlieFRuan reviewed Mar 25, 2024
View reviewed changes
Copy link
Contributor

@CharlieFRuan CharlieFRuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Tested with WebLLM and it is able to download and retrieve with IndexedDB. There are some items below we should address before merging it.

web/src/index.ts Outdated Show resolved Hide resolved
web/src/runtime.ts Outdated Show resolved Hide resolved
web/src/runtime.ts Outdated Show resolved Hide resolved
web/src/runtime.ts Outdated Show resolved Hide resolved
@DiegoCao DiegoCao force-pushed the main branch 2 times, most recently from 8af42af to 1ed3223 Compare April 1, 2024 17:19
@CharlieFRuan
Copy link
Contributor

Thank you for the updates! I will review soon today/tomorrow.

@CharlieFRuan
Merge branch 'main' into diego-tvm/main
41a15cb
@CharlieFRuan
Minor formatting
2f1f06e
@CharlieFRuan
Rename indexdb to indexeddb
0a60192
@CharlieFRuan
Modify addToCache and fetchWithCache logics
47310cc 
- We make addToCache not return anything, see new specification
- This allows us to skip downloaded files in fetchNDArrayCache instead of running into DOMException: Key already exists
- Call addToCache in fetchWithCache, so we only need to retrieve afterwards
- Remove cacheOnly for callback, use loading instead (since we separated download and loading)
- Fix responseTostoretype bug in ArtifactCache
@CharlieFRuan
Move all cache related code to artifact_cache
b9b06fc
@CharlieFRuan
Fix lint
081094b
@CharlieFRuan CharlieFRuan mentioned this pull request Apr 6, 2024
Merged
@MasterJH5574
Copy link
Contributor

@tvm-bot rerun

@tqchen tqchen merged commit d1e24ca into apache:main Apr 8, 2024
16 checks passed
@ysh329 ysh329 mentioned this pull request Apr 21, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CharlieFRuan CharlieFRuan CharlieFRuan left review comments

@tqchen tqchen tqchen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DiegoCao @CharlieFRuan @MasterJH5574 @tqchen