Skip to content

Conversation

@DiegoCao
Copy link
Contributor

@DiegoCao DiegoCao commented Mar 18, 2024

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
Copy link
Member

@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.

@DiegoCao DiegoCao force-pushed the main branch 2 times, most recently from 8af42af to 1ed3223 Compare April 1, 2024 17:19
@CharlieFRuan
Copy link
Member

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

- 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
@MasterJH5574
Copy link
Contributor

@tvm-bot rerun

@tqchen tqchen merged commit d1e24ca into apache:main Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants