Skip to content

Commit

Permalink
refactor(container): lazily-load Vector_Instrumentation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
strager committed Nov 7, 2023
1 parent 00af184 commit a79dcc5
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 20 deletions.
7 changes: 5 additions & 2 deletions src/quick-lint-js/container/vector-profiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand Down Expand Up @@ -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();

{
Expand Down
4 changes: 2 additions & 2 deletions src/quick-lint-js/container/vector-profiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class Vector_Instrumentation {
};

#if QLJS_FEATURE_VECTOR_PROFILING
static Vector_Instrumentation instance;
static Vector_Instrumentation &instance();
#endif

void clear();
Expand Down Expand Up @@ -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<std::uintptr_t>(this),
/*owner=*/this->debug_owner_,
/*event=*/event,
Expand Down
2 changes: 1 addition & 1 deletion src/quick-lint-js/debug/debug-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions test/test-debug-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::vector<int>> v("debug server test vector", {});
ASSERT_EQ(v.size(), 0);
Expand Down Expand Up @@ -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 =
Expand Down
26 changes: 13 additions & 13 deletions test/test-vector-profiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ using Test_Vector = Instrumented_Vector<std::vector<int>>;

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,
Expand All @@ -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),
Expand All @@ -74,7 +74,7 @@ TEST_F(Test_Instrumented_Vector, creating_vector_from_range_adds_entry) {

std::uintptr_t v_object_id = reinterpret_cast<std::uintptr_t>(&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),
Expand All @@ -87,15 +87,15 @@ TEST_F(Test_Instrumented_Vector, creating_vector_from_range_adds_entry) {

TEST_F(Test_Instrumented_Vector, append_to_vector_adds_entries) {
Test_Vector<int> v("test vector", {});
Vector_Instrumentation::instance.clear();
Vector_Instrumentation::instance().clear();

v.emplace_back(100);
v.emplace_back(200);
v.emplace_back(300);
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)),
Expand All @@ -114,11 +114,11 @@ TEST_F(Test_Instrumented_Vector, clearing_vector_adds_entry) {
Test_Vector<int> 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),
Expand All @@ -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<std::uintptr_t>(&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<int> v_2(v_2_owner, std::move(v_1));
std::uintptr_t v_2_object_id = reinterpret_cast<std::uintptr_t>(&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),
Expand All @@ -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<std::uintptr_t>(&v_1);
v_1.emplace_back(100);
v_1.emplace_back(200);
Vector_Instrumentation::instance.clear();
Vector_Instrumentation::instance().clear();

Test_Vector<int> v_2(std::move(v_1));
std::uintptr_t v_2_object_id = reinterpret_cast<std::uintptr_t>(&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),
Expand All @@ -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<std::uintptr_t>(&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),
Expand Down

0 comments on commit a79dcc5

Please sign in to comment.