This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Add export/import for function pointer table
ClosedPublic

Authored by ncw on Mar 13 2018, 7:10 AM.

Details

Summary

This enables callback-style programming where the JavaScript environment can call back into the Wasm environment using a function pointer received from the module.


Currently Emscripten post-processes the Wasm file to add stub functions to enable this. Alon was concerned that when the function table was exported, performance might suffer? I'll do some quick tests to confirm this.

Edit: I've now done some benchmarking. Whether the table is exported or not has no observable effect on performance that I could find (results: https://github.com/kripken/emscripten/issues/6155)

Whether you use the JS Table object to jump into Wasm, rather than using a named dynCall stub, may have a performance effect, but exporting the table doesn't force that decision in any way.

Event Timeline

ncw created this revision.Mar 13 2018, 7:10 AM
ncw edited the summary of this revision. (Show Details)Mar 13 2018, 8:51 AM

I don't think import or export should be on by default (even if emscripten decides to do that) but the option should be there. It's still pretty early days for the VMs and just because they don't optimize better today doesn't mean they never will.

ncw added a comment.Mar 13 2018, 10:17 AM

Since callback APIs are so common (eg high usage of dynCall in Emscripten applications) it would be nice if engines could optimise table-based access so that it's usable. Why standardise an API for doing an indirect call from JS, then tell users, "write a Wasm trampoline instead since that's faster than the browser-provided method".

I agree that things could get a lot faster in future - but it would be a shame (to me) if the official API were to become uncompetitively slow, so that people don't want to use it.

Agreed that I can put it behind a flag if you'd prefer.

lgtm, but I agree we should remove the export-by-default. At least for now, if only to keep this change small.

That way this change is also NFC for existing users, which is a nice feature.

ncw updated this revision to Diff 138389.Mar 14 2018, 9:19 AM

Updated to not export by default, and creates export/import based on last-specified of --export-table and --import-table

sbc100 accepted this revision.Mar 14 2018, 10:30 AM
This revision is now accepted and ready to land.Mar 14 2018, 10:30 AM
This revision was automatically updated to reflect the committed changes.
ruiu added a subscriber: ruiu.Mar 27 2018, 11:00 AM
ruiu added inline comments.
wasm/Driver.cpp
300

Config members are named after their command line options, so this member should be "ImportTable" or "ExportTable" and have a boolean value.

wasm/Writer.cpp
41

Does it make sense to add const to constexpr?

ncw added inline comments.Mar 28 2018, 5:57 AM
wasm/Driver.cpp
300

Good point. In this case, it's a tri-state, where the Table is either imported/exported, or neither, and I want to import/export based on the last-specified of those two options.

I'll expand that into two booleans ImportTable and ExportTable, if that's more idiomatic.

Done in rLLD328700.

wasm/Writer.cpp
41

Yes it does (in this case).

constexpr char * would be a const pointer to a non-const char.

constexpr const char * is equivalent to const char* const ie a const pointer to a const char.

Something like constexpr const int is silly/redundant, and constexpr const char *const would also be redundant.