From 42475db46725468d3f2769fbeba77615454646f3 Mon Sep 17 00:00:00 2001 From: Jean Rouge Date: Wed, 22 Jun 2016 11:32:58 -0700 Subject: [PATCH 1/2] Making it possible to set V8 runtime flags Only caveat: such flags can only be set before the V8 platform gets initialized. Comes in handy to deactivate for example V8's aggressive use of `pthread` which somehow seems to not play too nice with Unicorn: ``` MiniRacer::Platform.set_flag! '--noconcurrent_recompilation' MiniRacer::Platform.set_flag! '--noconcurrent_sweeping' ``` Also protecting the platform initialization with a mutex, not sure how/why current concurrent tests never seem to hit bugs without that mutex. Added just one test on the new `MiniRacer::Platform.set_flag!` method, and unsure how to make other tests without making tests run in order (bad), given that flags can only be set before anything else is done. --- .../mini_racer_extension.cc | 43 +++++++++++++++++-- lib/mini_racer.rb | 4 ++ test/mini_racer_test.rb | 9 ++++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/ext/mini_racer_extension/mini_racer_extension.cc b/ext/mini_racer_extension/mini_racer_extension.cc index 9b56fc1..9a9bf26 100644 --- a/ext/mini_racer_extension/mini_racer_extension.cc +++ b/ext/mini_racer_extension/mini_racer_extension.cc @@ -6,6 +6,7 @@ #include #include #include +#include using namespace v8; @@ -52,18 +53,48 @@ static VALUE rb_eParseError; static VALUE rb_eScriptRuntimeError; static VALUE rb_cJavaScriptFunction; static VALUE rb_eSnapshotError; +static VALUE rb_ePlatformAlreadyInitializedError; static VALUE rb_cDateTime = Qnil; static Platform* current_platform = NULL; +static std::mutex platform_lock; + +static VALUE rb_platform_set_flag(VALUE _klass, VALUE flag_as_str) { + bool platform_already_initialized = false; + + platform_lock.lock(); + + if (current_platform == NULL) { + V8::SetFlagsFromString(RSTRING_PTR(flag_as_str), (int)RSTRING_LEN(flag_as_str)); + } else { + platform_already_initialized = true; + } + + platform_lock.unlock(); + + // important to raise outside of the lock + if (platform_already_initialized) { + rb_raise(rb_ePlatformAlreadyInitializedError, "The V8 platform is already initialized"); + } + + return Qnil; +} static void init_v8() { + // no need to wait for the lock if already initialized + if (current_platform != NULL) return; + + platform_lock.lock(); + if (current_platform == NULL) { - V8::InitializeICU(); - current_platform = platform::CreateDefaultPlatform(); - V8::InitializePlatform(current_platform); - V8::Initialize(); + V8::InitializeICU(); + current_platform = platform::CreateDefaultPlatform(); + V8::InitializePlatform(current_platform); + V8::Initialize(); } + + platform_lock.unlock(); } void* breaker(void *d) { @@ -720,6 +751,7 @@ extern "C" { VALUE rb_mMiniRacer = rb_define_module("MiniRacer"); VALUE rb_cContext = rb_define_class_under(rb_mMiniRacer, "Context", rb_cObject); VALUE rb_cSnapshot = rb_define_class_under(rb_mMiniRacer, "Snapshot", rb_cObject); + VALUE rb_cPlatform = rb_define_class_under(rb_mMiniRacer, "Platform", rb_cObject); VALUE rb_eEvalError = rb_define_class_under(rb_mMiniRacer, "EvalError", rb_eStandardError); rb_eScriptTerminatedError = rb_define_class_under(rb_mMiniRacer, "ScriptTerminatedError", rb_eEvalError); @@ -727,6 +759,7 @@ extern "C" { rb_eScriptRuntimeError = rb_define_class_under(rb_mMiniRacer, "RuntimeError", rb_eEvalError); rb_cJavaScriptFunction = rb_define_class_under(rb_mMiniRacer, "JavaScriptFunction", rb_cObject); rb_eSnapshotError = rb_define_class_under(rb_mMiniRacer, "SnapshotError", rb_eStandardError); + rb_ePlatformAlreadyInitializedError = rb_define_class_under(rb_mMiniRacer, "PlatformAlreadyInitialized", rb_eStandardError); VALUE rb_cExternalFunction = rb_define_class_under(rb_cContext, "ExternalFunction", rb_cObject); rb_define_method(rb_cContext, "stop", (VALUE(*)(...))&rb_context_stop, 0); @@ -741,6 +774,8 @@ extern "C" { rb_define_method(rb_cSnapshot, "size", (VALUE(*)(...))&rb_snapshot_size, 0); rb_define_method(rb_cSnapshot, "warmup", (VALUE(*)(...))&rb_snapshot_warmup, 1); rb_define_private_method(rb_cSnapshot, "load", (VALUE(*)(...))&rb_snapshot_load, 1); + + rb_define_singleton_method(rb_cPlatform, "set_flag!", (VALUE(*)(...))&rb_platform_set_flag, 1); } } diff --git a/lib/mini_racer.rb b/lib/mini_racer.rb index 2cecc25..17bf278 100644 --- a/lib/mini_racer.rb +++ b/lib/mini_racer.rb @@ -8,6 +8,7 @@ module MiniRacer class ScriptTerminatedError < EvalError; end class ParseError < EvalError; end class SnapshotError < StandardError; end + class PlatformAlreadyInitialized < StandardError; end class RuntimeError < EvalError def initialize(message) @@ -39,6 +40,9 @@ module MiniRacer end end + # `::set_flag!` is defined in the C class + class Platform; end + # eval is defined in the C class class Context diff --git a/test/mini_racer_test.rb b/test/mini_racer_test.rb index 2a43b70..2e6d8c0 100644 --- a/test/mini_racer_test.rb +++ b/test/mini_racer_test.rb @@ -357,4 +357,13 @@ raise FooError, "I like foos" assert_equal 1, context.eval("Math.sin") end + + def test_platform_set_flag_raises_an_exception_if_already_initialized + # makes sure it's initialized + MiniRacer::Snapshot.new + + assert_raises(MiniRacer::PlatformAlreadyInitialized) do + MiniRacer::Platform.set_flag!("--noconcurrent_recompilation") + end + end end From 3762c8c2fdef99bb5976c3bf3bd594698149aae9 Mon Sep 17 00:00:00 2001 From: Jean Rouge Date: Mon, 27 Jun 2016 17:51:00 -0700 Subject: [PATCH 2/2] README on runtime flags + more tests on them Also changed the API for them. --- README.md | 41 ++++++++++++ .../mini_racer_extension.cc | 4 +- lib/mini_racer.rb | 32 +++++++++- test/mini_racer_test.rb | 62 ++++++++++++++++--- 4 files changed, 128 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index f43d93a..241aa44 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,47 @@ puts context.eval("counter") ``` +### V8 Runtime flags + +It is possible to set V8 Runtime flags: + +```ruby +MiniRacer::Platform.set_flags! :noconcurrent_recompilation, max_inlining_levels: 10 +``` + +This can come in handy if you want to use MiniRacer with Unicorn, which doesn't seem to alwatys appreciate V8's liberal use of threading: +```ruby +MiniRacer::Platform.set_flags! :noconcurrent_recompilation, :noconcurrent_sweeping +``` + +Or else to unlock experimental features in V8, for example tail recursion optimization: +```ruby +MiniRacer::Platform.set_flags! :harmony + +js = <<-JS +'use strict'; +var f = function f(n){ + if (n <= 0) { + return 'foo'; + } + return f(n - 1); +} + +f(1e6); +JS + +context = MiniRacer::Context.new + +context.eval js +# => "foo" +``` +The same code without the harmony runtime flag results in a `MiniRacer::RuntimeError: RangeError: Maximum call stack size exceeded` exception. +Please refer to http://node.green/ as a reference on other harmony features. + +A list of all V8 runtime flags can be found using `node --v8-options`, or else by perusing [the V8 source code for flags (make sure to use the right version of V8)](https://github.com/v8/v8/blob/master/src/flag-definitions.h). + +Note that runtime flags must be set before any other operation (e.g. creating a context, a snapshot or an isolate), otherwise an exception will be thrown. + ## Performance The `bench` folder contains benchmark. diff --git a/ext/mini_racer_extension/mini_racer_extension.cc b/ext/mini_racer_extension/mini_racer_extension.cc index 9a9bf26..cccd7c7 100644 --- a/ext/mini_racer_extension/mini_racer_extension.cc +++ b/ext/mini_racer_extension/mini_racer_extension.cc @@ -60,7 +60,7 @@ static VALUE rb_cDateTime = Qnil; static Platform* current_platform = NULL; static std::mutex platform_lock; -static VALUE rb_platform_set_flag(VALUE _klass, VALUE flag_as_str) { +static VALUE rb_platform_set_flag_as_str(VALUE _klass, VALUE flag_as_str) { bool platform_already_initialized = false; platform_lock.lock(); @@ -775,7 +775,7 @@ extern "C" { rb_define_method(rb_cSnapshot, "warmup", (VALUE(*)(...))&rb_snapshot_warmup, 1); rb_define_private_method(rb_cSnapshot, "load", (VALUE(*)(...))&rb_snapshot_load, 1); - rb_define_singleton_method(rb_cPlatform, "set_flag!", (VALUE(*)(...))&rb_platform_set_flag, 1); + rb_define_singleton_method(rb_cPlatform, "set_flag_as_str!", (VALUE(*)(...))&rb_platform_set_flag_as_str, 1); } } diff --git a/lib/mini_racer.rb b/lib/mini_racer.rb index 17bf278..950ced4 100644 --- a/lib/mini_racer.rb +++ b/lib/mini_racer.rb @@ -40,8 +40,36 @@ module MiniRacer end end - # `::set_flag!` is defined in the C class - class Platform; end + # `::` is defined in the C class + class Platform + class << self + def set_flags!(*args, **kwargs) + flags_to_strings([args, kwargs]).each do |flag| + # defined in the C class + set_flag_as_str!(flag) + end + end + + private + + def flags_to_strings(flags) + flags.flatten.map { |flag| flag_to_string(flag) }.flatten + end + + # normalize flags to strings, and adds leading dashes if needed + def flag_to_string(flag) + if flag.is_a?(Hash) + flag.map do |key, value| + "#{flag_to_string(key)} #{value}" + end + else + str = flag.to_s + str = "--#{str}" unless str.start_with?('--') + str + end + end + end + end # eval is defined in the C class class Context diff --git a/test/mini_racer_test.rb b/test/mini_racer_test.rb index 2e6d8c0..7507e66 100644 --- a/test/mini_racer_test.rb +++ b/test/mini_racer_test.rb @@ -1,6 +1,8 @@ require 'test_helper' class MiniRacerTest < Minitest::Test + # see `test_platform_set_flags_works` below + MiniRacer::Platform.set_flags! :use_strict def test_that_it_has_a_version_number refute_nil ::MiniRacer::VERSION @@ -25,7 +27,7 @@ class MiniRacerTest < Minitest::Test def test_object context = MiniRacer::Context.new # remember JavaScript is quirky {"1" : 1} magically turns to {1: 1} cause magic - assert_equal({1 => 2, "two" => "two"}, context.eval('a={"1" : 2, "two" : "two"}')) + assert_equal({1 => 2, "two" => "two"}, context.eval('var a={"1" : 2, "two" : "two"}; a')) end def test_it_returns_runtime_error @@ -77,7 +79,7 @@ class MiniRacerTest < Minitest::Test def test_returns_javascript_function context = MiniRacer::Context.new - assert_equal MiniRacer::JavaScriptFunction, context.eval("a = function(){}").class + assert_same MiniRacer::JavaScriptFunction, context.eval("var a = function(){}; a").class end def test_it_handles_malformed_js @@ -108,18 +110,19 @@ class MiniRacerTest < Minitest::Test def test_can_attach_functions context = MiniRacer::Context.new + context.eval 'var adder' context.attach("adder", proc{|a,b| a+b}) assert_equal 3, context.eval('adder(1,2)') end def test_es6_arrow_functions context = MiniRacer::Context.new - assert_equal 42, context.eval('adder=(x,y)=>x+y; adder(21,21);') + assert_equal 42, context.eval('var adder=(x,y)=>x+y; adder(21,21);') end def test_concurrent_access context = MiniRacer::Context.new - context.eval('counter=0; plus=()=>counter++;') + context.eval('var counter=0; var plus=()=>counter++;') (1..10).map do Thread.new { @@ -154,18 +157,21 @@ raise FooError, "I like foos" def test_attached_on_object context = MiniRacer::Context.new + context.eval 'var minion' context.attach("minion.speak", proc{"banana"}) assert_equal "banana", context.eval("minion.speak()") end def test_attached_on_nested_object context = MiniRacer::Context.new + context.eval 'var minion' context.attach("minion.kevin.speak", proc{"banana"}) assert_equal "banana", context.eval("minion.kevin.speak()") end def test_return_arrays context = MiniRacer::Context.new + context.eval 'var nose' context.attach("nose.type", proc{["banana",["nose"]]}) assert_equal ["banana", ["nose"]], context.eval("nose.type()") end @@ -259,13 +265,14 @@ raise FooError, "I like foos" def test_can_attach_method context = MiniRacer::Context.new + context.eval 'var Echo' context.attach("Echo.say", Echo.method(:say)) assert_equal "hello", context.eval("Echo.say('hello')") end def test_attach_error context = MiniRacer::Context.new - context.eval("minion = 2") + context.eval("var minion = 2") assert_raises do begin context.attach("minion.kevin.speak", proc{"banana"}) @@ -358,12 +365,53 @@ raise FooError, "I like foos" assert_equal 1, context.eval("Math.sin") end - def test_platform_set_flag_raises_an_exception_if_already_initialized + def test_platform_set_flags_raises_an_exception_if_already_initialized # makes sure it's initialized MiniRacer::Snapshot.new assert_raises(MiniRacer::PlatformAlreadyInitialized) do - MiniRacer::Platform.set_flag!("--noconcurrent_recompilation") + MiniRacer::Platform.set_flags! :noconcurrent_recompilation end end + + def test_platform_set_flags_works + context = MiniRacer::Context.new + + assert_raises(MiniRacer::RuntimeError) do + # should fail because of strict mode set for all these tests + context.eval 'x = 28' + end + end + + class TestPlatform < MiniRacer::Platform + def self.public_flags_to_strings(flags) + flags_to_strings(flags) + end + end + + def test_platform_flags_to_strings + flags = [ + :flag1, + [[[:flag2]]], + {key1: :value1}, + {key2: 42, + key3: 8.7}, + '--i_already_have_leading_hyphens', + [:'--me_too', + 'i_dont'] + ] + + expected_string_flags = [ + '--flag1', + '--flag2', + '--key1 value1', + '--key2 42', + '--key3 8.7', + '--i_already_have_leading_hyphens', + '--me_too', + '--i_dont' + ] + + assert_equal expected_string_flags, TestPlatform.public_flags_to_strings(flags) + end end