From 36dcb98b01251724b29aab79ea6f58dfc01c0dc6 Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Fri, 6 Dec 2019 23:28:55 +0000 Subject: [PATCH 01/13] Add stats to WASM VMs. Signed-off-by: John Plevyak --- bazel/external/wee8.genrule_cmd | 6 +- source/extensions/common/wasm/BUILD | 8 +++ source/extensions/common/wasm/null/BUILD | 1 + source/extensions/common/wasm/null/null.cc | 2 +- source/extensions/common/wasm/null/null.h | 2 +- source/extensions/common/wasm/null/null_vm.cc | 3 + source/extensions/common/wasm/null/null_vm.h | 15 +++-- source/extensions/common/wasm/v8/BUILD | 1 + source/extensions/common/wasm/v8/v8.cc | 36 +++++++++-- source/extensions/common/wasm/v8/v8.h | 2 +- source/extensions/common/wasm/wasm_vm.cc | 6 +- source/extensions/common/wasm/wasm_vm.h | 16 +++-- source/extensions/common/wasm/wasm_vm_base.h | 63 +++++++++++++++++++ test/extensions/common/wasm/wasm_vm_test.cc | 51 +++++++++------ 14 files changed, 169 insertions(+), 43 deletions(-) create mode 100644 source/extensions/common/wasm/wasm_vm_base.h diff --git a/bazel/external/wee8.genrule_cmd b/bazel/external/wee8.genrule_cmd index 323402b493927..9103dfb4d9fbc 100644 --- a/bazel/external/wee8.genrule_cmd +++ b/bazel/external/wee8.genrule_cmd @@ -72,7 +72,11 @@ WEE8_BUILD_ARGS+=" v8_use_external_startup_data=false" WEE8_BUILD_ARGS+=" v8_enable_shared_ro_heap=false" # Build wee8. -third_party/depot_tools/gn gen out/wee8 --args="$$WEE8_BUILD_ARGS" +if [[ `uname` == "Darwin" ]]; then + buildtools/mac/gn gen out/wee8 --args="$$WEE8_BUILD_ARGS" +else + buildtools/linux64/gn gen out/wee8 --args="$$WEE8_BUILD_ARGS" +fi third_party/depot_tools/ninja -C out/wee8 wee8 # Move compiled library to the expected destinations. diff --git a/source/extensions/common/wasm/BUILD b/source/extensions/common/wasm/BUILD index dbc0f83910be8..9333c679421f6 100644 --- a/source/extensions/common/wasm/BUILD +++ b/source/extensions/common/wasm/BUILD @@ -25,6 +25,14 @@ envoy_cc_library( ], ) +envoy_cc_library( + name = "wasm_vm_base", + hdrs = ["wasm_vm_base.h"], + deps = [ + "//source/common/stats:stats_lib", + ], +) + envoy_cc_library( name = "wasm_vm_lib", srcs = ["wasm_vm.cc"], diff --git a/source/extensions/common/wasm/null/BUILD b/source/extensions/common/wasm/null/BUILD index eed8e62d2e49b..6f93952bc9cfc 100644 --- a/source/extensions/common/wasm/null/BUILD +++ b/source/extensions/common/wasm/null/BUILD @@ -26,6 +26,7 @@ envoy_cc_library( "//external:abseil_node_hash_map", "//include/envoy/registry", "//source/common/common:assert_lib", + "//source/extensions/common/wasm:wasm_vm_base", "//source/extensions/common/wasm:wasm_vm_interface", "//source/extensions/common/wasm:well_known_names", ], diff --git a/source/extensions/common/wasm/null/null.cc b/source/extensions/common/wasm/null/null.cc index 185dde60780ab..1a2fc46924331 100644 --- a/source/extensions/common/wasm/null/null.cc +++ b/source/extensions/common/wasm/null/null.cc @@ -18,7 +18,7 @@ namespace Common { namespace Wasm { namespace Null { -WasmVmPtr createVm() { return std::make_unique(); } +WasmVmPtr createVm(Stats::ScopeSharedPtr scope) { return std::make_unique(scope); } } // namespace Null } // namespace Wasm diff --git a/source/extensions/common/wasm/null/null.h b/source/extensions/common/wasm/null/null.h index 7d88fb356923e..1fab2784f04bc 100644 --- a/source/extensions/common/wasm/null/null.h +++ b/source/extensions/common/wasm/null/null.h @@ -11,7 +11,7 @@ namespace Common { namespace Wasm { namespace Null { -WasmVmPtr createVm(); +WasmVmPtr createVm(Stats::ScopeSharedPtr scope); } // namespace Null } // namespace Wasm diff --git a/source/extensions/common/wasm/null/null_vm.cc b/source/extensions/common/wasm/null/null_vm.cc index d2cb16b994f16..c23d6d7bd0c48 100644 --- a/source/extensions/common/wasm/null/null_vm.cc +++ b/source/extensions/common/wasm/null/null_vm.cc @@ -7,6 +7,7 @@ #include "envoy/registry/registry.h" #include "common/common/assert.h" +#include "common/singleton/threadsafe_singleton.h" #include "extensions/common/wasm/null/null_vm_plugin.h" #include "extensions/common/wasm/well_known_names.h" @@ -17,6 +18,8 @@ namespace Common { namespace Wasm { namespace Null { +ThreadSafeSingleton global_stats_; + WasmVmPtr NullVm::clone() { auto cloned_null_vm = std::make_unique(*this); cloned_null_vm->load(plugin_name_, false /* unused */); diff --git a/source/extensions/common/wasm/null/null_vm.h b/source/extensions/common/wasm/null/null_vm.h index d23e332bcb77d..64f0e666978e1 100644 --- a/source/extensions/common/wasm/null/null_vm.h +++ b/source/extensions/common/wasm/null/null_vm.h @@ -7,8 +7,10 @@ #include "envoy/registry/registry.h" #include "common/common/assert.h" +#include "common/singleton/threadsafe_singleton.h" #include "extensions/common/wasm/null/null_vm_plugin.h" +#include "extensions/common/wasm/wasm_vm_base.h" #include "extensions/common/wasm/well_known_names.h" namespace Envoy { @@ -17,16 +19,21 @@ namespace Common { namespace Wasm { namespace Null { +extern ThreadSafeSingleton global_stats_; + // The NullVm wraps a C++ WASM plugin which has been compiled with the WASM API // and linked directly into the Envoy process. This is useful for development // in that it permits the debugger to set breakpoints in both Envoy and the plugin. -struct NullVm : public WasmVm { - NullVm() = default; - NullVm(const NullVm& other) : plugin_name_(other.plugin_name_) {} +struct NullVm : public WasmVmBase { + NullVm(Stats::ScopeSharedPtr scope) + : WasmVmBase(scope, &global_stats_.get(), WasmRuntimeNames::get().Null) {} + NullVm(const NullVm& other) + : WasmVmBase(other.scope_, &global_stats_.get(), WasmRuntimeNames::get().Null), + plugin_name_(other.plugin_name_) {} // WasmVm absl::string_view runtime() override { return WasmRuntimeNames::get().Null; } - bool cloneable() override { return true; }; + Cloneable cloneable() override { return Cloneable::InstantiatedModule; }; WasmVmPtr clone() override; bool load(const std::string& code, bool allow_precompiled) override; void link(absl::string_view debug_name) override; diff --git a/source/extensions/common/wasm/v8/BUILD b/source/extensions/common/wasm/v8/BUILD index fb5a473026402..04d0954d1b2cd 100644 --- a/source/extensions/common/wasm/v8/BUILD +++ b/source/extensions/common/wasm/v8/BUILD @@ -17,6 +17,7 @@ envoy_cc_library( ], deps = [ "//source/common/common:assert_lib", + "//source/extensions/common/wasm:wasm_vm_base", "//source/extensions/common/wasm:wasm_vm_interface", "//source/extensions/common/wasm:well_known_names", ], diff --git a/source/extensions/common/wasm/v8/v8.cc b/source/extensions/common/wasm/v8/v8.cc index 44e4a46f1989d..e7fbea3620d90 100644 --- a/source/extensions/common/wasm/v8/v8.cc +++ b/source/extensions/common/wasm/v8/v8.cc @@ -5,7 +5,9 @@ #include #include "common/common/assert.h" +#include "common/singleton/threadsafe_singleton.h" +#include "extensions/common/wasm/wasm_vm_base.h" #include "extensions/common/wasm/well_known_names.h" #include "absl/container/flat_hash_map.h" @@ -18,6 +20,8 @@ namespace Common { namespace Wasm { namespace V8 { +ThreadSafeSingleton global_stats_; + wasm::Engine* engine() { static const auto engine = wasm::Engine::make(); return engine.get(); @@ -33,9 +37,10 @@ struct FuncData { using FuncDataPtr = std::unique_ptr; -class V8 : public WasmVm { +class V8 : public WasmVmBase { public: - V8() = default; + V8(Stats::ScopeSharedPtr scope) + : WasmVmBase(scope, &global_stats_.get(), WasmRuntimeNames::get().V8) {} // Extensions::Common::Wasm::WasmVm absl::string_view runtime() override { return WasmRuntimeNames::get().V8; } @@ -44,9 +49,8 @@ class V8 : public WasmVm { absl::string_view getCustomSection(absl::string_view name) override; void link(absl::string_view debug_name) override; - // V8 is currently not cloneable. - bool cloneable() override { return false; } - WasmVmPtr clone() override { return nullptr; } + Cloneable cloneable() override { return Cloneable::CompiledBytecode; } + WasmVmPtr clone() override; uint64_t getMemorySize() override; absl::optional getMemory(uint64_t pointer, uint64_t size) override; @@ -89,6 +93,7 @@ class V8 : public WasmVm { wasm::vec source_ = wasm::vec::invalid(); wasm::own store_; wasm::own module_; + wasm::own> shared_module_; wasm::own instance_; wasm::own memory_; wasm::own table_; @@ -247,9 +252,28 @@ bool V8::load(const std::string& code, bool /* allow_precompiled */) { ::memcpy(source_.get(), code.data(), code.size()); module_ = wasm::Module::make(store_.get(), source_); + if (module_) { + shared_module_ = module_->share(); + RELEASE_ASSERT(shared_module_ != nullptr, ""); + } + return module_ != nullptr; } +WasmVmPtr V8::clone() { + ENVOY_LOG(trace, "clone()"); + ASSERT(shared_module_ != nullptr); + + auto clone = std::make_unique(scope_); + clone->store_ = wasm::Store::make(engine()); + RELEASE_ASSERT(clone->store_ != nullptr, ""); + + clone->module_ = wasm::Module::obtain(clone->store_.get(), shared_module_.get()); + RELEASE_ASSERT(clone->module_ != nullptr, ""); + + return clone; +} + absl::string_view V8::getCustomSection(absl::string_view name) { ENVOY_LOG(trace, "getCustomSection(\"{}\")", name); ASSERT(source_.get() != nullptr); @@ -562,7 +586,7 @@ void V8::getModuleFunctionImpl(absl::string_view function_name, }; } -WasmVmPtr createVm() { return std::make_unique(); } +WasmVmPtr createVm(Stats::ScopeSharedPtr scope) { return std::make_unique(scope); } } // namespace V8 } // namespace Wasm diff --git a/source/extensions/common/wasm/v8/v8.h b/source/extensions/common/wasm/v8/v8.h index 3650f190a73c8..1cee4424e8152 100644 --- a/source/extensions/common/wasm/v8/v8.h +++ b/source/extensions/common/wasm/v8/v8.h @@ -10,7 +10,7 @@ namespace Common { namespace Wasm { namespace V8 { -WasmVmPtr createVm(); +WasmVmPtr createVm(Stats::ScopeSharedPtr scope); } // namespace V8 } // namespace Wasm diff --git a/source/extensions/common/wasm/wasm_vm.cc b/source/extensions/common/wasm/wasm_vm.cc index 34f2745331440..88ee11c60bc0d 100644 --- a/source/extensions/common/wasm/wasm_vm.cc +++ b/source/extensions/common/wasm/wasm_vm.cc @@ -14,13 +14,13 @@ namespace Wasm { thread_local Envoy::Extensions::Common::Wasm::Context* current_context_ = nullptr; thread_local uint32_t effective_context_id_ = 0; -WasmVmPtr createWasmVm(absl::string_view runtime) { +WasmVmPtr createWasmVm(absl::string_view runtime, Stats::ScopeSharedPtr scope) { if (runtime.empty()) { throw WasmVmException("Failed to create WASM VM with unspecified runtime."); } else if (runtime == WasmRuntimeNames::get().Null) { - return Null::createVm(); + return Null::createVm(scope); } else if (runtime == WasmRuntimeNames::get().V8) { - return V8::createVm(); + return V8::createVm(scope); } else { throw WasmVmException(fmt::format( "Failed to create WASM VM using {} runtime. Envoy was compiled without support for it.", diff --git a/source/extensions/common/wasm/wasm_vm.h b/source/extensions/common/wasm/wasm_vm.h index 7cf58ed561550..45e5c08d0a81b 100644 --- a/source/extensions/common/wasm/wasm_vm.h +++ b/source/extensions/common/wasm/wasm_vm.h @@ -3,6 +3,7 @@ #include #include "envoy/common/exception.h" +#include "envoy/stats/scope.h" #include "common/common/logger.h" @@ -98,8 +99,7 @@ template using WasmCallWord = std::function) _f(WasmCallVoid<1>) _f(WasmCallVoid<2>) _f(WasmCallVoid<3>) \ - _f(WasmCallVoid<4>) _f(WasmCallVoid<5>) _f(WasmCallVoid<8>) _f(WasmCallWord<0>) \ - _f(WasmCallWord<1>) _f(WasmCallWord<2>) _f(WasmCallWord<3>) + _f(WasmCallVoid<5>) _f(WasmCallWord<1>) _f(WasmCallWord<2>) _f(WasmCallWord<3>) // Calls out of the WASM VM. // 1st arg is always a pointer to raw_context (void*). @@ -111,6 +111,7 @@ template using WasmCallbackWord = WasmFuncType* // Extended with W = Word // Z = void, j = uint32_t, l = int64_t, m = uint64_t using WasmCallback_WWl = Word (*)(void*, Word, int64_t); +using WasmCallback_WWlWW = Word (*)(void*, Word, int64_t, Word, Word); using WasmCallback_WWm = Word (*)(void*, Word, uint64_t); using WasmCallback_dd = double (*)(void*, double); @@ -119,8 +120,11 @@ using WasmCallback_dd = double (*)(void*, double); _f(WasmCallbackVoid<4>) _f(WasmCallbackWord<0>) _f(WasmCallbackWord<1>) \ _f(WasmCallbackWord<2>) _f(WasmCallbackWord<3>) _f(WasmCallbackWord<4>) \ _f(WasmCallbackWord<5>) _f(WasmCallbackWord<6>) _f(WasmCallbackWord<7>) \ - _f(WasmCallbackWord<8>) _f(WasmCallbackWord<9>) _f(WasmCallback_WWl) \ - _f(WasmCallback_WWm) _f(WasmCallback_dd) + _f(WasmCallbackWord<8>) _f(WasmCallbackWord<9>) _f(WasmCallbackWord<10>) \ + _f(WasmCallback_WWl) _f(WasmCallback_WWlWW) _f(WasmCallback_WWm) \ + _f(WasmCallback_dd) + +enum class Cloneable { NotCloneable, CompiledBytecode, InstantiatedModule }; // Wasm VM instance. Provides the low level WASM interface. class WasmVm : public Logger::Loggable { @@ -143,7 +147,7 @@ class WasmVm : public Logger::Loggable { * VM from scratch for each worker. * @return true if the VM is cloneable. */ - virtual bool cloneable() PURE; + virtual Cloneable cloneable() PURE; /** * Make a worker/thread-specific copy if supported by the underlying VM system (see cloneable() @@ -287,7 +291,7 @@ struct SaveRestoreContext { }; // Create a new low-level WASM VM using runtime of the given type (e.g. "envoy.wasm.runtime.wavm"). -WasmVmPtr createWasmVm(absl::string_view runtime); +WasmVmPtr createWasmVm(absl::string_view runtime, Stats::ScopeSharedPtr scope); } // namespace Wasm } // namespace Common diff --git a/source/extensions/common/wasm/wasm_vm_base.h b/source/extensions/common/wasm/wasm_vm_base.h new file mode 100644 index 0000000000000..4ad0950ffa72c --- /dev/null +++ b/source/extensions/common/wasm/wasm_vm_base.h @@ -0,0 +1,63 @@ +#pragma once + +#include "envoy/stats/scope.h" +#include "envoy/stats/stats.h" +#include "envoy/stats/stats_macros.h" + +#include "extensions/common/wasm/wasm_vm.h" + +#include "absl/strings/str_cat.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Wasm { + +/** + * Wasm host stats. + */ +#define ALL_VM_STATS(COUNTER, GAUGE) \ + COUNTER(created) \ + COUNTER(cloned) \ + GAUGE(active, NeverImport) + +struct VmStats { + ALL_VM_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT) +}; + +struct VmGlobalStats { + std::atomic active_; +}; + +// Wasm VM base instance. Provides common behavior (e.g. Stats). +class WasmVmBase : public WasmVm { +public: + WasmVmBase(Stats::ScopeSharedPtr scope, VmGlobalStats* global_stats_ptr, + absl::string_view runtime) + : scope_(scope), global_stats_ptr_(global_stats_ptr), + stats_(VmStats{ + ALL_VM_STATS(POOL_COUNTER_PREFIX(*scope_, absl::StrCat("wasm_vm.", runtime, ".")), + POOL_GAUGE_PREFIX(*scope_, absl::StrCat("wasm_vm.", runtime, ".")))}), + runtime_(std::string(runtime)) { + global_stats_ptr_->active_++; + stats_.created_.inc(); + stats_.active_.set(global_stats_ptr_->active_); + ENVOY_LOG(debug, "WasmVm created {} now active", runtime_, global_stats_ptr_->active_); + } + virtual ~WasmVmBase() { + global_stats_ptr_->active_--; + stats_.active_.set(global_stats_ptr_->active_); + ENVOY_LOG(debug, "~WasmVm {} {} remaining active", runtime_, global_stats_ptr_->active_); + } + +protected: + Stats::ScopeSharedPtr scope_; + VmGlobalStats* global_stats_ptr_; + VmStats stats_; + std::string runtime_; +}; + +} // namespace Wasm +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/common/wasm/wasm_vm_test.cc b/test/extensions/common/wasm/wasm_vm_test.cc index 1584406c89097..87085a97475ef 100644 --- a/test/extensions/common/wasm/wasm_vm_test.cc +++ b/test/extensions/common/wasm/wasm_vm_test.cc @@ -1,5 +1,7 @@ #include "envoy/registry/registry.h" +#include "common/stats/isolated_store_impl.h" + #include "extensions/common/wasm/null/null_vm_plugin.h" #include "extensions/common/wasm/wasm_vm.h" @@ -35,7 +37,7 @@ class PluginFactory : public Null::NullVmPluginFactory { }; TestNullVmPlugin* test_null_vm_plugin_ = nullptr; -REGISTER_FACTORY(PluginFactory, Null::NullVmPluginFactory); +Envoy::Registry::RegisterFactory register_; std::unique_ptr PluginFactory::create() const { auto result = std::make_unique(); @@ -43,29 +45,38 @@ std::unique_ptr PluginFactory::create() const { return result; } -TEST(BadVmTest, NoRuntime) { - EXPECT_THROW_WITH_MESSAGE(createWasmVm(""), WasmVmException, +class BaseVmTest : public testing::Test { +public: + BaseVmTest() : scope_(Stats::ScopeSharedPtr(stats_store.createScope("wasm."))) {} + +protected: + Stats::IsolatedStoreImpl stats_store; + Stats::ScopeSharedPtr scope_; +}; + +TEST_F(BaseVmTest, NoRuntime) { + EXPECT_THROW_WITH_MESSAGE(createWasmVm("", scope_), WasmVmException, "Failed to create WASM VM with unspecified runtime."); } -TEST(BadVmTest, BadRuntime) { - EXPECT_THROW_WITH_MESSAGE(createWasmVm("envoy.wasm.runtime.invalid"), WasmVmException, +TEST_F(BaseVmTest, BadRuntime) { + EXPECT_THROW_WITH_MESSAGE(createWasmVm("envoy.wasm.runtime.invalid", scope_), WasmVmException, "Failed to create WASM VM using envoy.wasm.runtime.invalid runtime. " "Envoy was compiled without support for it."); } -TEST(NullVmTest, NullVmStartup) { - auto wasm_vm = createWasmVm("envoy.wasm.runtime.null"); +TEST_F(BaseVmTest, NullVmStartup) { + auto wasm_vm = createWasmVm("envoy.wasm.runtime.null", scope_); EXPECT_TRUE(wasm_vm != nullptr); EXPECT_TRUE(wasm_vm->runtime() == "envoy.wasm.runtime.null"); - EXPECT_TRUE(wasm_vm->cloneable()); + EXPECT_TRUE(wasm_vm->cloneable() == Cloneable::InstantiatedModule); auto wasm_vm_clone = wasm_vm->clone(); EXPECT_TRUE(wasm_vm_clone != nullptr); EXPECT_TRUE(wasm_vm->getCustomSection("user").empty()); } -TEST(NullVmTest, NullVmMemory) { - auto wasm_vm = createWasmVm("envoy.wasm.runtime.null"); +TEST_F(BaseVmTest, NullVmMemory) { + auto wasm_vm = createWasmVm("envoy.wasm.runtime.null", scope_); EXPECT_EQ(wasm_vm->getMemorySize(), std::numeric_limits::max()); std::string d = "data"; auto m = wasm_vm->getMemory(reinterpret_cast(d.data()), d.size()).value(); @@ -114,26 +125,23 @@ Word bad_pong2(void*, Word) { return 2; } // pong() with wrong argument type. double bad_pong3(void*, double) { return 3; } -class WasmVmTest : public testing::Test { +class WasmVmTest : public BaseVmTest { public: void SetUp() override { g_host_functions = new MockHostFunctions(); } void TearDown() override { delete g_host_functions; } }; TEST_F(WasmVmTest, V8BadCode) { - auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8"); + auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8", scope_); ASSERT_TRUE(wasm_vm != nullptr); EXPECT_FALSE(wasm_vm->load("bad code", false)); } TEST_F(WasmVmTest, V8Code) { - auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8"); + auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8", scope_); ASSERT_TRUE(wasm_vm != nullptr); - EXPECT_TRUE(wasm_vm->runtime() == "envoy.wasm.runtime.v8"); - EXPECT_FALSE(wasm_vm->cloneable()); - EXPECT_TRUE(wasm_vm->clone() == nullptr); auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( "{{ test_rundir }}/test/extensions/common/wasm/test_data/test_rust.wasm")); @@ -141,10 +149,13 @@ TEST_F(WasmVmTest, V8Code) { EXPECT_THAT(wasm_vm->getCustomSection("producers"), HasSubstr("rustc")); EXPECT_TRUE(wasm_vm->getCustomSection("emscripten_metadata").empty()); + + EXPECT_TRUE(wasm_vm->cloneable() == Cloneable::CompiledBytecode); + EXPECT_TRUE(wasm_vm->clone() != nullptr); } TEST_F(WasmVmTest, V8BadHostFunctions) { - auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8"); + auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8", scope_); ASSERT_TRUE(wasm_vm != nullptr); auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( @@ -172,7 +183,7 @@ TEST_F(WasmVmTest, V8BadHostFunctions) { } TEST_F(WasmVmTest, V8BadModuleFunctions) { - auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8"); + auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8", scope_); ASSERT_TRUE(wasm_vm != nullptr); auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( @@ -200,7 +211,7 @@ TEST_F(WasmVmTest, V8BadModuleFunctions) { } TEST_F(WasmVmTest, V8FunctionCalls) { - auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8"); + auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8", scope_); ASSERT_TRUE(wasm_vm != nullptr); auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( @@ -238,7 +249,7 @@ TEST_F(WasmVmTest, V8FunctionCalls) { } TEST_F(WasmVmTest, V8Memory) { - auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8"); + auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8", scope_); ASSERT_TRUE(wasm_vm != nullptr); auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( From 0ba0bdfffb149bde4b2d64ebf69d04c42d6ebf69 Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Thu, 12 Dec 2019 21:50:21 +0000 Subject: [PATCH 02/13] Convert Singleton to a global. Signed-off-by: John Plevyak --- source/extensions/common/wasm/null/null_vm.cc | 2 +- source/extensions/common/wasm/null/null_vm.h | 6 ++--- source/extensions/common/wasm/v8/v8.cc | 5 ++--- source/extensions/common/wasm/wasm_vm_base.h | 22 ++++++++----------- 4 files changed, 15 insertions(+), 20 deletions(-) diff --git a/source/extensions/common/wasm/null/null_vm.cc b/source/extensions/common/wasm/null/null_vm.cc index c23d6d7bd0c48..d661c43134946 100644 --- a/source/extensions/common/wasm/null/null_vm.cc +++ b/source/extensions/common/wasm/null/null_vm.cc @@ -18,7 +18,7 @@ namespace Common { namespace Wasm { namespace Null { -ThreadSafeSingleton global_stats_; +std::atomic active_vms_; WasmVmPtr NullVm::clone() { auto cloned_null_vm = std::make_unique(*this); diff --git a/source/extensions/common/wasm/null/null_vm.h b/source/extensions/common/wasm/null/null_vm.h index 64f0e666978e1..4c0aae998d71b 100644 --- a/source/extensions/common/wasm/null/null_vm.h +++ b/source/extensions/common/wasm/null/null_vm.h @@ -19,16 +19,16 @@ namespace Common { namespace Wasm { namespace Null { -extern ThreadSafeSingleton global_stats_; +extern std::atomic active_vms_; // The NullVm wraps a C++ WASM plugin which has been compiled with the WASM API // and linked directly into the Envoy process. This is useful for development // in that it permits the debugger to set breakpoints in both Envoy and the plugin. struct NullVm : public WasmVmBase { NullVm(Stats::ScopeSharedPtr scope) - : WasmVmBase(scope, &global_stats_.get(), WasmRuntimeNames::get().Null) {} + : WasmVmBase(scope, &active_vms_, WasmRuntimeNames::get().Null) {} NullVm(const NullVm& other) - : WasmVmBase(other.scope_, &global_stats_.get(), WasmRuntimeNames::get().Null), + : WasmVmBase(other.scope_, &active_vms_, WasmRuntimeNames::get().Null), plugin_name_(other.plugin_name_) {} // WasmVm diff --git a/source/extensions/common/wasm/v8/v8.cc b/source/extensions/common/wasm/v8/v8.cc index e7fbea3620d90..7d54d30d8a174 100644 --- a/source/extensions/common/wasm/v8/v8.cc +++ b/source/extensions/common/wasm/v8/v8.cc @@ -20,7 +20,7 @@ namespace Common { namespace Wasm { namespace V8 { -ThreadSafeSingleton global_stats_; +std::atomic active_vms_; wasm::Engine* engine() { static const auto engine = wasm::Engine::make(); @@ -39,8 +39,7 @@ using FuncDataPtr = std::unique_ptr; class V8 : public WasmVmBase { public: - V8(Stats::ScopeSharedPtr scope) - : WasmVmBase(scope, &global_stats_.get(), WasmRuntimeNames::get().V8) {} + V8(Stats::ScopeSharedPtr scope) : WasmVmBase(scope, &active_vms_, WasmRuntimeNames::get().V8) {} // Extensions::Common::Wasm::WasmVm absl::string_view runtime() override { return WasmRuntimeNames::get().V8; } diff --git a/source/extensions/common/wasm/wasm_vm_base.h b/source/extensions/common/wasm/wasm_vm_base.h index 4ad0950ffa72c..7c95099265ecd 100644 --- a/source/extensions/common/wasm/wasm_vm_base.h +++ b/source/extensions/common/wasm/wasm_vm_base.h @@ -25,34 +25,30 @@ struct VmStats { ALL_VM_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT) }; -struct VmGlobalStats { - std::atomic active_; -}; - // Wasm VM base instance. Provides common behavior (e.g. Stats). class WasmVmBase : public WasmVm { public: - WasmVmBase(Stats::ScopeSharedPtr scope, VmGlobalStats* global_stats_ptr, + WasmVmBase(Stats::ScopeSharedPtr scope, std::atomic* active_vms_ptr, absl::string_view runtime) - : scope_(scope), global_stats_ptr_(global_stats_ptr), + : scope_(scope), active_vms_ptr_(active_vms_ptr), stats_(VmStats{ ALL_VM_STATS(POOL_COUNTER_PREFIX(*scope_, absl::StrCat("wasm_vm.", runtime, ".")), POOL_GAUGE_PREFIX(*scope_, absl::StrCat("wasm_vm.", runtime, ".")))}), runtime_(std::string(runtime)) { - global_stats_ptr_->active_++; stats_.created_.inc(); - stats_.active_.set(global_stats_ptr_->active_); - ENVOY_LOG(debug, "WasmVm created {} now active", runtime_, global_stats_ptr_->active_); + (*active_vms_ptr_)++; + stats_.active_.set(*active_vms_ptr_); + ENVOY_LOG(debug, "WasmVm created {} now active", runtime_, *active_vms_ptr_); } virtual ~WasmVmBase() { - global_stats_ptr_->active_--; - stats_.active_.set(global_stats_ptr_->active_); - ENVOY_LOG(debug, "~WasmVm {} {} remaining active", runtime_, global_stats_ptr_->active_); + (*active_vms_ptr_)--; + stats_.active_.set(*active_vms_ptr_); + ENVOY_LOG(debug, "~WasmVm {} {} remaining active", runtime_, *active_vms_ptr_); } protected: Stats::ScopeSharedPtr scope_; - VmGlobalStats* global_stats_ptr_; + std::atomic* active_vms_ptr_; VmStats stats_; std::string runtime_; }; From 80d36f2c2fddeaed498c05748bf2c271dbc8a33a Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Thu, 12 Dec 2019 22:07:17 +0000 Subject: [PATCH 03/13] Address coments. Signed-off-by: John Plevyak --- source/extensions/common/wasm/wasm_vm.h | 2 +- source/extensions/common/wasm/wasm_vm_base.h | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/source/extensions/common/wasm/wasm_vm.h b/source/extensions/common/wasm/wasm_vm.h index 45e5c08d0a81b..1b90fe988161b 100644 --- a/source/extensions/common/wasm/wasm_vm.h +++ b/source/extensions/common/wasm/wasm_vm.h @@ -145,7 +145,7 @@ class WasmVm : public Logger::Loggable { * compilation. Then, if cloning is supported, we clone that VM for each worker, potentially * copying and sharing the initialized data structures for efficiency. Otherwise we create an new * VM from scratch for each worker. - * @return true if the VM is cloneable. + * @return one of enum Cloneable with the VMs cloneablity. */ virtual Cloneable cloneable() PURE; diff --git a/source/extensions/common/wasm/wasm_vm_base.h b/source/extensions/common/wasm/wasm_vm_base.h index 7c95099265ecd..0a1ab94ee88a2 100644 --- a/source/extensions/common/wasm/wasm_vm_base.h +++ b/source/extensions/common/wasm/wasm_vm_base.h @@ -31,9 +31,9 @@ class WasmVmBase : public WasmVm { WasmVmBase(Stats::ScopeSharedPtr scope, std::atomic* active_vms_ptr, absl::string_view runtime) : scope_(scope), active_vms_ptr_(active_vms_ptr), - stats_(VmStats{ - ALL_VM_STATS(POOL_COUNTER_PREFIX(*scope_, absl::StrCat("wasm_vm.", runtime, ".")), - POOL_GAUGE_PREFIX(*scope_, absl::StrCat("wasm_vm.", runtime, ".")))}), + runtime_prefix_(absl::StrCat("wasm_vm.", runtime, ".")), + stats_(VmStats{ALL_VM_STATS(POOL_COUNTER_PREFIX(*scope_, runtime_prefix_), + POOL_GAUGE_PREFIX(*scope_, runtime_prefix_))}), runtime_(std::string(runtime)) { stats_.created_.inc(); (*active_vms_ptr_)++; @@ -42,6 +42,7 @@ class WasmVmBase : public WasmVm { } virtual ~WasmVmBase() { (*active_vms_ptr_)--; + // The stats referrd to by stats_ are resolved by name in the stats system. stats_.active_.set(*active_vms_ptr_); ENVOY_LOG(debug, "~WasmVm {} {} remaining active", runtime_, *active_vms_ptr_); } @@ -49,8 +50,9 @@ class WasmVmBase : public WasmVm { protected: Stats::ScopeSharedPtr scope_; std::atomic* active_vms_ptr_; + std::string runtime_prefix_; VmStats stats_; - std::string runtime_; + std::string runtime_; // The runtime e.g. "v8". }; } // namespace Wasm From 766c7d9c7787e4db4a7ab3710ee9c6ea727a6529 Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Fri, 13 Dec 2019 19:07:56 +0000 Subject: [PATCH 04/13] Address comments. Signed-off-by: John Plevyak --- source/extensions/common/wasm/null/null_vm.cc | 2 -- source/extensions/common/wasm/null/null_vm.h | 7 +++--- source/extensions/common/wasm/v8/v8.cc | 6 ++--- source/extensions/common/wasm/wasm_vm_base.h | 25 +++++++++++-------- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/source/extensions/common/wasm/null/null_vm.cc b/source/extensions/common/wasm/null/null_vm.cc index d661c43134946..3dd45ea96069f 100644 --- a/source/extensions/common/wasm/null/null_vm.cc +++ b/source/extensions/common/wasm/null/null_vm.cc @@ -18,8 +18,6 @@ namespace Common { namespace Wasm { namespace Null { -std::atomic active_vms_; - WasmVmPtr NullVm::clone() { auto cloned_null_vm = std::make_unique(*this); cloned_null_vm->load(plugin_name_, false /* unused */); diff --git a/source/extensions/common/wasm/null/null_vm.h b/source/extensions/common/wasm/null/null_vm.h index 4c0aae998d71b..98fa6b6321165 100644 --- a/source/extensions/common/wasm/null/null_vm.h +++ b/source/extensions/common/wasm/null/null_vm.h @@ -19,16 +19,14 @@ namespace Common { namespace Wasm { namespace Null { -extern std::atomic active_vms_; - // The NullVm wraps a C++ WASM plugin which has been compiled with the WASM API // and linked directly into the Envoy process. This is useful for development // in that it permits the debugger to set breakpoints in both Envoy and the plugin. struct NullVm : public WasmVmBase { NullVm(Stats::ScopeSharedPtr scope) - : WasmVmBase(scope, &active_vms_, WasmRuntimeNames::get().Null) {} + : WasmVmBase(scope, globalStats(), WasmRuntimeNames::get().Null) {} NullVm(const NullVm& other) - : WasmVmBase(other.scope_, &active_vms_, WasmRuntimeNames::get().Null), + : WasmVmBase(other.scope_, globalStats(), WasmRuntimeNames::get().Null), plugin_name_(other.plugin_name_) {} // WasmVm @@ -43,6 +41,7 @@ struct NullVm : public WasmVmBase { bool setWord(uint64_t pointer, Word data) override; bool getWord(uint64_t pointer, Word* data) override; absl::string_view getCustomSection(absl::string_view name) override; + VmGlobalStats& globalStats() { MUTABLE_CONSTRUCT_ON_FIRST_USE(VmGlobalStats); } #define _FORWARD_GET_FUNCTION(_T) \ void getFunction(absl::string_view function_name, _T* f) override { \ diff --git a/source/extensions/common/wasm/v8/v8.cc b/source/extensions/common/wasm/v8/v8.cc index 7d54d30d8a174..5213bfc71be5b 100644 --- a/source/extensions/common/wasm/v8/v8.cc +++ b/source/extensions/common/wasm/v8/v8.cc @@ -20,8 +20,6 @@ namespace Common { namespace Wasm { namespace V8 { -std::atomic active_vms_; - wasm::Engine* engine() { static const auto engine = wasm::Engine::make(); return engine.get(); @@ -39,7 +37,7 @@ using FuncDataPtr = std::unique_ptr; class V8 : public WasmVmBase { public: - V8(Stats::ScopeSharedPtr scope) : WasmVmBase(scope, &active_vms_, WasmRuntimeNames::get().V8) {} + V8(Stats::ScopeSharedPtr scope) : WasmVmBase(scope, globalStats(), WasmRuntimeNames::get().V8) {} // Extensions::Common::Wasm::WasmVm absl::string_view runtime() override { return WasmRuntimeNames::get().V8; } @@ -89,6 +87,8 @@ class V8 : public WasmVmBase { void getModuleFunctionImpl(absl::string_view function_name, std::function* function); + VmGlobalStats& globalStats() { MUTABLE_CONSTRUCT_ON_FIRST_USE(VmGlobalStats); } + wasm::vec source_ = wasm::vec::invalid(); wasm::own store_; wasm::own module_; diff --git a/source/extensions/common/wasm/wasm_vm_base.h b/source/extensions/common/wasm/wasm_vm_base.h index 0a1ab94ee88a2..c2e5506decc68 100644 --- a/source/extensions/common/wasm/wasm_vm_base.h +++ b/source/extensions/common/wasm/wasm_vm_base.h @@ -25,32 +25,35 @@ struct VmStats { ALL_VM_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT) }; +struct VmGlobalStats { + std::atomic active_vms_{0}; +}; + // Wasm VM base instance. Provides common behavior (e.g. Stats). class WasmVmBase : public WasmVm { public: - WasmVmBase(Stats::ScopeSharedPtr scope, std::atomic* active_vms_ptr, - absl::string_view runtime) - : scope_(scope), active_vms_ptr_(active_vms_ptr), - runtime_prefix_(absl::StrCat("wasm_vm.", runtime, ".")), + WasmVmBase(Stats::ScopeSharedPtr scope, VmGlobalStats& global_stats, absl::string_view runtime) + : scope_(scope), runtime_prefix_(absl::StrCat("wasm_vm.", runtime, ".")), + global_stats_(global_stats), stats_(VmStats{ALL_VM_STATS(POOL_COUNTER_PREFIX(*scope_, runtime_prefix_), POOL_GAUGE_PREFIX(*scope_, runtime_prefix_))}), runtime_(std::string(runtime)) { stats_.created_.inc(); - (*active_vms_ptr_)++; - stats_.active_.set(*active_vms_ptr_); - ENVOY_LOG(debug, "WasmVm created {} now active", runtime_, *active_vms_ptr_); + auto active = ++global_stats_.active_vms_; + stats_.active_.set(active); + ENVOY_LOG(debug, "WasmVm created {} now active", runtime_, active); } virtual ~WasmVmBase() { - (*active_vms_ptr_)--; + auto active = --global_stats_.active_vms_; // The stats referrd to by stats_ are resolved by name in the stats system. - stats_.active_.set(*active_vms_ptr_); - ENVOY_LOG(debug, "~WasmVm {} {} remaining active", runtime_, *active_vms_ptr_); + stats_.active_.set(active); + ENVOY_LOG(debug, "~WasmVm {} {} remaining active", runtime_, active); } protected: Stats::ScopeSharedPtr scope_; - std::atomic* active_vms_ptr_; std::string runtime_prefix_; + VmGlobalStats& global_stats_; VmStats stats_; std::string runtime_; // The runtime e.g. "v8". }; From bfefaf416b9ad652758ce909fc90cbcd63c593d6 Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Wed, 18 Dec 2019 18:48:43 +0000 Subject: [PATCH 05/13] Address comments. Signed-off-by: John Plevyak --- source/extensions/common/wasm/null/null_vm.h | 7 ++----- source/extensions/common/wasm/v8/v8.cc | 4 +--- source/extensions/common/wasm/wasm_vm_base.h | 19 +++++-------------- 3 files changed, 8 insertions(+), 22 deletions(-) diff --git a/source/extensions/common/wasm/null/null_vm.h b/source/extensions/common/wasm/null/null_vm.h index 98fa6b6321165..6adafd22c0a65 100644 --- a/source/extensions/common/wasm/null/null_vm.h +++ b/source/extensions/common/wasm/null/null_vm.h @@ -23,11 +23,9 @@ namespace Null { // and linked directly into the Envoy process. This is useful for development // in that it permits the debugger to set breakpoints in both Envoy and the plugin. struct NullVm : public WasmVmBase { - NullVm(Stats::ScopeSharedPtr scope) - : WasmVmBase(scope, globalStats(), WasmRuntimeNames::get().Null) {} + NullVm(Stats::ScopeSharedPtr scope) : WasmVmBase(scope, WasmRuntimeNames::get().Null) {} NullVm(const NullVm& other) - : WasmVmBase(other.scope_, globalStats(), WasmRuntimeNames::get().Null), - plugin_name_(other.plugin_name_) {} + : WasmVmBase(other.scope_, WasmRuntimeNames::get().Null), plugin_name_(other.plugin_name_) {} // WasmVm absl::string_view runtime() override { return WasmRuntimeNames::get().Null; } @@ -41,7 +39,6 @@ struct NullVm : public WasmVmBase { bool setWord(uint64_t pointer, Word data) override; bool getWord(uint64_t pointer, Word* data) override; absl::string_view getCustomSection(absl::string_view name) override; - VmGlobalStats& globalStats() { MUTABLE_CONSTRUCT_ON_FIRST_USE(VmGlobalStats); } #define _FORWARD_GET_FUNCTION(_T) \ void getFunction(absl::string_view function_name, _T* f) override { \ diff --git a/source/extensions/common/wasm/v8/v8.cc b/source/extensions/common/wasm/v8/v8.cc index 5213bfc71be5b..d54dab9212556 100644 --- a/source/extensions/common/wasm/v8/v8.cc +++ b/source/extensions/common/wasm/v8/v8.cc @@ -37,7 +37,7 @@ using FuncDataPtr = std::unique_ptr; class V8 : public WasmVmBase { public: - V8(Stats::ScopeSharedPtr scope) : WasmVmBase(scope, globalStats(), WasmRuntimeNames::get().V8) {} + V8(Stats::ScopeSharedPtr scope) : WasmVmBase(scope, WasmRuntimeNames::get().V8) {} // Extensions::Common::Wasm::WasmVm absl::string_view runtime() override { return WasmRuntimeNames::get().V8; } @@ -87,8 +87,6 @@ class V8 : public WasmVmBase { void getModuleFunctionImpl(absl::string_view function_name, std::function* function); - VmGlobalStats& globalStats() { MUTABLE_CONSTRUCT_ON_FIRST_USE(VmGlobalStats); } - wasm::vec source_ = wasm::vec::invalid(); wasm::own store_; wasm::own module_; diff --git a/source/extensions/common/wasm/wasm_vm_base.h b/source/extensions/common/wasm/wasm_vm_base.h index c2e5506decc68..1ab0506008835 100644 --- a/source/extensions/common/wasm/wasm_vm_base.h +++ b/source/extensions/common/wasm/wasm_vm_base.h @@ -25,35 +25,26 @@ struct VmStats { ALL_VM_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT) }; -struct VmGlobalStats { - std::atomic active_vms_{0}; -}; - // Wasm VM base instance. Provides common behavior (e.g. Stats). class WasmVmBase : public WasmVm { public: - WasmVmBase(Stats::ScopeSharedPtr scope, VmGlobalStats& global_stats, absl::string_view runtime) + WasmVmBase(Stats::ScopeSharedPtr scope, absl::string_view runtime) : scope_(scope), runtime_prefix_(absl::StrCat("wasm_vm.", runtime, ".")), - global_stats_(global_stats), stats_(VmStats{ALL_VM_STATS(POOL_COUNTER_PREFIX(*scope_, runtime_prefix_), POOL_GAUGE_PREFIX(*scope_, runtime_prefix_))}), runtime_(std::string(runtime)) { stats_.created_.inc(); - auto active = ++global_stats_.active_vms_; - stats_.active_.set(active); - ENVOY_LOG(debug, "WasmVm created {} now active", runtime_, active); + stats_.active_.inc(); + ENVOY_LOG(debug, "WasmVm created {} now active", runtime_, stats_.active_.value()); } virtual ~WasmVmBase() { - auto active = --global_stats_.active_vms_; - // The stats referrd to by stats_ are resolved by name in the stats system. - stats_.active_.set(active); - ENVOY_LOG(debug, "~WasmVm {} {} remaining active", runtime_, active); + stats_.active_.dec(); + ENVOY_LOG(debug, "~WasmVm {} {} remaining active", runtime_, stats_.active_.value()); } protected: Stats::ScopeSharedPtr scope_; std::string runtime_prefix_; - VmGlobalStats& global_stats_; VmStats stats_; std::string runtime_; // The runtime e.g. "v8". }; From a3d8030838e4e7e28e008a5e44d89496d25ac7b7 Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Thu, 19 Dec 2019 20:40:47 +0000 Subject: [PATCH 06/13] Fix pedantic spelling. Signed-off-by: John Plevyak --- source/extensions/common/wasm/wasm_vm.h | 2 +- tools/spelling_dictionary.txt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/source/extensions/common/wasm/wasm_vm.h b/source/extensions/common/wasm/wasm_vm.h index 1b90fe988161b..d9dc432c3a54f 100644 --- a/source/extensions/common/wasm/wasm_vm.h +++ b/source/extensions/common/wasm/wasm_vm.h @@ -145,7 +145,7 @@ class WasmVm : public Logger::Loggable { * compilation. Then, if cloning is supported, we clone that VM for each worker, potentially * copying and sharing the initialized data structures for efficiency. Otherwise we create an new * VM from scratch for each worker. - * @return one of enum Cloneable with the VMs cloneablity. + * @return one of enum Cloneable with the VMs cloneability. */ virtual Cloneable cloneable() PURE; diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index de07712e8ae64..3f1fe7766e59e 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -408,6 +408,7 @@ ciphersuites circllhist CITT cloneable +cloneability cmd cmsghdr codebase From 48bfc76779f4f9497874001d2b8ad2e624c27f99 Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Mon, 23 Dec 2019 18:48:49 +0000 Subject: [PATCH 07/13] Address comments. Signed-off-by: John Plevyak --- source/extensions/common/wasm/null/null.cc | 2 +- source/extensions/common/wasm/null/null.h | 2 +- source/extensions/common/wasm/null/null_vm.h | 2 +- source/extensions/common/wasm/v8/v8.cc | 4 ++-- source/extensions/common/wasm/v8/v8.h | 2 +- source/extensions/common/wasm/wasm_vm.cc | 2 +- source/extensions/common/wasm/wasm_vm.h | 2 +- source/extensions/common/wasm/wasm_vm_base.h | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/source/extensions/common/wasm/null/null.cc b/source/extensions/common/wasm/null/null.cc index 1a2fc46924331..af2ba77d1dc56 100644 --- a/source/extensions/common/wasm/null/null.cc +++ b/source/extensions/common/wasm/null/null.cc @@ -18,7 +18,7 @@ namespace Common { namespace Wasm { namespace Null { -WasmVmPtr createVm(Stats::ScopeSharedPtr scope) { return std::make_unique(scope); } +WasmVmPtr createVm(const Stats::ScopeSharedPtr& scope) { return std::make_unique(scope); } } // namespace Null } // namespace Wasm diff --git a/source/extensions/common/wasm/null/null.h b/source/extensions/common/wasm/null/null.h index 1fab2784f04bc..285b13373fbc8 100644 --- a/source/extensions/common/wasm/null/null.h +++ b/source/extensions/common/wasm/null/null.h @@ -11,7 +11,7 @@ namespace Common { namespace Wasm { namespace Null { -WasmVmPtr createVm(Stats::ScopeSharedPtr scope); +WasmVmPtr createVm(const Stats::ScopeSharedPtr& scope); } // namespace Null } // namespace Wasm diff --git a/source/extensions/common/wasm/null/null_vm.h b/source/extensions/common/wasm/null/null_vm.h index 6adafd22c0a65..ca962b2eefd97 100644 --- a/source/extensions/common/wasm/null/null_vm.h +++ b/source/extensions/common/wasm/null/null_vm.h @@ -23,7 +23,7 @@ namespace Null { // and linked directly into the Envoy process. This is useful for development // in that it permits the debugger to set breakpoints in both Envoy and the plugin. struct NullVm : public WasmVmBase { - NullVm(Stats::ScopeSharedPtr scope) : WasmVmBase(scope, WasmRuntimeNames::get().Null) {} + NullVm(const Stats::ScopeSharedPtr& scope) : WasmVmBase(scope, WasmRuntimeNames::get().Null) {} NullVm(const NullVm& other) : WasmVmBase(other.scope_, WasmRuntimeNames::get().Null), plugin_name_(other.plugin_name_) {} diff --git a/source/extensions/common/wasm/v8/v8.cc b/source/extensions/common/wasm/v8/v8.cc index d54dab9212556..d83f6e8acde85 100644 --- a/source/extensions/common/wasm/v8/v8.cc +++ b/source/extensions/common/wasm/v8/v8.cc @@ -37,7 +37,7 @@ using FuncDataPtr = std::unique_ptr; class V8 : public WasmVmBase { public: - V8(Stats::ScopeSharedPtr scope) : WasmVmBase(scope, WasmRuntimeNames::get().V8) {} + V8(const Stats::ScopeSharedPtr& scope) : WasmVmBase(scope, WasmRuntimeNames::get().V8) {} // Extensions::Common::Wasm::WasmVm absl::string_view runtime() override { return WasmRuntimeNames::get().V8; } @@ -583,7 +583,7 @@ void V8::getModuleFunctionImpl(absl::string_view function_name, }; } -WasmVmPtr createVm(Stats::ScopeSharedPtr scope) { return std::make_unique(scope); } +WasmVmPtr createVm(const Stats::ScopeSharedPtr& scope) { return std::make_unique(scope); } } // namespace V8 } // namespace Wasm diff --git a/source/extensions/common/wasm/v8/v8.h b/source/extensions/common/wasm/v8/v8.h index 1cee4424e8152..a7288f0004a59 100644 --- a/source/extensions/common/wasm/v8/v8.h +++ b/source/extensions/common/wasm/v8/v8.h @@ -10,7 +10,7 @@ namespace Common { namespace Wasm { namespace V8 { -WasmVmPtr createVm(Stats::ScopeSharedPtr scope); +WasmVmPtr createVm(const Stats::ScopeSharedPtr& scope); } // namespace V8 } // namespace Wasm diff --git a/source/extensions/common/wasm/wasm_vm.cc b/source/extensions/common/wasm/wasm_vm.cc index 88ee11c60bc0d..9299eceba2d10 100644 --- a/source/extensions/common/wasm/wasm_vm.cc +++ b/source/extensions/common/wasm/wasm_vm.cc @@ -14,7 +14,7 @@ namespace Wasm { thread_local Envoy::Extensions::Common::Wasm::Context* current_context_ = nullptr; thread_local uint32_t effective_context_id_ = 0; -WasmVmPtr createWasmVm(absl::string_view runtime, Stats::ScopeSharedPtr scope) { +WasmVmPtr createWasmVm(absl::string_view runtime, const Stats::ScopeSharedPtr& scope) { if (runtime.empty()) { throw WasmVmException("Failed to create WASM VM with unspecified runtime."); } else if (runtime == WasmRuntimeNames::get().Null) { diff --git a/source/extensions/common/wasm/wasm_vm.h b/source/extensions/common/wasm/wasm_vm.h index d9dc432c3a54f..a383b95ce7c4d 100644 --- a/source/extensions/common/wasm/wasm_vm.h +++ b/source/extensions/common/wasm/wasm_vm.h @@ -291,7 +291,7 @@ struct SaveRestoreContext { }; // Create a new low-level WASM VM using runtime of the given type (e.g. "envoy.wasm.runtime.wavm"). -WasmVmPtr createWasmVm(absl::string_view runtime, Stats::ScopeSharedPtr scope); +WasmVmPtr createWasmVm(absl::string_view runtime, const Stats::ScopeSharedPtr& scope); } // namespace Wasm } // namespace Common diff --git a/source/extensions/common/wasm/wasm_vm_base.h b/source/extensions/common/wasm/wasm_vm_base.h index 1ab0506008835..4eae2fc0e9240 100644 --- a/source/extensions/common/wasm/wasm_vm_base.h +++ b/source/extensions/common/wasm/wasm_vm_base.h @@ -28,7 +28,7 @@ struct VmStats { // Wasm VM base instance. Provides common behavior (e.g. Stats). class WasmVmBase : public WasmVm { public: - WasmVmBase(Stats::ScopeSharedPtr scope, absl::string_view runtime) + WasmVmBase(const Stats::ScopeSharedPtr& scope, absl::string_view runtime) : scope_(scope), runtime_prefix_(absl::StrCat("wasm_vm.", runtime, ".")), stats_(VmStats{ALL_VM_STATS(POOL_COUNTER_PREFIX(*scope_, runtime_prefix_), POOL_GAUGE_PREFIX(*scope_, runtime_prefix_))}), From 66cdce03e94d93a695438414bf3e82f64105bbab Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Sat, 28 Dec 2019 17:55:48 +0000 Subject: [PATCH 08/13] Address comments. Signed-off-by: John Plevyak --- source/extensions/common/wasm/wasm_vm_base.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/source/extensions/common/wasm/wasm_vm_base.h b/source/extensions/common/wasm/wasm_vm_base.h index 4eae2fc0e9240..a709534cba521 100644 --- a/source/extensions/common/wasm/wasm_vm_base.h +++ b/source/extensions/common/wasm/wasm_vm_base.h @@ -30,9 +30,9 @@ class WasmVmBase : public WasmVm { public: WasmVmBase(const Stats::ScopeSharedPtr& scope, absl::string_view runtime) : scope_(scope), runtime_prefix_(absl::StrCat("wasm_vm.", runtime, ".")), + runtime_(std::string(runtime)), stats_(VmStats{ALL_VM_STATS(POOL_COUNTER_PREFIX(*scope_, runtime_prefix_), - POOL_GAUGE_PREFIX(*scope_, runtime_prefix_))}), - runtime_(std::string(runtime)) { + POOL_GAUGE_PREFIX(*scope_, runtime_prefix_))}) { stats_.created_.inc(); stats_.active_.inc(); ENVOY_LOG(debug, "WasmVm created {} now active", runtime_, stats_.active_.value()); @@ -43,10 +43,10 @@ class WasmVmBase : public WasmVm { } protected: - Stats::ScopeSharedPtr scope_; - std::string runtime_prefix_; + const Stats::ScopeSharedPtr scope_; + const std::string runtime_prefix_; + const std::string runtime_; // The runtime e.g. "v8". VmStats stats_; - std::string runtime_; // The runtime e.g. "v8". }; } // namespace Wasm From 4069f426826934cb6b5f05657a9425f37052dd28 Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Sat, 28 Dec 2019 18:01:05 +0000 Subject: [PATCH 09/13] Address comments. Signed-off-by: John Plevyak --- source/extensions/common/wasm/wasm_vm.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/source/extensions/common/wasm/wasm_vm.h b/source/extensions/common/wasm/wasm_vm.h index a383b95ce7c4d..94ddae0174b9a 100644 --- a/source/extensions/common/wasm/wasm_vm.h +++ b/source/extensions/common/wasm/wasm_vm.h @@ -124,7 +124,11 @@ using WasmCallback_dd = double (*)(void*, double); _f(WasmCallback_WWl) _f(WasmCallback_WWlWW) _f(WasmCallback_WWm) \ _f(WasmCallback_dd) -enum class Cloneable { NotCloneable, CompiledBytecode, InstantiatedModule }; +enum class Cloneable { + NotCloneable, // VMs can not be cloned and should be created from scratch. + CompiledBytecode, // VMs can be clone with compiled bytecode. + InstantiatedModule // VMs can be cloned from an instatiated module. +}; // Wasm VM instance. Provides the low level WASM interface. class WasmVm : public Logger::Loggable { From 3374ab364f8a11f2c9ce75a19680a880426d746d Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Sat, 28 Dec 2019 18:25:50 +0000 Subject: [PATCH 10/13] Address comments. Signed-off-by: John Plevyak --- source/extensions/common/wasm/wasm_vm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/common/wasm/wasm_vm.h b/source/extensions/common/wasm/wasm_vm.h index 94ddae0174b9a..1ef3dd2887795 100644 --- a/source/extensions/common/wasm/wasm_vm.h +++ b/source/extensions/common/wasm/wasm_vm.h @@ -126,7 +126,7 @@ using WasmCallback_dd = double (*)(void*, double); enum class Cloneable { NotCloneable, // VMs can not be cloned and should be created from scratch. - CompiledBytecode, // VMs can be clone with compiled bytecode. + CompiledBytecode, // VMs can be cloned with compiled bytecode. InstantiatedModule // VMs can be cloned from an instatiated module. }; From 963f3b1066c98c1cdf4d8d99b3a41cce67047474 Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Sat, 28 Dec 2019 19:01:45 +0000 Subject: [PATCH 11/13] Address comments. Signed-off-by: John Plevyak --- tools/spelling_dictionary.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index 3f1fe7766e59e..f2ac8989e583b 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -383,6 +383,7 @@ builtin builtins bulkstrings bursty +bytecode callee callsite callsites From bb8dfc05f188e28faabcb187ee3e48c7fb1bc366 Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Sat, 28 Dec 2019 21:29:47 +0000 Subject: [PATCH 12/13] Address comments. Signed-off-by: John Plevyak --- source/extensions/common/wasm/wasm_vm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/common/wasm/wasm_vm.h b/source/extensions/common/wasm/wasm_vm.h index 1ef3dd2887795..95b43aee47512 100644 --- a/source/extensions/common/wasm/wasm_vm.h +++ b/source/extensions/common/wasm/wasm_vm.h @@ -127,7 +127,7 @@ using WasmCallback_dd = double (*)(void*, double); enum class Cloneable { NotCloneable, // VMs can not be cloned and should be created from scratch. CompiledBytecode, // VMs can be cloned with compiled bytecode. - InstantiatedModule // VMs can be cloned from an instatiated module. + InstantiatedModule // VMs can be cloned from an instantiated module. }; // Wasm VM instance. Provides the low level WASM interface. From 4ec423289b88480e3571d0163afb461e9a56d336 Mon Sep 17 00:00:00 2001 From: John Plevyak Date: Thu, 9 Jan 2020 23:21:37 +0000 Subject: [PATCH 13/13] Address comments. Signed-off-by: John Plevyak --- source/extensions/common/wasm/null/null_vm.cc | 1 - source/extensions/common/wasm/null/null_vm.h | 1 - source/extensions/common/wasm/v8/v8.cc | 6 ------ 3 files changed, 8 deletions(-) diff --git a/source/extensions/common/wasm/null/null_vm.cc b/source/extensions/common/wasm/null/null_vm.cc index 3dd45ea96069f..d2cb16b994f16 100644 --- a/source/extensions/common/wasm/null/null_vm.cc +++ b/source/extensions/common/wasm/null/null_vm.cc @@ -7,7 +7,6 @@ #include "envoy/registry/registry.h" #include "common/common/assert.h" -#include "common/singleton/threadsafe_singleton.h" #include "extensions/common/wasm/null/null_vm_plugin.h" #include "extensions/common/wasm/well_known_names.h" diff --git a/source/extensions/common/wasm/null/null_vm.h b/source/extensions/common/wasm/null/null_vm.h index ca962b2eefd97..82da033019723 100644 --- a/source/extensions/common/wasm/null/null_vm.h +++ b/source/extensions/common/wasm/null/null_vm.h @@ -7,7 +7,6 @@ #include "envoy/registry/registry.h" #include "common/common/assert.h" -#include "common/singleton/threadsafe_singleton.h" #include "extensions/common/wasm/null/null_vm_plugin.h" #include "extensions/common/wasm/wasm_vm_base.h" diff --git a/source/extensions/common/wasm/v8/v8.cc b/source/extensions/common/wasm/v8/v8.cc index d83f6e8acde85..bd158874ba731 100644 --- a/source/extensions/common/wasm/v8/v8.cc +++ b/source/extensions/common/wasm/v8/v8.cc @@ -5,7 +5,6 @@ #include #include "common/common/assert.h" -#include "common/singleton/threadsafe_singleton.h" #include "extensions/common/wasm/wasm_vm_base.h" #include "extensions/common/wasm/well_known_names.h" @@ -243,7 +242,6 @@ template constexpr T convertValTypesToArgsTuple(const U bool V8::load(const std::string& code, bool /* allow_precompiled */) { ENVOY_LOG(trace, "load()"); store_ = wasm::Store::make(engine()); - RELEASE_ASSERT(store_ != nullptr, ""); source_ = wasm::vec::make_uninitialized(code.size()); ::memcpy(source_.get(), code.data(), code.size()); @@ -251,7 +249,6 @@ bool V8::load(const std::string& code, bool /* allow_precompiled */) { module_ = wasm::Module::make(store_.get(), source_); if (module_) { shared_module_ = module_->share(); - RELEASE_ASSERT(shared_module_ != nullptr, ""); } return module_ != nullptr; @@ -263,10 +260,8 @@ WasmVmPtr V8::clone() { auto clone = std::make_unique(scope_); clone->store_ = wasm::Store::make(engine()); - RELEASE_ASSERT(clone->store_ != nullptr, ""); clone->module_ = wasm::Module::obtain(clone->store_.get(), shared_module_.get()); - RELEASE_ASSERT(clone->module_ != nullptr, ""); return clone; } @@ -377,7 +372,6 @@ void V8::link(absl::string_view debug_name) { ASSERT(import_types.size() == imports.size()); instance_ = wasm::Instance::make(store_.get(), module_.get(), imports.data()); - RELEASE_ASSERT(instance_ != nullptr, ""); const auto export_types = module_.get()->exports(); const auto exports = instance_.get()->exports();