Skip to content

Commit

Permalink
refactor(configuration): prefer Vector over std::vector
Browse files Browse the repository at this point in the history
Reduce our use of std::vector to reduce binary bloat:
#689
  • Loading branch information
strager committed Nov 6, 2023
1 parent 57d1fbf commit 2017c93
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 23 deletions.
3 changes: 2 additions & 1 deletion src/quick-lint-js/cli/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<const Watch_IO_Error>(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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,12 @@ void Change_Detecting_Filesystem_Inotify::handle_poll_event(short revents) {
}
}

std::vector<Watch_IO_Error>
Change_Detecting_Filesystem_Inotify::take_watch_errors() {
return std::exchange(this->watch_errors_, std::vector<Watch_IO_Error>());
Span<Watch_IO_Error> Change_Detecting_Filesystem_Inotify::take_watch_errors(
Monotonic_Allocator* allocator) {
Span<Watch_IO_Error> result = allocator->new_objects_copy(
Span<const Watch_IO_Error>(this->watch_errors_));
this->watch_errors_.clear();
return result;
}

void Change_Detecting_Filesystem_Inotify::read_inotify() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,12 @@ void Change_Detecting_Filesystem_Kqueue::on_canonicalize_child_of_directory(
}
}

std::vector<Watch_IO_Error>
Change_Detecting_Filesystem_Kqueue::take_watch_errors() {
return std::exchange(this->watch_errors_, std::vector<Watch_IO_Error>());
Span<Watch_IO_Error> Change_Detecting_Filesystem_Kqueue::take_watch_errors(
Monotonic_Allocator* allocator) {
Span<Watch_IO_Error> result = allocator->new_objects_copy(
Span<const Watch_IO_Error>(this->watch_errors_));
this->watch_errors_.clear();
return result;
}

void Change_Detecting_Filesystem_Kqueue::on_canonicalize_child_of_directory(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,12 @@ void Change_Detecting_Filesystem_Win32::clear_watches() {
}
}

std::vector<Watch_IO_Error>
Change_Detecting_Filesystem_Win32::take_watch_errors() {
return std::exchange(this->watch_errors_, std::vector<Watch_IO_Error>());
Span<Watch_IO_Error> Change_Detecting_Filesystem_Win32::take_watch_errors(
Monotonic_Allocator* allocator) {
Span<Watch_IO_Error> result = allocator->new_objects_copy(
Span<const Watch_IO_Error>(this->watch_errors_));
this->watch_errors_.clear();
return result;
}

void Change_Detecting_Filesystem_Win32::cancel_watch(
Expand Down
14 changes: 8 additions & 6 deletions src/quick-lint-js/configuration/change-detecting-filesystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
#include <optional>
#include <quick-lint-js/configuration/configuration-loader.h>
#include <quick-lint-js/container/hash-map.h>
#include <quick-lint-js/container/vector.h>
#include <quick-lint-js/io/file-canonical.h>
#include <quick-lint-js/io/file-handle.h>
#include <quick-lint-js/io/file.h>
#include <quick-lint-js/port/have.h>
#include <quick-lint-js/port/memory-resource.h>
#include <quick-lint-js/port/warning.h>
#include <string>
#include <vector>
Expand Down Expand Up @@ -72,7 +74,7 @@ class Change_Detecting_Filesystem_Inotify : public Configuration_Filesystem,
std::optional<POSIX_FD_File_Ref> get_inotify_fd();
void handle_poll_event(short revents);

std::vector<Watch_IO_Error> take_watch_errors();
Span<Watch_IO_Error> take_watch_errors(Monotonic_Allocator*);

private:
// Sets errno and returns false on failure.
Expand All @@ -82,7 +84,7 @@ class Change_Detecting_Filesystem_Inotify : public Configuration_Filesystem,
void read_inotify();

std::vector<int> watch_descriptors_;
std::vector<Watch_IO_Error> watch_errors_;
Vector<Watch_IO_Error> watch_errors_{"watch_errors_", new_delete_resource()};
Result<POSIX_FD_File, POSIX_File_IO_Error> inotify_fd_;
};
#endif
Expand Down Expand Up @@ -111,7 +113,7 @@ class Change_Detecting_Filesystem_Kqueue : public Configuration_Filesystem,

void handle_kqueue_event(const struct ::kevent&);

std::vector<Watch_IO_Error> take_watch_errors();
Span<Watch_IO_Error> take_watch_errors(Monotonic_Allocator*);

private:
struct File_ID {
Expand Down Expand Up @@ -142,7 +144,7 @@ class Change_Detecting_Filesystem_Kqueue : public Configuration_Filesystem,
void* udata_;

Hash_Map<Canonical_Path, Watched_File> watched_files_;
std::vector<Watch_IO_Error> watch_errors_;
Vector<Watch_IO_Error> watch_errors_{"watch_errors_", new_delete_resource()};
};
#endif

Expand Down Expand Up @@ -175,7 +177,7 @@ class Change_Detecting_Filesystem_Win32 : public Configuration_Filesystem {

void clear_watches();

std::vector<Watch_IO_Error> take_watch_errors();
Span<Watch_IO_Error> take_watch_errors(Monotonic_Allocator*);

private:
struct Watched_Directory {
Expand Down Expand Up @@ -214,7 +216,7 @@ class Change_Detecting_Filesystem_Win32 : public Configuration_Filesystem {
watched_directories_;
std::vector<std::unique_ptr<Watched_Directory>>
cancelling_watched_directories_;
std::vector<Watch_IO_Error> watch_errors_;
Vector<Watch_IO_Error> watch_errors_{"watch_errors_", new_delete_resource()};
};
#endif
QLJS_WARNING_POP
Expand Down
17 changes: 10 additions & 7 deletions test/test-configuration-loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Mutex> lock(this->mutex_);
#endif
return this->fs_.take_watch_errors();
return this->fs_.take_watch_errors(allocator);
}

std::vector<Configuration_Change> detect_changes_and_refresh();
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<Watch_IO_Error> errors = loader.fs_take_watch_errors();
Span<Watch_IO_Error> 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();
Expand All @@ -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<Watch_IO_Error> errors = loader.fs_take_watch_errors();
Span<Watch_IO_Error> errors = loader.fs_take_watch_errors(&this->allocator);
std::vector<std::string> error_paths;
for (Watch_IO_Error& error : errors) {
EXPECT_EQ(error.io_error.error, ENOSPC) << error.to_string();
Expand Down Expand Up @@ -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<Watch_IO_Error> errors = loader.fs_take_watch_errors();
Span<Watch_IO_Error> errors = loader.fs_take_watch_errors(&this->allocator);
std::vector<std::string> error_paths;
for (Watch_IO_Error& error : errors) {
EXPECT_EQ(error.io_error.error, EMFILE) << error.to_string();
Expand Down Expand Up @@ -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<Watch_IO_Error> errors = loader.fs_take_watch_errors();
Span<Watch_IO_Error> errors = loader.fs_take_watch_errors(&this->allocator);
std::vector<std::string> error_paths;
for (Watch_IO_Error& error : errors) {
EXPECT_EQ(error.io_error.error, mock_error) << error.to_string();
Expand Down

0 comments on commit 2017c93

Please sign in to comment.