-
Notifications
You must be signed in to change notification settings - Fork 151
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.
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
Hub: Move hash-wasm into vendor (emscripten) #682
Conversation
This reverts commit d1dfbb2.
I published the package with tag I added some E2Es with Svelte and vite-build, which work great. But then I tried including the file directly from a CDN: https://huggingface.co/spaces/coyotte508/hub-sha256
Reference links for the tests:
That said, I'm very happy with this PR :) |
|
||
export function createSHA256WorkerCode(): string { | ||
return ` | ||
const _scriptDir = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coyotte508 Thanks for testing that out.
I had a look on the minified version. Turns out _scriptDir
get optimized out.
I attempt to this fix in 13df340 . Would you mind to redeploy it? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just in case, you can do things like:
if (typeof _scriptDir !== 'undefined' && _scriptDir) {
...
}
This should avoid failure when the variable is optimized out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, that's not really "optimized out" (Sorry my comment above was quite misleading - please ignore it).
What really happened is:
- emscripten set
_scriptDir
toimport.meta.url
, just to know where script is. This is useful for example when the.js
and.wasm
files are compiled separately, which is not our case. _scriptDir
is provided at global scope of the module, soWasmModule.toString()
cannot capture_scriptDir
. That's why inside web worker code, I "fake" the value to empty string.
When the script is minified, _scriptDir
is changed to another variable, that makes the "fake" value above no longer work. What I did in 13df340 was to use sed
to remove it from the emscripten script.
After having a deeper look, turns out I made a silly mistake: Signature of It should be fixed in the last commit. |
Just for fun, I also do a benchmark. Seems like we're gaining 20% performance: https://gist.github.com/ngxson/dfb4735e2f43b1b8b5d0c16113e84145
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks again for your help - that motivates me a lot. Feel free to merge (or to further test it) if you want. Bon weekend ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nice contrib
Resolves #221
In this PR, I moved source code of
hash-wasm
into vendor directory (c source code), also add compile script using emscripten. As a bonus, SIMD is also enabled, which should allow better performance.I also added
sha256-wrapper.ts
as a thin wrapper for the compiled wasm module.createSHA256
create the wasm module, works just like before.createSHA256WorkerCode
this function useFunction.toString()
trick to copy the module code into worker. This trick is already used in one of my project, wllama, so it should work on Firefox/Chrome/Safari (but feel free to test it further)