diff --git a/bin/pre-commit/clang_tidy_check.py b/bin/pre-commit/clang_tidy_check.py index 7fb51d1c46..f3c75033bb 100755 --- a/bin/pre-commit/clang_tidy_check.py +++ b/bin/pre-commit/clang_tidy_check.py @@ -168,7 +168,19 @@ def main(): if not os.environ.get("TIDY"): return 0 - repo_root = Path(__file__).parent.parent + # Derive the repo root from git so this keeps working regardless of where + # under the tree this script lives. Fall back to the script location + # (bin/pre-commit/ is two levels below the repo root) if git is unavailable. + result = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + cwd=Path(__file__).parent, + ) + if result.returncode == 0: + repo_root = Path(result.stdout.strip()) + else: + repo_root = Path(__file__).parent.parent.parent files = staged_files(repo_root) if not files: return 0 diff --git a/include/xrpl/core/CollectorManager.h b/include/xrpl/core/CollectorManager.h new file mode 100644 index 0000000000..e736ae57db --- /dev/null +++ b/include/xrpl/core/CollectorManager.h @@ -0,0 +1,24 @@ +#pragma once + +#include +#include + +namespace xrpl { + +/** Provides the beast::insight::Collector service. */ +class CollectorManager +{ +public: + virtual ~CollectorManager() = default; + + virtual beast::insight::Collector::ptr const& + collector() = 0; + + virtual beast::insight::Group::ptr const& + group(std::string const& name) = 0; +}; + +std::unique_ptr +makeCollectorManager(Section const& params, beast::Journal journal); + +} // namespace xrpl diff --git a/include/xrpl/tx/wasm/HostFunc.h b/include/xrpl/tx/wasm/HostFunc.h index 7c5eb18b0c..4f989ba66e 100644 --- a/include/xrpl/tx/wasm/HostFunc.h +++ b/include/xrpl/tx/wasm/HostFunc.h @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -121,6 +122,12 @@ struct HostFunctions return j; } + [[nodiscard]] virtual beast::insight::Event + executionTimeEvent(std::string_view name) const + { + return {}; + } + [[nodiscard]] virtual bool checkSelf() const { diff --git a/include/xrpl/tx/wasm/HostFuncImpl.h b/include/xrpl/tx/wasm/HostFuncImpl.h index 689ce85a59..a3f3511579 100644 --- a/include/xrpl/tx/wasm/HostFuncImpl.h +++ b/include/xrpl/tx/wasm/HostFuncImpl.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -77,6 +78,15 @@ public: return rt_; } + beast::insight::Event + executionTimeEvent(std::string_view name) const override + { + return ctx_.registry.get() + .getCollectorManager() + .group(std::string{name.data(), name.size()}) + ->makeEvent("finish_time"); + } + bool checkSelf() const override { diff --git a/src/libxrpl/tx/wasm/WasmVM.cpp b/src/libxrpl/tx/wasm/WasmVM.cpp index 6aa26a1f9f..2ab277e49b 100644 --- a/src/libxrpl/tx/wasm/WasmVM.cpp +++ b/src/libxrpl/tx/wasm/WasmVM.cpp @@ -6,6 +6,7 @@ #include // IWYU pragma: keep #include +#include #include #include #include @@ -126,9 +127,16 @@ runEscrowWasm( auto& vm = WasmEngine::instance(); // vm.initMaxPages(MAX_PAGES); + auto const start = std::chrono::steady_clock::now(); + auto const ret = vm.run(wasmCode, hfs, gasLimit, funcName, params, createWasmImport(hfs), hfs.getJournal()); + hfs.executionTimeEvent("runEscrowWasm") + .notify( + std::chrono::duration_cast( + std::chrono::steady_clock::now() - start)); + if (!ret) { #ifdef DEBUG_OUTPUT @@ -140,7 +148,7 @@ runEscrowWasm( #ifdef DEBUG_OUTPUT std::cout << ", ret: " << ret->result << ", gas spent: " << ret->cost << std::endl; #endif - return EscrowResult{ret->result, ret->cost}; + return EscrowResult{.result = ret->result, .cost = ret->cost}; } NotTEC @@ -154,9 +162,16 @@ preflightEscrowWasm( auto& vm = WasmEngine::instance(); // vm.initMaxPages(MAX_PAGES); + auto const start = std::chrono::steady_clock::now(); + auto const ret = vm.check(wasmCode, hfs, funcName, params, createWasmImport(hfs), hfs.getJournal()); + hfs.executionTimeEvent("preflightEscrowWasm") + .notify( + std::chrono::duration_cast( + std::chrono::steady_clock::now() - start)); + return ret; } diff --git a/src/libxrpl/tx/wasm/WasmiVM.cpp b/src/libxrpl/tx/wasm/WasmiVM.cpp index e22380741b..e1747e3578 100644 --- a/src/libxrpl/tx/wasm/WasmiVM.cpp +++ b/src/libxrpl/tx/wasm/WasmiVM.cpp @@ -277,7 +277,7 @@ InstanceWrapper::setGas(std::int64_t gas) const ModulePtr ModuleWrapper::init(StorePtr& s, Bytes const& wasmBin, beast::Journal j) { - wasm_byte_vec_t const code{wasmBin.size(), (char*)(wasmBin.data())}; + wasm_byte_vec_t const code{.size = wasmBin.size(), .data = (char*)(wasmBin.data())}; ModulePtr m = ModulePtr(wasm_module_new(s.get(), &code), &wasm_module_delete); if (!m) throw std::runtime_error("can't create module"); @@ -711,8 +711,8 @@ WasmiResult WasmiEngine::call(FuncInfo const& f, std::vector& in) { WasmiResult ret(NR); - wasm_val_vec_t const inv = - in.empty() ? wasm_val_vec_t WASM_EMPTY_VEC : wasm_val_vec_t{in.size(), in.data()}; + wasm_val_vec_t const inv = in.empty() ? wasm_val_vec_t WASM_EMPTY_VEC + : wasm_val_vec_t{.size = in.size(), .data = in.data()}; #ifdef SHOW_CALL_TIME auto const start = usecs(); @@ -960,7 +960,7 @@ wasm_trap_t* WasmiEngine::newTrap(std::string const& txt) { static char empty[1] = {0}; - wasm_message_t msg = {1, empty}; + wasm_message_t msg = {.size = 1, .data = empty}; if (!txt.empty()) wasm_name_new(&msg, txt.size() + 1, txt.c_str()); // include 0 diff --git a/src/test/app/TestHostFunctions.h b/src/test/app/TestHostFunctions.h index 26d30dd53c..80e0abae14 100644 --- a/src/test/app/TestHostFunctions.h +++ b/src/test/app/TestHostFunctions.h @@ -2,6 +2,8 @@ #include #include +#include +#include #include #include #include @@ -18,6 +20,23 @@ namespace xrpl::test { +/** + * Lets a test assert that the WASM execution-timing path fired (and inspect + * the recorded duration) without needing a StatsD sink or a full Collector. + */ +struct RecordingEventImpl : public beast::insight::EventImpl +{ + std::size_t count = 0; + value_type last{}; + + void + notify(value_type const& value) override + { + ++count; + last = value; + } +}; + struct TestLedgerDataProvider : public HostFunctions { jtx::Env& env; @@ -54,6 +73,7 @@ struct TestHostFunctions : public HostFunctions Bytes data; int clock_drift = 0; void* rt = nullptr; + std::shared_ptr execTimeEvent = std::make_shared(); public: TestHostFunctions(test::jtx::Env& env, int cd = 0) @@ -76,6 +96,15 @@ public: return rt; } + // Return an Event backed by our recording impl so a test can assert that + // the WASM execution was timed. The name is ignored -- every call records + // into the same impl. + [[nodiscard]] beast::insight::Event + executionTimeEvent(std::string_view name) const override + { + return beast::insight::Event(execTimeEvent); + } + Expected getLedgerSqn() const override { diff --git a/src/test/app/Wasm_test.cpp b/src/test/app/Wasm_test.cpp index fd164feefe..e39bb98769 100644 --- a/src/test/app/Wasm_test.cpp +++ b/src/test/app/Wasm_test.cpp @@ -16,7 +16,6 @@ #include #include -#include #include #include #include @@ -1547,27 +1546,14 @@ struct Wasm_test : public beast::unit_test::Suite template void - perf(size_t runs, Functor&& f) + perf(TestHostFunctions& hfs, size_t runs, Functor&& f) { - using std::chrono::duration_cast; - using std::chrono::steady_clock; - - // Warm up first. - for (auto i = size_t{}; i < 10; ++i) - { - BEAST_EXPECT(f()); - } - - auto totalTime = uint64_t{}; for (auto i = size_t{}; i < runs; ++i) { - auto const start = steady_clock::now(); auto result = f(); - auto const end = steady_clock::now(); - totalTime += duration_cast(end - start).count(); BEAST_EXPECT(result); } - log << "Average time for " << runs << " runs: " << (totalTime / runs) << " ns\n"; + BEAST_EXPECT(hfs.execTimeEvent->count == runs); } void @@ -1583,7 +1569,7 @@ struct Wasm_test : public beast::unit_test::Suite Env env{*this}; auto hfns = TestHostFunctions{env, 0}; - perf(kRuns, [&] { + perf(hfns, kRuns, [&] { return runEscrowWasm(wasm, hfns, 1'000'000, escrowFunctionName, {}).has_value(); }); } @@ -1598,10 +1584,11 @@ struct Wasm_test : public beast::unit_test::Suite auto const wasm = hexToBytes(kAllHostFunctionsWasmHex); - Env const env{*this}; + Env env{*this}; auto mock = HostFunctions{env.journal}; + auto hfns = TestHostFunctions{env, 0}; - perf(kRuns, [&] { return !preflightEscrowWasm(wasm, mock, escrowFunctionName); }); + perf(hfns, kRuns, [&] { return !preflightEscrowWasm(wasm, mock, escrowFunctionName); }); } void diff --git a/src/xrpld/app/main/CollectorManager.cpp b/src/xrpld/app/main/CollectorManager.cpp index 6cdbca8d8a..f025fd0f57 100644 --- a/src/xrpld/app/main/CollectorManager.cpp +++ b/src/xrpld/app/main/CollectorManager.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include diff --git a/src/xrpld/app/main/CollectorManager.h b/src/xrpld/app/main/CollectorManager.h index e736ae57db..c3b1530763 100644 --- a/src/xrpld/app/main/CollectorManager.h +++ b/src/xrpld/app/main/CollectorManager.h @@ -1,24 +1,6 @@ #pragma once -#include -#include - -namespace xrpl { - -/** Provides the beast::insight::Collector service. */ -class CollectorManager -{ -public: - virtual ~CollectorManager() = default; - - virtual beast::insight::Collector::ptr const& - collector() = 0; - - virtual beast::insight::Group::ptr const& - group(std::string const& name) = 0; -}; - -std::unique_ptr -makeCollectorManager(Section const& params, beast::Journal journal); - -} // namespace xrpl +// The CollectorManager interface moved to so it +// is visible to libxrpl (which cannot depend on xrpld). This shim preserves the +// historical include path for existing xrpld consumers. +#include