From 44d06b20f1c8158e4ba6e7e1ee21756bce72f6fd Mon Sep 17 00:00:00 2001 From: "Matthew \"strager\" Glazar" Date: Sat, 28 Oct 2023 14:41:43 -0400 Subject: [PATCH] refactor(container): std::ostream -> Output_Stream for profile dumps The vector profiling code uses std::ostream. I want us to use our memory streams more consistently. Switch the vector profile dumping code to use Output_Stream instead of std::ostream. --- .../container/vector-profiler-debug.cpp | 88 ++++++++----- src/quick-lint-js/container/vector-profiler.h | 7 +- test/test-vector-profiler.cpp | 120 ++++++++++-------- 3 files changed, 128 insertions(+), 87 deletions(-) diff --git a/src/quick-lint-js/container/vector-profiler-debug.cpp b/src/quick-lint-js/container/vector-profiler-debug.cpp index 1452adfbef..aebd4252a1 100644 --- a/src/quick-lint-js/container/vector-profiler-debug.cpp +++ b/src/quick-lint-js/container/vector-profiler-debug.cpp @@ -21,10 +21,6 @@ #include #include -#if QLJS_FEATURE_VECTOR_PROFILING -#include -#endif - QLJS_WARNING_IGNORE_MSVC(4996) // Function or variable may be unsafe. namespace quick_lint_js { @@ -94,31 +90,42 @@ void Vector_Instrumentation::register_dump_on_exit_if_requested() { const char *dump_vectors_value = std::getenv("QLJS_DUMP_VECTORS"); bool should_dump_on_exit = dump_vectors_value && *dump_vectors_value != '\0'; if (should_dump_on_exit) { + // HACK(strager): Force the stderr File_Output_Stream to be destructed + // *after* our std::atexit callback is called. + // + // Without this dummy call, the File_Output_Stream is destructed before our + // callback, causing uses of the File_Output_Stream to likely crash due to + // the vtable being wiped during destruction. + (void)File_Output_Stream::get_stderr(); + std::atexit([]() -> void { auto entries = instance.entries(); + Output_Stream &out = *File_Output_Stream::get_stderr(); { Vector_Max_Size_Histogram_By_Owner hist; hist.add_entries(entries); Monotonic_Allocator memory("Vector_Instrumentation dump_on_exit"); Vector_Max_Size_Histogram_By_Owner::dump( - hist.histogram(&memory), std::cerr, + hist.histogram(&memory), out, Vector_Max_Size_Histogram_By_Owner::Dump_Options{ .maximum_line_length = 80, .max_adjacent_empty_rows = 5, }); } - std::cerr << '\n'; + out.append_copy(u8'\n'); { Vector_Capacity_Change_Histogram_By_Owner hist; hist.add_entries(entries); Vector_Capacity_Change_Histogram_By_Owner::dump( - hist.histogram(), std::cerr, + hist.histogram(), out, Vector_Capacity_Change_Histogram_By_Owner::Dump_Options{ .maximum_line_length = 80, }); } + + out.flush(); }); } } @@ -196,23 +203,25 @@ Vector_Max_Size_Histogram_By_Owner::histogram( void Vector_Max_Size_Histogram_By_Owner::dump( Span histogram, - std::ostream &out) { + Output_Stream &out) { return dump(histogram, out, Dump_Options()); } void Vector_Max_Size_Histogram_By_Owner::dump( Span histogram, - std::ostream &out, const Dump_Options &options) { + Output_Stream &out, const Dump_Options &options) { bool need_blank_line = false; for (const auto &[group_name, object_size_histogram] : histogram) { QLJS_ASSERT(!object_size_histogram.empty()); if (need_blank_line) { - out << '\n'; + out.append_copy(u8'\n'); } need_blank_line = true; - out << "Max sizes for " << out_string8(group_name) << ":\n"; + out.append_literal(u8"Max sizes for "_sv); + out.append_copy(group_name); + out.append_literal(u8":\n"_sv); std::uint64_t max_count = 0; std::uint64_t total_count = 0; @@ -239,35 +248,42 @@ void Vector_Max_Size_Histogram_By_Owner::dump( QLJS_ASSERT(options.max_adjacent_empty_rows > 0); if (object_size - next_object_size > narrow_cast(options.max_adjacent_empty_rows)) { - out << "...\n"; + out.append_literal(u8"...\n"_sv); } else { for (std::size_t i = next_object_size; i < object_size; ++i) { - out << std::setw(max_digits_in_legend) << i << " ( 0%)\n"; + out.append_padded_decimal_integer(i, max_digits_in_legend, u8' '); + out.append_literal(u8" ( 0%)\n"_sv); } } - out << std::setw(max_digits_in_legend) << object_size << " ("; + out.append_padded_decimal_integer(object_size, max_digits_in_legend, + u8' '); + out.append_literal(u8" ("_sv); if (count == total_count) { - out << "ALL"; + out.append_literal(u8"ALL"_sv); } else { double count_fraction = static_cast(count) / static_cast(total_count); - out << std::setw(2) << std::round(count_fraction * 100) << "%"; + out.append_padded_decimal_integer( + static_cast(std::round(count_fraction * 100)), 2, u8' '); + out.append_literal(u8"%"_sv); } - out << ')'; + out.append_copy(u8')'); int bar_width = std::max(1, static_cast(std::floor(static_cast(count) * bar_scale_factor))); - out << " "; + out.append_literal(u8" "_sv); for (int i = 0; i < bar_width; ++i) { - out << '*'; + out.append_copy(u8'*'); } - out << '\n'; + out.append_copy(u8'\n'); next_object_size = object_size + 1; } } + + out.flush(); } Vector_Capacity_Change_Histogram_By_Owner:: @@ -307,10 +323,11 @@ Vector_Capacity_Change_Histogram_By_Owner::histogram() const { void Vector_Capacity_Change_Histogram_By_Owner::dump( const std::map &histogram, - std::ostream &out, const Dump_Options &options) { - out << R"(vector capacity changes: + Output_Stream &out, const Dump_Options &options) { + out.append_literal( + u8R"(vector capacity changes: (C=copied; z=initial alloc; -=used internal capacity) -)"; +)"_sv); int count_width = 1; for (auto &[owner, h] : histogram) { @@ -330,13 +347,20 @@ void Vector_Capacity_Change_Histogram_By_Owner::dump( continue; } - out << owner << ":\n"; + out.append_copy(to_string8_view(owner)); + out.append_literal(u8":\n"_sv); // Example: // 5C 0z 15_ |CCCCC_______________| - out << std::setw(count_width) << h.appends_growing_capacity << "C " - << std::setw(count_width) << h.appends_initial_capacity << "z " - << std::setw(count_width) << h.appends_reusing_capacity << "_ |"; + out.append_padded_decimal_integer(h.appends_growing_capacity, count_width, + u8' '); + out.append_literal(u8"C "_sv); + out.append_padded_decimal_integer(h.appends_initial_capacity, count_width, + u8' '); + out.append_literal(u8"z "_sv); + out.append_padded_decimal_integer(h.appends_reusing_capacity, count_width, + u8' '); + out.append_literal(u8"_ |"_sv); int graph_columns = options.maximum_line_length - (count_width * 3 + narrow_cast(std::strlen("C z _ ||"))); @@ -344,24 +368,24 @@ void Vector_Capacity_Change_Histogram_By_Owner::dump( narrow_cast(narrow_cast(graph_columns) * h.appends_growing_capacity / append_count); // 'C' for (int i = 0; i < columns_growing_capacity; ++i) { - out << 'C'; + out.append_copy(u8'C'); } int columns_initial_capacity = narrow_cast(narrow_cast(graph_columns) * h.appends_initial_capacity / append_count); // 'z' for (int i = 0; i < columns_initial_capacity; ++i) { - out << 'z'; + out.append_copy(u8'z'); } int columns_reusing_capacity = graph_columns - (columns_growing_capacity + columns_initial_capacity); // '_' for (int i = 0; i < columns_reusing_capacity; ++i) { - out << '_'; + out.append_copy(u8'_'); } - out << "|\n"; + out.append_copy(u8"|\n"_sv); } + out.flush(); } - } // quick-lint-js finds bugs in JavaScript programs. diff --git a/src/quick-lint-js/container/vector-profiler.h b/src/quick-lint-js/container/vector-profiler.h index 8e81451583..a84bdf78b6 100644 --- a/src/quick-lint-js/container/vector-profiler.h +++ b/src/quick-lint-js/container/vector-profiler.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -97,9 +98,9 @@ class Vector_Max_Size_Histogram_By_Owner { }; static void dump(Span, - std::ostream &); + Output_Stream &); static void dump(Span, - std::ostream &, const Dump_Options &options); + Output_Stream &, const Dump_Options &options); private: Hash_Map> histogram_; @@ -138,7 +139,7 @@ class Vector_Capacity_Change_Histogram_By_Owner { static void dump( const std::map &, - std::ostream &, const Dump_Options &); + Output_Stream &, const Dump_Options &); private: struct Vector_Info { diff --git a/test/test-vector-profiler.cpp b/test/test-vector-profiler.cpp index 00d4219846..c35ecfe705 100644 --- a/test/test-vector-profiler.cpp +++ b/test/test-vector-profiler.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -578,15 +579,15 @@ TEST_F(Test_Vector_Instrumentation_Max_Size_Histogram_By_Owner, TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, dump_empty_histogram) { - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Max_Size_Histogram_By_Owner::dump( Span(), stream); - EXPECT_EQ(stream.str(), ""); + EXPECT_EQ(stream.get_flushed_string8(), u8""_sv); } TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, dump_histogram_with_one_group) { - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Max_Size_Histogram_By_Owner::dump( Span({ Trace_Vector_Max_Size_Histogram_By_Owner_Entry{ @@ -600,16 +601,17 @@ TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, }, }), stream); - EXPECT_EQ(stream.str(), R"(Max sizes for test group: + EXPECT_EQ(stream.get_flushed_string8(), + u8R"(Max sizes for test group: 0 (50%) *** 1 (33%) ** 2 (17%) * -)"); +)"_sv); } TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, dump_histogram_with_one_data_point_per_group) { - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Max_Size_Histogram_By_Owner::dump( Span({ Trace_Vector_Max_Size_Histogram_By_Owner_Entry{ @@ -621,14 +623,15 @@ TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, }, }), stream); - EXPECT_EQ(stream.str(), R"(Max sizes for test group: + EXPECT_EQ(stream.get_flushed_string8(), + u8R"(Max sizes for test group: 0 (ALL) ** -)"); +)"_sv); } TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, dump_histogram_with_multiple_groups) { - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Max_Size_Histogram_By_Owner::dump( Span({ Trace_Vector_Max_Size_Histogram_By_Owner_Entry{ @@ -649,19 +652,20 @@ TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, }, }), stream); - EXPECT_EQ(stream.str(), R"(Max sizes for group A: + EXPECT_EQ(stream.get_flushed_string8(), + u8R"(Max sizes for group A: 0 (50%) *** 1 (50%) *** Max sizes for group B: 0 (50%) ** 1 (50%) ** -)"); +)"_sv); } TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, dump_sparse_histogram) { - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Max_Size_Histogram_By_Owner::dump( Span({ Trace_Vector_Max_Size_Histogram_By_Owner_Entry{ @@ -676,7 +680,8 @@ TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, }, }), stream); - EXPECT_EQ(stream.str(), R"(Max sizes for test group: + EXPECT_EQ(stream.get_flushed_string8(), + u8R"(Max sizes for test group: 0 ( 0%) 1 (25%) * 2 ( 0%) @@ -687,12 +692,12 @@ TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, 7 ( 0%) 8 ( 0%) 9 (25%) * -)"); +)"_sv); } TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, histogram_legend_is_padded_with_spaces) { - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Max_Size_Histogram_By_Owner::dump( Span({ Trace_Vector_Max_Size_Histogram_By_Owner_Entry{ @@ -705,8 +710,8 @@ TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, }, }), stream); - EXPECT_THAT(stream.str(), HasSubstr("\n 3 (")); - EXPECT_THAT(stream.str(), HasSubstr("\n100 (")); + EXPECT_THAT(to_string(stream.get_flushed_string8()), HasSubstr("\n 3 (")); + EXPECT_THAT(to_string(stream.get_flushed_string8()), HasSubstr("\n100 (")); } TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, @@ -716,7 +721,7 @@ TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, histogram["test group"][1] = 50; histogram["test group"][2] = 25; histogram["test group"][3] = 1; - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Max_Size_Histogram_By_Owner::dump( Span({ Trace_Vector_Max_Size_Histogram_By_Owner_Entry{ @@ -734,17 +739,18 @@ TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, Vector_Max_Size_Histogram_By_Owner::Dump_Options{ .maximum_line_length = 20, }); - EXPECT_EQ(stream.str(), R"(Max sizes for test group: + EXPECT_EQ(stream.get_flushed_string8(), + u8R"(Max sizes for test group: 0 (57%) ********** 1 (28%) ***** 2 (14%) ** 3 ( 1%) * -)"); +)"_sv); } TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, histogram_skips_many_empty_rows) { - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Max_Size_Histogram_By_Owner::dump( Span({ Trace_Vector_Max_Size_Histogram_By_Owner_Entry{ @@ -762,18 +768,19 @@ TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, Vector_Max_Size_Histogram_By_Owner::Dump_Options{ .max_adjacent_empty_rows = 3, }); - EXPECT_EQ(stream.str(), R"(Max sizes for test group: + EXPECT_EQ(stream.get_flushed_string8(), + u8R"(Max sizes for test group: 0 (20%) * 1 (40%) ** 2 (20%) * ... 8 (20%) * -)"); +)"_sv); } TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, histogram_including_legend_is_limited_to_max_screen_width) { - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Max_Size_Histogram_By_Owner::dump( Span({ Trace_Vector_Max_Size_Histogram_By_Owner_Entry{ @@ -788,7 +795,8 @@ TEST(Test_Vector_Instrumentation_Dump_Max_Size_Histogram, Vector_Max_Size_Histogram_By_Owner::Dump_Options{ .maximum_line_length = 20, }); - EXPECT_THAT(stream.str(), HasSubstr("\n100 (ALL) ********\n")); + EXPECT_THAT(to_string(stream.get_flushed_string8()), + HasSubstr("\n100 (ALL) ********\n")); } TEST(Test_Vector_Instrumentation_Capacity_Change_Histogram_By_Owner, @@ -1084,20 +1092,21 @@ TEST(Test_Vector_Instrumentation_Capacity_Change_Histogram_By_Owner, EXPECT_EQ(hist["myvector"].appends_growing_capacity, 1) << "first vector"; } -std::string dump_capacity_change_header = R"(vector capacity changes: +constexpr String8_View dump_capacity_change_header = + u8R"(vector capacity changes: (C=copied; z=initial alloc; -=used internal capacity) -)"; +)"_sv; TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, dump_empty_histogram) { std::map histogram; - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Capacity_Change_Histogram_By_Owner::dump( histogram, stream, Vector_Capacity_Change_Histogram_By_Owner::Dump_Options{}); - EXPECT_EQ(stream.str(), dump_capacity_change_header); + EXPECT_EQ(stream.get_flushed_string8(), dump_capacity_change_header); } TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, @@ -1106,15 +1115,16 @@ TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, Vector_Capacity_Change_Histogram_By_Owner::Capacity_Change_Histogram> histogram; histogram["myvector"].appends_reusing_capacity = 10; - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Capacity_Change_Histogram_By_Owner::dump( histogram, stream, Vector_Capacity_Change_Histogram_By_Owner::Dump_Options{ .maximum_line_length = 34, }); - EXPECT_EQ(stream.str(), dump_capacity_change_header + R"(myvector: + EXPECT_EQ(stream.get_flushed_string8(), concat(dump_capacity_change_header, + u8R"(myvector: 0C 0z 10_ |____________________| -)"); +)"_sv)); } TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, @@ -1123,15 +1133,16 @@ TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, Vector_Capacity_Change_Histogram_By_Owner::Capacity_Change_Histogram> histogram; histogram["myvector"].appends_growing_capacity = 10; - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Capacity_Change_Histogram_By_Owner::dump( histogram, stream, Vector_Capacity_Change_Histogram_By_Owner::Dump_Options{ .maximum_line_length = 34, }); - EXPECT_EQ(stream.str(), dump_capacity_change_header + R"(myvector: + EXPECT_EQ(stream.get_flushed_string8(), concat(dump_capacity_change_header, + u8R"(myvector: 10C 0z 0_ |CCCCCCCCCCCCCCCCCCCC| -)"); +)"_sv)); } TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, @@ -1140,15 +1151,16 @@ TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, Vector_Capacity_Change_Histogram_By_Owner::Capacity_Change_Histogram> histogram; histogram["myvector"].appends_initial_capacity = 10; - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Capacity_Change_Histogram_By_Owner::dump( histogram, stream, Vector_Capacity_Change_Histogram_By_Owner::Dump_Options{ .maximum_line_length = 34, }); - EXPECT_EQ(stream.str(), dump_capacity_change_header + R"(myvector: + EXPECT_EQ(stream.get_flushed_string8(), concat(dump_capacity_change_header, + u8R"(myvector: 0C 10z 0_ |zzzzzzzzzzzzzzzzzzzz| -)"); +)"_sv)); } TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, @@ -1159,15 +1171,16 @@ TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, histogram["myvector"].appends_growing_capacity = 5; histogram["myvector"].appends_initial_capacity = 5; histogram["myvector"].appends_reusing_capacity = 10; - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Capacity_Change_Histogram_By_Owner::dump( histogram, stream, Vector_Capacity_Change_Histogram_By_Owner::Dump_Options{ .maximum_line_length = 34, }); - EXPECT_EQ(stream.str(), dump_capacity_change_header + R"(myvector: + EXPECT_EQ(stream.get_flushed_string8(), concat(dump_capacity_change_header, + u8R"(myvector: 5C 5z 10_ |CCCCCzzzzz__________| -)"); +)"_sv)); } TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, @@ -1178,27 +1191,29 @@ TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, histogram["myvector"].appends_growing_capacity = 9001; { - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Capacity_Change_Histogram_By_Owner::dump( histogram, stream, Vector_Capacity_Change_Histogram_By_Owner::Dump_Options{ .maximum_line_length = 30, }); - EXPECT_EQ(stream.str(), dump_capacity_change_header + R"(myvector: + EXPECT_EQ(stream.get_flushed_string8(), concat(dump_capacity_change_header, + u8R"(myvector: 9001C 0z 0_ |CCCCCCCCCC| -)"); +)"_sv)); } { - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Capacity_Change_Histogram_By_Owner::dump( histogram, stream, Vector_Capacity_Change_Histogram_By_Owner::Dump_Options{ .maximum_line_length = 50, }); - EXPECT_EQ(stream.str(), dump_capacity_change_header + R"(myvector: + EXPECT_EQ(stream.get_flushed_string8(), concat(dump_capacity_change_header, + u8R"(myvector: 9001C 0z 0_ |CCCCCCCCCCCCCCCCCCCCCCCCCCCCCC| -)"); +)"_sv)); } } @@ -1211,17 +1226,18 @@ TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, histogram["first"].appends_reusing_capacity = 1; histogram["second"].appends_growing_capacity = 30; histogram["second"].appends_reusing_capacity = 30; - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Capacity_Change_Histogram_By_Owner::dump( histogram, stream, Vector_Capacity_Change_Histogram_By_Owner::Dump_Options{ .maximum_line_length = 30, }); - EXPECT_EQ(stream.str(), dump_capacity_change_header + R"(first: + EXPECT_EQ(stream.get_flushed_string8(), concat(dump_capacity_change_header, + u8R"(first: 1C 0z 1_ |CCCCCCCC________| second: 30C 0z 30_ |CCCCCCCC________| -)"); +)"_sv)); } TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, @@ -1231,11 +1247,11 @@ TEST(Test_Vector_Instrumentation_Dump_Capacity_Change_Histogram, histogram; histogram["first"]; // Zeroes. histogram["second"]; // Zeroes. - std::ostringstream stream; + Memory_Output_Stream stream; Vector_Capacity_Change_Histogram_By_Owner::dump( histogram, stream, Vector_Capacity_Change_Histogram_By_Owner::Dump_Options{}); - EXPECT_EQ(stream.str(), dump_capacity_change_header); + EXPECT_EQ(stream.get_flushed_string8(), dump_capacity_change_header); } } }