From 2017c93d36051d84cb17833ec16905a3e62c3b20 Mon Sep 17 00:00:00 2001 From: "Matthew \"strager\" Glazar" Date: Mon, 6 Nov 2023 17:28:23 -0500 Subject: [PATCH] refactor(configuration): prefer Vector over std::vector Reduce our use of std::vector to reduce binary bloat: https://github.com/quick-lint/quick-lint-js/issues/689 --- src/quick-lint-js/cli/main.cpp | 3 ++- .../change-detecting-filesystem-inotify.cpp | 9 ++++++--- .../change-detecting-filesystem-kqueue.cpp | 9 ++++++--- .../change-detecting-filesystem-win32.cpp | 9 ++++++--- .../configuration/change-detecting-filesystem.h | 14 ++++++++------ test/test-configuration-loader.cpp | 17 ++++++++++------- 6 files changed, 38 insertions(+), 23 deletions(-) diff --git a/src/quick-lint-js/cli/main.cpp b/src/quick-lint-js/cli/main.cpp index 7e5e8a5f30..0124daa9f7 100644 --- a/src/quick-lint-js/cli/main.cpp +++ b/src/quick-lint-js/cli/main.cpp @@ -590,8 +590,9 @@ void run_lsp_server() { } void report_pending_watch_io_errors() { + Monotonic_Allocator temporary_allocator("report_pending_watch_io_errors"); this->handler_.add_watch_io_errors( - Span(this->fs_.take_watch_errors())); + this->fs_.take_watch_errors(&temporary_allocator)); this->handler_.flush_pending_notifications(this->writer_); #if QLJS_EVENT_LOOP2_PIPE_WRITE this->enable_or_disable_writer_events_as_needed(); diff --git a/src/quick-lint-js/configuration/change-detecting-filesystem-inotify.cpp b/src/quick-lint-js/configuration/change-detecting-filesystem-inotify.cpp index 8335efd31f..fb7cae7be9 100644 --- a/src/quick-lint-js/configuration/change-detecting-filesystem-inotify.cpp +++ b/src/quick-lint-js/configuration/change-detecting-filesystem-inotify.cpp @@ -158,9 +158,12 @@ void Change_Detecting_Filesystem_Inotify::handle_poll_event(short revents) { } } -std::vector -Change_Detecting_Filesystem_Inotify::take_watch_errors() { - return std::exchange(this->watch_errors_, std::vector()); +Span Change_Detecting_Filesystem_Inotify::take_watch_errors( + Monotonic_Allocator* allocator) { + Span result = allocator->new_objects_copy( + Span(this->watch_errors_)); + this->watch_errors_.clear(); + return result; } void Change_Detecting_Filesystem_Inotify::read_inotify() { diff --git a/src/quick-lint-js/configuration/change-detecting-filesystem-kqueue.cpp b/src/quick-lint-js/configuration/change-detecting-filesystem-kqueue.cpp index bdfccc6f98..72222086c1 100644 --- a/src/quick-lint-js/configuration/change-detecting-filesystem-kqueue.cpp +++ b/src/quick-lint-js/configuration/change-detecting-filesystem-kqueue.cpp @@ -103,9 +103,12 @@ void Change_Detecting_Filesystem_Kqueue::on_canonicalize_child_of_directory( } } -std::vector -Change_Detecting_Filesystem_Kqueue::take_watch_errors() { - return std::exchange(this->watch_errors_, std::vector()); +Span Change_Detecting_Filesystem_Kqueue::take_watch_errors( + Monotonic_Allocator* allocator) { + Span result = allocator->new_objects_copy( + Span(this->watch_errors_)); + this->watch_errors_.clear(); + return result; } void Change_Detecting_Filesystem_Kqueue::on_canonicalize_child_of_directory( diff --git a/src/quick-lint-js/configuration/change-detecting-filesystem-win32.cpp b/src/quick-lint-js/configuration/change-detecting-filesystem-win32.cpp index 98a54b680c..fda88953c9 100644 --- a/src/quick-lint-js/configuration/change-detecting-filesystem-win32.cpp +++ b/src/quick-lint-js/configuration/change-detecting-filesystem-win32.cpp @@ -154,9 +154,12 @@ void Change_Detecting_Filesystem_Win32::clear_watches() { } } -std::vector -Change_Detecting_Filesystem_Win32::take_watch_errors() { - return std::exchange(this->watch_errors_, std::vector()); +Span Change_Detecting_Filesystem_Win32::take_watch_errors( + Monotonic_Allocator* allocator) { + Span result = allocator->new_objects_copy( + Span(this->watch_errors_)); + this->watch_errors_.clear(); + return result; } void Change_Detecting_Filesystem_Win32::cancel_watch( diff --git a/src/quick-lint-js/configuration/change-detecting-filesystem.h b/src/quick-lint-js/configuration/change-detecting-filesystem.h index 0e04113d43..e5d734585f 100644 --- a/src/quick-lint-js/configuration/change-detecting-filesystem.h +++ b/src/quick-lint-js/configuration/change-detecting-filesystem.h @@ -11,10 +11,12 @@ #include #include #include +#include #include #include #include #include +#include #include #include #include @@ -72,7 +74,7 @@ class Change_Detecting_Filesystem_Inotify : public Configuration_Filesystem, std::optional get_inotify_fd(); void handle_poll_event(short revents); - std::vector take_watch_errors(); + Span take_watch_errors(Monotonic_Allocator*); private: // Sets errno and returns false on failure. @@ -82,7 +84,7 @@ class Change_Detecting_Filesystem_Inotify : public Configuration_Filesystem, void read_inotify(); std::vector watch_descriptors_; - std::vector watch_errors_; + Vector watch_errors_{"watch_errors_", new_delete_resource()}; Result inotify_fd_; }; #endif @@ -111,7 +113,7 @@ class Change_Detecting_Filesystem_Kqueue : public Configuration_Filesystem, void handle_kqueue_event(const struct ::kevent&); - std::vector take_watch_errors(); + Span take_watch_errors(Monotonic_Allocator*); private: struct File_ID { @@ -142,7 +144,7 @@ class Change_Detecting_Filesystem_Kqueue : public Configuration_Filesystem, void* udata_; Hash_Map watched_files_; - std::vector watch_errors_; + Vector watch_errors_{"watch_errors_", new_delete_resource()}; }; #endif @@ -175,7 +177,7 @@ class Change_Detecting_Filesystem_Win32 : public Configuration_Filesystem { void clear_watches(); - std::vector take_watch_errors(); + Span take_watch_errors(Monotonic_Allocator*); private: struct Watched_Directory { @@ -214,7 +216,7 @@ class Change_Detecting_Filesystem_Win32 : public Configuration_Filesystem { watched_directories_; std::vector> cancelling_watched_directories_; - std::vector watch_errors_; + Vector watch_errors_{"watch_errors_", new_delete_resource()}; }; #endif QLJS_WARNING_POP diff --git a/test/test-configuration-loader.cpp b/test/test-configuration-loader.cpp index e20ffd4113..931a1ade47 100644 --- a/test/test-configuration-loader.cpp +++ b/test/test-configuration-loader.cpp @@ -142,11 +142,11 @@ class Change_Detecting_Configuration_Loader { return this->loader_.unwatch_all_files(); } - auto fs_take_watch_errors() { + auto fs_take_watch_errors(Monotonic_Allocator* allocator) { #if defined(_WIN32) std::lock_guard lock(this->mutex_); #endif - return this->fs_.take_watch_errors(); + return this->fs_.take_watch_errors(allocator); } std::vector detect_changes_and_refresh(); @@ -190,7 +190,10 @@ class Change_Detecting_Configuration_Loader { }; class Test_Configuration_Loader : public ::testing::Test, - protected Filesystem_Test {}; + protected Filesystem_Test { + public: + Monotonic_Allocator allocator{"Test_Configuration_Loader"}; +}; TEST_F(Test_Configuration_Loader, file_with_no_config_file_gets_default_config) { @@ -2101,7 +2104,7 @@ TEST_F(Test_Configuration_Loader, loader.watch_and_load_config_file(config_file, /*token=*/nullptr); EXPECT_TRUE(loaded_config.ok()) << loaded_config.error_to_string(); - std::vector errors = loader.fs_take_watch_errors(); + Span errors = loader.fs_take_watch_errors(&this->allocator); ASSERT_THAT(errors, ElementsAreArray({::testing::_})); const Watch_IO_Error& error = errors[0]; EXPECT_EQ(error.io_error.error, EMFILE) << error.to_string(); @@ -2123,7 +2126,7 @@ TEST_F(Test_Configuration_Loader, loader.watch_and_load_config_file(config_file, /*token=*/nullptr); EXPECT_TRUE(loaded_config.ok()) << loaded_config.error_to_string(); - std::vector errors = loader.fs_take_watch_errors(); + Span errors = loader.fs_take_watch_errors(&this->allocator); std::vector error_paths; for (Watch_IO_Error& error : errors) { EXPECT_EQ(error.io_error.error, ENOSPC) << error.to_string(); @@ -2152,7 +2155,7 @@ TEST_F(Test_Configuration_Loader, loader.watch_and_load_config_file(config_file, /*token=*/nullptr); EXPECT_TRUE(loaded_config.ok()) << loaded_config.error_to_string(); - std::vector errors = loader.fs_take_watch_errors(); + Span errors = loader.fs_take_watch_errors(&this->allocator); std::vector error_paths; for (Watch_IO_Error& error : errors) { EXPECT_EQ(error.io_error.error, EMFILE) << error.to_string(); @@ -2193,7 +2196,7 @@ TEST_F(Test_Configuration_Loader, loader.watch_and_load_config_file(config_file, /*token=*/nullptr); EXPECT_TRUE(loaded_config.ok()) << loaded_config.error_to_string(); - std::vector errors = loader.fs_take_watch_errors(); + Span errors = loader.fs_take_watch_errors(&this->allocator); std::vector error_paths; for (Watch_IO_Error& error : errors) { EXPECT_EQ(error.io_error.error, mock_error) << error.to_string();