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

wasm: 3x performance overhead of using webassembly in node 8 #26277

Open
trashhalo opened this Issue Jul 8, 2018 · 6 comments

Comments

Projects
None yet
4 participants
@trashhalo

trashhalo commented Jul 8, 2018

Im really not sure where to submit this issue, golang, node or chromium. So I am starting here. Feel free to close the issue and provide feedback if you think I should go elsewhere.

What version of Go are you using (go version)?

Master Head // b001ffb

Does this issue reproduce with the latest release?

N/A latest release does not have wasm.

What operating system and processor architecture are you using (go env)?

wasm on linux
node 8.11

What did you do?

I built a connection from webassembly to node http and benchmarked it against native http version

recipe for reproducing
https://github.com/trashhalo/go_wasm_node_http

benchmark results
https://gist.github.com/trashhalo/b2f120fb9d20bd4003bf125c0200b601

node hello world
https://gist.github.com/thebinarypenguin/122cda772438c80ad683

What did you expect to see?

I expected a performance hit but not one so large

What did you see instead?

A 3x hit to performance of equivalent javascript code.

@trashhalo

This comment has been minimized.

Show comment
Hide comment
@trashhalo

trashhalo Jul 8, 2018

If you want to skip the docker build
app.wasm.gz

trashhalo commented Jul 8, 2018

If you want to skip the docker build
app.wasm.gz

@agnivade

This comment has been minimized.

Show comment
Hide comment
@agnivade

agnivade Jul 8, 2018

Member

If I understand this correctly, there is very little "actual" webassembly here. This is still running a node http server, in a very roundabout fashion. What I believe you are seeing is the js-wasm transfer overhead.

/cc @neelance

Member

agnivade commented Jul 8, 2018

If I understand this correctly, there is very little "actual" webassembly here. This is still running a node http server, in a very roundabout fashion. What I believe you are seeing is the js-wasm transfer overhead.

/cc @neelance

@agnivade agnivade added the Arch-Wasm label Jul 8, 2018

@neelance

This comment has been minimized.

Show comment
Hide comment
@neelance

neelance Jul 8, 2018

Member

I bet a good portion of it is due to the setTimeout here:

setTimeout(this._resolveCallbackPromise, 0); // make sure it is asynchronous

Performance optimizations were not yet in scope.

Member

neelance commented Jul 8, 2018

I bet a good portion of it is due to the setTimeout here:

setTimeout(this._resolveCallbackPromise, 0); // make sure it is asynchronous

Performance optimizations were not yet in scope.

@trashhalo

This comment has been minimized.

Show comment
Hide comment
@trashhalo

trashhalo Jul 8, 2018

@neelance I played around to see if I can shave off time.

Removing setTimeout didn't change too much. I ran it through chrome profiler and it seems the hotspot is loading strings out of the memory blob and decoding them on the js side. The strings in my example on the method names i want to call on my js values.

Perhaps some sort of constant pool is in order?

trashhalo commented Jul 8, 2018

@neelance I played around to see if I can shave off time.

Removing setTimeout didn't change too much. I ran it through chrome profiler and it seems the hotspot is loading strings out of the memory blob and decoding them on the js side. The strings in my example on the method names i want to call on my js values.

Perhaps some sort of constant pool is in order?

@neelance

This comment has been minimized.

Show comment
Hide comment
@neelance

neelance Jul 8, 2018

Member

Perhaps some sort of constant pool is in order?

Maybe. We might even need some additional type like this:

idFoo := js.ID("foo")
for i := 0; i < 1000000; i++ {
   v.CallByID(idFoo)
}

This way, we could avoid referencing a method by string each time.
But we should wait with this until WebAssembly has its improved JS interop features ready.

Member

neelance commented Jul 8, 2018

Perhaps some sort of constant pool is in order?

Maybe. We might even need some additional type like this:

idFoo := js.ID("foo")
for i := 0; i < 1000000; i++ {
   v.CallByID(idFoo)
}

This way, we could avoid referencing a method by string each time.
But we should wait with this until WebAssembly has its improved JS interop features ready.

@trashhalo

This comment has been minimized.

Show comment
Hide comment
@trashhalo

trashhalo Jul 9, 2018

That would be cool!

I dont know how safe this is but I changed loadString in the wasm_exec to have a js side string cache. Its not bombing... and now im getting 8-10k requests/second, vs original implementation 5540.49.

const strCache = {};
const loadString = (addr) => {
	const saddr = getInt64(addr + 0);
	const cache = strCache[saddr.toString()];
	if (cache) {
		return cache;
	}
	const len = getInt64(addr + 8);
	const ret = decoder.decode(new DataView(this._inst.exports.mem.buffer, saddr, len));
	strCache[saddr.toString()] = ret;
	return ret;
}

trashhalo commented Jul 9, 2018

That would be cool!

I dont know how safe this is but I changed loadString in the wasm_exec to have a js side string cache. Its not bombing... and now im getting 8-10k requests/second, vs original implementation 5540.49.

const strCache = {};
const loadString = (addr) => {
	const saddr = getInt64(addr + 0);
	const cache = strCache[saddr.toString()];
	if (cache) {
		return cache;
	}
	const len = getInt64(addr + 8);
	const ret = decoder.decode(new DataView(this._inst.exports.mem.buffer, saddr, len));
	strCache[saddr.toString()] = ret;
	return ret;
}

@bradfitz bradfitz added the Performance label Jul 9, 2018

@bradfitz bradfitz added this to the Unplanned milestone Jul 9, 2018

@bradfitz bradfitz changed the title from wasm 3x performance overhead of using webassembly in node 8 to wasm: 3x performance overhead of using webassembly in node 8 Jul 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment