[testing] Execjs runtime benchmarks #290
Conversation
…to execjs-benchmark
| React::ServerRendering.renderer_options = {code: JS_CODE} | ||
| React::ServerRendering.pool_timeout = 10 | ||
|
|
||
| def test_runtime(runtime, renders:, pool_size:) |
There was a problem hiding this comment.
would be nice to have these explicit
There was a problem hiding this comment.
what do you mean? I think keyword args are good for expressing meaning
| React::ServerRendering.renderer_options = {code: JS_CODE} | ||
| React::ServerRendering.pool_timeout = 1000 | ||
|
|
||
| def test_runtime(runtime, renders:, pool_size:, threaded:) |
There was a problem hiding this comment.
I meant with the default values to be explicit- nil. Just easier to read, nothing specific :-)
|
Great script. When running the tests though, my ruby process now deadlocks consistently when pool size > 1 and using the threaded version of the test. This is a much easier way to reproduce the problem then trying to stress the entire web stack. Looking through the thread stacks in gdb it looks like the deadlock problem is something with libv8 itself. Most of those threads look OK, just the ruby interpreter doing it's threading thing, BUT threads 10 and 4 seem suspicious. |
…to execjs-benchmark
|
Yeah, I have lock problems sometimes too, I couldn't even get
|
|
I just made a pull request with some tweaks to the script that should make it execute more like how a web server would be calling into ExecJS. Should address your concern about creating lots of threads. I also agree that connection_pool seems to not be doing much. |
Execjs benchmark
|
💸 mine keeps locking here: But I get the point anyways! @johnthethird, it looks like ExecJS/V8 has a lock which nullifies the use of |
|
I've been testing both with a connection pool and without a connection pool running 10000 renders / test. I modified the script slightly so that when using more than one thread it executes 10000 / # of threads renders per thread, so each test should be calling render the same amount of times. The first results are without using a connection_pool and the second is with using the pool. You can clearly see the performance degradation when using the pool, I would definitely go ahead and remove it. I was able to fix the deadlock issue by wrapping all calls to ExecJS with Mutex#synchronize so that only one thread can be compiling or rendering at the same time. It seems silly to have to also provide our own 'GIL' for interacting with ExecJS, but it definitely fixes the deadlock problem, and it only seems to matter for therubyracer, none of the other runtimes would deadlock. |
|
@rmosolgo I wouldnt think the connection pool would matter much on MRI. (Also, V8 has its own GIL, similar to MRI). But JRuby is a different story. The connection pool gem is battle hardened from its use in Sidekiq, works great on JRuby/JVM, and is used to mediate access to potentially long-running external resources/processes. So it was a perfect fit for mediating access to the JS engine, or so I thought. Ill try to come up with a benchmark that shows what happens with a truly multi-threaded ruby runtime. |
|
@johnthethird I thought about JRuby as well, but wouldn't the GIL in V8 negate any sort of performance from the JRuby threads? Regardless of accessing an ExecJS context through connection_pool or not they will still share the same GIL in V8. |
|
Jruby has TheRubyRhino which is wicked fast and multithreaded, and lives in the same JVM. |
|
Thanks @johnthethird , I forgot about JRuby! Here's the benchmark with Ruby Rhino & Node.js on jruby: |
|
Gotcha, that doesn't use V8 at all so is not subject to its GIL. The whole reason this pull request exists is because using ExecJS w/ therubyracer in a threaded environment exhibits a deadlock. So while trying to debug/figure out what/where the problem is we discovered that using the connection_pool with MRI actually hurts performance. So it looks like perhaps the best solution is one where a connection_pool is used when the underlying JS runtime is thread-safe and JRuby is being used, otherwise don't bother because it hurts performance on MRI. The issue of therubyracer deadlocking still isn't fixed though. until the real bug is fixed whether it is in therubyracer, libv8, or MRI itself, there needs to be some locking going on inside of react-rails to prevent against concurrent calls to anything in ExecJS. Perhaps that could also be conditional on whether or not therubyracer is being used so that we don't step in the way of JRuby. |
|
Here's a full test run using jruby and therubyrhino running 20k renders per test. Node.js is so slow that I'm not bothering to test against it. I altered the script test loop order to change the size of the connection pool before it changes the thread count and you can clearly see the performance benefit of using a pool size >= thread count by looking at the real column. My machine has hyperthreading so I guess that's why everything above 4 threads fails to show any improvement. |
|
Here's a full run using MRI(2.2.1). Maybe I'm doing something wrong with JRuby, but that performance is quite bad. I was using version 9.0.0.0.pre2 installed through rbenv and the runtime numbers definitely show it was utilizing more than one CPU core at a time(I could see through sysmon tools as well this was the case). So MRI running on one CPU core is almost 4x faster than JRuby running on all 4 CPU cores?? There's obviously something else going on... right? The way this benchmark script is setup it pretty much purely tests the performance of the associated JS runtimes, so does that mean Rhino is that much slower than V8, at least when used in this context? |
|
I replaced the JS code that is being rendered with and now I get the following results for JRuby and for MRI Those results make more sense, so my conclusion is that Math.random() causes drastically different performance on the two JS runtimes. |
|
Yea, V8 is a lot faster than Rhino, from a pure speed perspective. The new JS engine in Java 8 (Nashorn) approaches V8 in speed. Focusing on just the pool concept, I changed to benchmark script to show the advantages of a pool (I had to normalize the renders so that each test is doing the same number of total renders taking into account the number of threads, to make it a fair test. Also, instead of Math.random do some more real life React things.) https://gist.github.com/johnthethird/19074284591b6091e367 The thing to note in the results, is that 10 threads against a size 10 pool, makes a huge difference, obviously, for JRuby and for even for MRI if it uses an out-of-process JS engine like Node. Re the MRI deadlock issue, it looks like not only does V8 have a GIL of its own but ExecJS also has locks around https://github.com/sstephenson/execjs/blob/master/lib/execjs/ruby_racer_runtime.rb#L32 So Im not sure why putting a lock around our render fixes the deadlock, but it sure does. For MRI/RubyRacer, probably the right thing to do is have a pool size of 1. Having the pool there shouldnt really hurt performance, as the overhead of checking in/out of the pool is minimal. The question is whether to make this a hard-coded limit or just issue a warning if pool size > 1 when using RubyRacer. |
|
How's this for takeaways:
?? |
|
👍 Perhaps also include a note about the deadlock condition if you configure Pool Size > 1 with RubyRacer on MRI. |
|
Per my comments on #222, I'm definitely throwing my weight behind officially recommending therubyracer on MRI. Should definitely throw in a note there saying that you should do it even though the Heroku docs say not to, to avoid chumps like me thinking one suggestion overrides the other. |
|
Anybody want to cook up a PR 😬 ?? TODO:
|
|
Updating code & docs here: https://github.com/rmosolgo/react-rails/commit/850f2b89695cc0974a7444834db815b0b2cb6e89 |
compilation of libv8 takes allot of time though reactjs/react-rails#290
|
People finding this thread later should note that per #886 'therubyracer' should not be used anymore. |
|
👍 @BobbyMcWho Yep! It was deprecated because it only supports LibV8 version 3, when the current version is v5. It's also notorious for segfaulting your servers in production. MiniRacer is the current recommendation and it was even a little faster since it's using a newer version of libV8. If you remind me in a few, I might get a benchmark out of it later. |
(I don't really want to merge this, I just want to share some code & results)
I ran some tests to compare ExecJS runtimes & connection pool sizes:
ExecJSRendererto sidestep the dependency on Rails asset pipelineResults
Sorted
Takeaways