Skip to content
New issue

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

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

Already on GitHub? # to your account

WebAssembly Dot Product #1825

Closed
wants to merge 2 commits into from
Closed

WebAssembly Dot Product #1825

wants to merge 2 commits into from

Conversation

GirkovArpa
Copy link

Replaced dot product with WebAssembly implementation.

@GirkovArpa
Copy link
Author

The linter failed because WebAssembly isn't defined? Why?

@josdejong
Copy link
Owner

Thanks Girkov for your PR.

I think if we start integrating WebAssembly in mathjs we should give it a bit more thought. Think through what functions can support WebAssembly. Do some benchmarking to see if it is worth the effort or too much of an edge case. Think through if all JS engines do have support for this or not. Etc. Also, I do not understand the binary code inside const wasmSrc = new Uint8Array(...), so we should somehow come up with a solution that gives us maintainable code. What do you think?

Copy link
Collaborator

@harrysarson harrysarson left a comment

Choose a reason for hiding this comment

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

Thanks for this! My gut feeling is that development of a wasm enhanced mathjs is probably best done outside of this main repository. Certainly I am against embedding binary blobs in the source code and I am probably also against introducing a new language (c, etc) into the project with all the hassle of maintaining the build chain that comes with it.

What are your thoughts Jos?

@@ -61,11 +61,22 @@ export const createDot = /* #__PURE__ */ factory(name, dependencies, ({ typed, a
if (xSize[0] !== ySize[0]) throw new RangeError('Vectors must have equal length (' + xSize[0] + ' != ' + ySize[0] + ')')
if (len === 0) throw new RangeError('Cannot calculate the dot product of empty vectors')

let prod = 0
const wasmSrc = new Uint8Array([0, 97, 115, 109, 1, 0, 0, 0, 1, 8, 1, 96, 3, 127, 127, 127, 1, 124, 3, 2, 1, 0, 5, 4, 1, 1, 1, 100, 7, 16, 2, 6, 109, 101, 109, 111, 114, 121, 2, 0, 3, 100, 111, 116, 0, 0, 10, 60, 1, 58, 2, 2, 127, 1, 124, 32, 2, 65, 0, 74, 4, 64, 3, 64, 32, 5, 32, 0, 32, 3, 65, 3, 116, 34, 4, 106, 43, 3, 0, 32, 1, 32, 4, 106, 43, 3, 0, 162, 160, 33, 5, 32, 3, 65, 1, 106, 34, 3, 32, 2, 71, 13, 0, 11, 11, 32, 5, 11, 11, 9, 1, 0, 65, 128, 12, 11, 2, 160, 6])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Whilst I like the idea of using wasm in principle, embedding compiled assets into the source of this library would make it impossible for folk to verify mathjs and also make it very hard to maintain.

@GirkovArpa GirkovArpa closed this May 1, 2020
@josdejong
Copy link
Owner

My gut feeling is that development of a wasm enhanced mathjs is probably best done outside of this main repository.

Thanks for your input. I think you're right, best approach would probably be to integrate an external library with WebAssembly support, similar to how we integrate a BigNumber and Complex number libraries.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants