elixir-nodejs icon indicating copy to clipboard operation
elixir-nodejs copied to clipboard

Async function call performance

Open mwojtul opened this issue 5 years ago • 4 comments

I wrote a JS function that fetches some data and uses it to render a React/Vue component. I decided to do some load testing and for consistency's sake I mocked the data call with a promise that returns after 40ms.

50 virtual users sending 5 requests each:

artillery quick --count 50 -n 5 http://localhost:4000/render

Results with a pool size of 4:

Summary report @ 02:09:01(-0500) 2019-05-16 Scenarios launched: 50 Scenarios completed: 50 Requests completed: 250 RPS sent: 84.46 Request latency: min: 44.3 max: 512.7 median: 357.2 p95: 483.4 p99: 503.2 Scenario counts: 0: 50 (100%) Codes: 200: 250

I haven't worked much with OTP yet but it seems like each worker needs to wait for the async request to resolve. The same behavior can be observed in test "gets resolved value". Calling NodeJS.call("slow-async-echo", [1234]) a second time will add another second to the test execution time.

@bryanjos Any way around this? I'd be happy to take a stab at a PR or help in whatever way. Figured I'd inquire if anything jumps out to you.

mwojtul avatar May 16 '19 07:05 mwojtul

@mwojtul nothing really jumps out to me. If by each worker needs to wait for the async request to resolve you mean that it waits for the promise to resolve, I think you are correct. I think that's how it's implemented here today. A PR is welcomed if you decide to give it a shot! I'll assign this to @jwietelmann to keep it on his radar

bryanjos avatar May 21 '19 13:05 bryanjos

I know it's been forever since anyone responded, but I have a feeling this is because of our undocumented environment variable optimization: https://github.com/revelrylabs/elixir-nodejs/blob/master/priv/server.js#L6

We blow away require's cache on every call unless you set NODE_ENV=production.

jwietelmann avatar Feb 05 '20 20:02 jwietelmann

I tried setting NODE_ENV to production, but so far I'm seeing the same results. Maybe I didn't do it correctly? I also edited the server.js file directly as well and didn't observe any improvements, so I'm thinking that might not be it.

mwojtul avatar Apr 26 '20 09:04 mwojtul

I looked into this some time ago with elixir_react_render and its definitely tied to the way Task works/used.

https://github.com/revelrylabs/elixir-nodejs/blob/master/lib/nodejs/supervisor.ex#L52-L65

As you can see in the code above, NodeJS.call is a pretty much a synchronous blocking call which awaits for the JS code to finish so that we can return the result. If you want to view it as JS, its the equivalent of, which would take about 2s to complete:

async function call() {
  return new Promise.resolve((resolve) =>
    setTimeout(() => resolve(Math.random()), 1000)
  )
}

await call()
await call()

I think we'd need to change or add something to make it not await in the call but return the Task itself. I didn't have that much time to experiment and try out that solution to see if it was valid and solved the problem. This does mean that we'd have to handle the await in another location or towards the end when we render everything after kicking off each task.

oohnoitz avatar May 15 '20 22:05 oohnoitz