From a79dcc5e63aeeb5a115902d55ebaaf9cf6a74c90 Mon Sep 17 00:00:00 2001 From: "Matthew \"strager\" Glazar" Date: Mon, 6 Nov 2023 22:59:26 -0500 Subject: [PATCH] refactor(container): lazily-load Vector_Instrumentation Some global objects, such as Configuration objects in tests, want to use Vector. However, they cannot use Vector because, in instrumented builds, Instrumented_Vector<>'s constructor appends data to Vector_Instrumentation and the initialization order is not guaranteed (thus Vector_Instrumentation could be initialized *after* the Instrumented_Vector<>.) Fix the initialization order problem by lazily-loading the Vector_Instrumentation singleton. Deinitialization order is still a problem and will be fixed in a future commit. --- .../container/vector-profiler.cpp | 7 +++-- src/quick-lint-js/container/vector-profiler.h | 4 +-- src/quick-lint-js/debug/debug-server.cpp | 2 +- test/test-debug-server.cpp | 4 +-- test/test-vector-profiler.cpp | 26 +++++++++---------- 5 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/quick-lint-js/container/vector-profiler.cpp b/src/quick-lint-js/container/vector-profiler.cpp index fcd319a586..71437b8dba 100644 --- a/src/quick-lint-js/container/vector-profiler.cpp +++ b/src/quick-lint-js/container/vector-profiler.cpp @@ -23,7 +23,10 @@ QLJS_WARNING_IGNORE_MSVC(4996) // Function or variable may be unsafe. namespace quick_lint_js { #if QLJS_FEATURE_VECTOR_PROFILING -Vector_Instrumentation Vector_Instrumentation::instance; +Vector_Instrumentation &Vector_Instrumentation::instance() { + static Vector_Instrumentation instrumentation; + return instrumentation; +} #endif void Vector_Instrumentation::clear() { this->entries_.lock()->clear(); } @@ -68,7 +71,7 @@ void Vector_Instrumentation::register_dump_on_exit_if_requested() { (void)File_Output_Stream::get_stderr(); std::atexit([]() -> void { - auto entries = instance.entries(); + auto entries = instance().entries(); Output_Stream &out = *File_Output_Stream::get_stderr(); { diff --git a/src/quick-lint-js/container/vector-profiler.h b/src/quick-lint-js/container/vector-profiler.h index 87d53699e2..c7c53684e8 100644 --- a/src/quick-lint-js/container/vector-profiler.h +++ b/src/quick-lint-js/container/vector-profiler.h @@ -52,7 +52,7 @@ class Vector_Instrumentation { }; #if QLJS_FEATURE_VECTOR_PROFILING - static Vector_Instrumentation instance; + static Vector_Instrumentation &instance(); #endif void clear(); @@ -366,7 +366,7 @@ class Instrumented_Vector { private: QLJS_FORCE_INLINE void add_instrumentation_entry( Vector_Instrumentation::Event event) { - Vector_Instrumentation::instance.add_entry( + Vector_Instrumentation::instance().add_entry( /*object_id=*/reinterpret_cast(this), /*owner=*/this->debug_owner_, /*event=*/event, diff --git a/src/quick-lint-js/debug/debug-server.cpp b/src/quick-lint-js/debug/debug-server.cpp index 21131181ba..45b0523186 100644 --- a/src/quick-lint-js/debug/debug-server.cpp +++ b/src/quick-lint-js/debug/debug-server.cpp @@ -356,7 +356,7 @@ void Debug_Server::wakeup_pipe_callback(::mg_connection *c, int ev, void *) { this->need_publish_vector_profile_.store(false); this->max_size_histogram_.add_entries( - Vector_Instrumentation::instance.take_entries()); + Vector_Instrumentation::instance().take_entries()); Trace_Writer *tw = Trace_Flusher::instance()->trace_writer_for_current_thread(); diff --git a/test/test-debug-server.cpp b/test/test-debug-server.cpp index 7ffbd49f59..518082508c 100644 --- a/test/test-debug-server.cpp +++ b/test/test-debug-server.cpp @@ -164,7 +164,7 @@ TEST_F(Test_Debug_Server, two_servers_listening_on_same_port_fails) { #if QLJS_FEATURE_VECTOR_PROFILING TEST_F(Test_Debug_Server, web_socket_publishes_vector_profile_stats_on_connect) { - Vector_Instrumentation::instance.clear(); + Vector_Instrumentation::instance().clear(); { Instrumented_Vector> v("debug server test vector", {}); ASSERT_EQ(v.size(), 0); @@ -222,7 +222,7 @@ TEST_F(Test_Debug_Server, } TEST_F(Test_Debug_Server, vector_profile_probe_publishes_stats) { - Vector_Instrumentation::instance.clear(); + Vector_Instrumentation::instance().clear(); bool received_vector_profile_entry = false; auto on_trace_event = diff --git a/test/test-vector-profiler.cpp b/test/test-vector-profiler.cpp index c35ecfe705..d4b4327d5e 100644 --- a/test/test-vector-profiler.cpp +++ b/test/test-vector-profiler.cpp @@ -40,7 +40,7 @@ using Test_Vector = Instrumented_Vector>; class Test_Instrumented_Vector : public ::testing::Test { public: - void SetUp() override { Vector_Instrumentation::instance.clear(); } + void SetUp() override { Vector_Instrumentation::instance().clear(); } }; TEST_F(Test_Instrumented_Vector, @@ -53,7 +53,7 @@ TEST_F(Test_Instrumented_Vector, } EXPECT_THAT( - Vector_Instrumentation::instance.take_entries(), + Vector_Instrumentation::instance().take_entries(), ElementsAre( AllOf(FIELD_EQ(Vector_Instrumentation::Entry, object_id, v_object_id), FIELD_EQ(Vector_Instrumentation::Entry, owner, owner), @@ -74,7 +74,7 @@ TEST_F(Test_Instrumented_Vector, creating_vector_from_range_adds_entry) { std::uintptr_t v_object_id = reinterpret_cast(&v); EXPECT_THAT( - Vector_Instrumentation::instance.take_entries(), + Vector_Instrumentation::instance().take_entries(), ElementsAreArray({ AllOf(FIELD_EQ(Vector_Instrumentation::Entry, object_id, v_object_id), FIELD_EQ(Vector_Instrumentation::Entry, owner, owner), @@ -87,7 +87,7 @@ TEST_F(Test_Instrumented_Vector, creating_vector_from_range_adds_entry) { TEST_F(Test_Instrumented_Vector, append_to_vector_adds_entries) { Test_Vector v("test vector", {}); - Vector_Instrumentation::instance.clear(); + Vector_Instrumentation::instance().clear(); v.emplace_back(100); v.emplace_back(200); @@ -95,7 +95,7 @@ TEST_F(Test_Instrumented_Vector, append_to_vector_adds_entries) { v.emplace_back(400); EXPECT_THAT( - Vector_Instrumentation::instance.take_entries(), + Vector_Instrumentation::instance().take_entries(), ElementsAre(AllOf(FIELD_EQ(Vector_Instrumentation::Entry, event, Vector_Instrumentation::Event::append), FIELD_EQ(Vector_Instrumentation::Entry, size, 1)), @@ -114,11 +114,11 @@ TEST_F(Test_Instrumented_Vector, clearing_vector_adds_entry) { Test_Vector v("test vector", {}); v.emplace_back(100); v.emplace_back(200); - Vector_Instrumentation::instance.clear(); + Vector_Instrumentation::instance().clear(); v.clear(); - EXPECT_THAT(Vector_Instrumentation::instance.take_entries(), + EXPECT_THAT(Vector_Instrumentation::instance().take_entries(), ElementsAreArray({ AllOf(FIELD_EQ(Vector_Instrumentation::Entry, event, Vector_Instrumentation::Event::clear), @@ -132,14 +132,14 @@ TEST_F(Test_Instrumented_Vector, moving_vector_with_new_owner_adds_entries) { std::uintptr_t v_1_object_id = reinterpret_cast(&v_1); v_1.emplace_back(100); v_1.emplace_back(200); - Vector_Instrumentation::instance.clear(); + Vector_Instrumentation::instance().clear(); const char *v_2_owner = "v2"; Test_Vector v_2(v_2_owner, std::move(v_1)); std::uintptr_t v_2_object_id = reinterpret_cast(&v_2); EXPECT_THAT( - Vector_Instrumentation::instance.take_entries(), + Vector_Instrumentation::instance().take_entries(), ElementsAre( AllOf( FIELD_EQ(Vector_Instrumentation::Entry, owner, v_2_owner), @@ -161,13 +161,13 @@ TEST_F(Test_Instrumented_Vector, moving_vector_with_no_owner_adds_entries) { std::uintptr_t v_1_object_id = reinterpret_cast(&v_1); v_1.emplace_back(100); v_1.emplace_back(200); - Vector_Instrumentation::instance.clear(); + Vector_Instrumentation::instance().clear(); Test_Vector v_2(std::move(v_1)); std::uintptr_t v_2_object_id = reinterpret_cast(&v_2); EXPECT_THAT( - Vector_Instrumentation::instance.take_entries(), + Vector_Instrumentation::instance().take_entries(), ElementsAre( AllOf( FIELD_EQ(Vector_Instrumentation::Entry, owner, v_1_owner), @@ -193,12 +193,12 @@ TEST_F(Test_Instrumented_Vector, move_assigning_vector_adds_entries) { v_2.emplace_back(200); v_2.emplace_back(300); std::uintptr_t v_2_object_id = reinterpret_cast(&v_2); - Vector_Instrumentation::instance.clear(); + Vector_Instrumentation::instance().clear(); v_1 = std::move(v_2); EXPECT_THAT( - Vector_Instrumentation::instance.take_entries(), + Vector_Instrumentation::instance().take_entries(), ElementsAre( AllOf( FIELD_EQ(Vector_Instrumentation::Entry, owner, v_1_owner),