Skip to content

Commit

Permalink
refactor(debug): prefer Bump_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 Oct 27, 2023
1 parent 497c58b commit 6c2a407
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 49 deletions.
4 changes: 3 additions & 1 deletion src/quick-lint-js/cli/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,12 +348,14 @@ Linter_Options get_linter_options_from_language(
}

void list_debug_apps() {
Monotonic_Allocator temporary_allocator("list_debug_apps");

struct Table_Row {
std::string process_id;
std::string server_url;
};
std::vector<Table_Row> table;
for (const Found_Debug_Server &s : find_debug_servers()) {
for (const Found_Debug_Server &s : find_debug_servers(&temporary_allocator)) {
table.push_back(Table_Row{
.process_id = std::to_string(s.process_id),
.server_url =
Expand Down
101 changes: 56 additions & 45 deletions src/quick-lint-js/debug/find-debug-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <optional>
#include <quick-lint-js/assert.h>
#include <quick-lint-js/container/string-view.h>
#include <quick-lint-js/container/vector.h>
#include <quick-lint-js/debug/find-debug-server.h>
#include <quick-lint-js/feature.h>
#include <quick-lint-js/io/file-handle.h>
Expand All @@ -22,7 +23,6 @@
#include <quick-lint-js/util/narrow-cast.h>
#include <quick-lint-js/util/utf-16.h>
#include <string_view>
#include <vector>

#if defined(__linux__)
#include <dirent.h>
Expand Down Expand Up @@ -257,8 +257,9 @@ void enumerate_all_process_thread_names(Callback&& callback) {
#endif

#if defined(__linux__)
std::vector<Found_Debug_Server> find_debug_servers() {
std::vector<Found_Debug_Server> debug_servers;
Span<Found_Debug_Server> find_debug_servers(Monotonic_Allocator* allocator) {
Bump_Vector<Found_Debug_Server, Monotonic_Allocator> debug_servers(
"debug_servers", allocator);

enumerate_all_process_thread_names([&](std::string_view process_id_string,
std::string_view thread_name) {
Expand All @@ -277,13 +278,14 @@ std::vector<Found_Debug_Server> find_debug_servers() {
}
});

return debug_servers;
return debug_servers.release_to_span();
}
#endif

#if defined(__APPLE__)
template <class Callback>
void enumerate_all_process_threads(Callback&& callback) {
void enumerate_all_process_threads(Monotonic_Allocator& allocator,
Callback&& callback) {
// Kernel code uses sizeof(int), not sizeof(::pid_t):
// https://github.com/apple/darwin-xnu/blob/8f02f2a044b9bb1ad951987ef5bab20ec9486310/bsd/kern/proc_info.c#L339
//
Expand All @@ -300,10 +302,12 @@ void enumerate_all_process_threads(Callback&& callback) {
QLJS_ASSERT(
narrow_cast<std::size_t>(process_id_buffer_size) % sizeof(::pid_t) == 0);

Bump_Vector<::pid_t, Monotonic_Allocator> process_ids("process_ids",
&allocator);
// NOTE(strager): It's okay if our buffer is to small. We miss out on some
// processes, but they were just created anyway. Harmless race condition.
std::vector<::pid_t> process_ids(
narrow_cast<std::size_t>(process_id_buffer_size) / sizeof(::pid_t));
process_ids.resize(narrow_cast<std::size_t>(process_id_buffer_size) /
sizeof(::pid_t));
process_id_buffer_size =
::proc_listpids(PROC_ALL_PIDS, 0, process_ids.data(),
narrow_cast<int>(process_ids.size() * sizeof(::pid_t)));
Expand All @@ -315,7 +319,8 @@ void enumerate_all_process_threads(Callback&& callback) {
process_ids.resize(narrow_cast<std::size_t>(process_id_buffer_size) /
sizeof(int));

std::vector<std::uint64_t> thread_ids;
Bump_Vector<std::uint64_t, Monotonic_Allocator> thread_ids("thread_ids",
&allocator);
constexpr std::size_t initial_thread_ids_buffer_count = 128; // Arbitrary.
for (::pid_t process_id : process_ids) {
thread_ids.resize(initial_thread_ids_buffer_count);
Expand All @@ -336,7 +341,7 @@ void enumerate_all_process_threads(Callback&& callback) {
std::size_t thread_count =
narrow_cast<std::size_t>(thread_ids_buffer_size) /
sizeof(std::uint64_t);
if (thread_count == thread_ids.size()) {
if (narrow_cast<Bump_Vector_Size>(thread_count) == thread_ids.size()) {
// We can't tell if we read exactly all the threads or if there are more
// threads. Assume there are more threads.
thread_ids.resize(thread_ids.size() * 2);
Expand All @@ -352,42 +357,45 @@ void enumerate_all_process_threads(Callback&& callback) {
#endif

#if defined(__APPLE__)
std::vector<Found_Debug_Server> find_debug_servers() {
Span<Found_Debug_Server> find_debug_servers(Monotonic_Allocator* allocator) {
static constexpr char func[] = "find_debug_servers";

std::vector<Found_Debug_Server> debug_servers;
enumerate_all_process_threads([&](::pid_t process_id,
std::uint64_t thread_id) -> void {
::proc_threadinfo thread_info;
int rc =
::proc_pidinfo(process_id, PROC_PIDTHREADID64INFO,
/*arg=*/thread_id, &thread_info, sizeof(thread_info));
if (rc == -1) {
QLJS_DEBUG_LOG(
"%s: ignoring failure to get name of thread %llu in process %d: %s\n",
func, narrow_cast<unsigned long long>(thread_id), process_id,
std::strerror(errno));
return;
}
QLJS_ASSERT(rc == sizeof(thread_info));

std::string_view thread_name(
thread_info.pth_name,
::strnlen(thread_info.pth_name, MAXTHREADNAMESIZE));
if (std::optional<std::uint16_t> port_number =
parse_port_number_from_thread_name(to_string8_view(thread_name))) {
debug_servers.push_back(Found_Debug_Server{
.process_id = narrow_cast<std::uint64_t>(process_id),
.port_number = *port_number,
Bump_Vector<Found_Debug_Server, Monotonic_Allocator> debug_servers(
"debug_servers", allocator);
enumerate_all_process_threads(
*allocator, [&](::pid_t process_id, std::uint64_t thread_id) -> void {
::proc_threadinfo thread_info;
int rc = ::proc_pidinfo(process_id, PROC_PIDTHREADID64INFO,
/*arg=*/thread_id, &thread_info,
sizeof(thread_info));
if (rc == -1) {
QLJS_DEBUG_LOG(
"%s: ignoring failure to get name of thread %llu in process %d: "
"%s\n",
func, narrow_cast<unsigned long long>(thread_id), process_id,
std::strerror(errno));
return;
}
QLJS_ASSERT(rc == sizeof(thread_info));

std::string_view thread_name(
thread_info.pth_name,
::strnlen(thread_info.pth_name, MAXTHREADNAMESIZE));
if (std::optional<std::uint16_t> port_number =
parse_port_number_from_thread_name(
to_string8_view(thread_name))) {
debug_servers.push_back(Found_Debug_Server{
.process_id = narrow_cast<std::uint64_t>(process_id),
.port_number = *port_number,
});
}
});
}
});
return debug_servers;
return debug_servers.release_to_span();
}
#endif

#if defined(__FreeBSD__)
std::vector<Found_Debug_Server> find_debug_servers() {
Span<Found_Debug_Server> find_debug_servers(Monotonic_Allocator* allocator) {
static constexpr char func[] = "find_debug_servers";

// NOTE(Nico): On FreeBSD we can just fetch the list of all active LWPs by
Expand All @@ -409,7 +417,8 @@ std::vector<Found_Debug_Server> find_debug_servers() {
size_t own_jid_size;
::kinfo_proc* p;
::kvm_t* kd;
std::vector<Found_Debug_Server> debug_servers;
Bump_Vector<Found_Debug_Server, Monotonic_Allocator> debug_servers(
"debug_servers", allocator);

// Query our own jail id
own_jid_size = sizeof own_jid;
Expand Down Expand Up @@ -458,7 +467,7 @@ std::vector<Found_Debug_Server> find_debug_servers() {
error_get_procs:
::kvm_close(kd);
error_open_kvm:
return debug_servers;
return debug_servers.release_to_span();
}
#endif // __FreeBSD__

Expand Down Expand Up @@ -505,10 +514,11 @@ void enumerate_all_process_threads(Callback&& callback) {
#endif

#if defined(_WIN32)
std::vector<Found_Debug_Server> find_debug_servers() {
Span<Found_Debug_Server> find_debug_servers(Monotonic_Allocator* allocator) {
static constexpr char func[] = "find_debug_servers";

std::vector<Found_Debug_Server> debug_servers;
Bump_Vector<Found_Debug_Server, Monotonic_Allocator> debug_servers(
"debug_servers", allocator);
enumerate_all_process_threads([&](::DWORD process_id,
::DWORD thread_id) -> void {
Windows_Handle_File thread_handle(
Expand Down Expand Up @@ -564,14 +574,15 @@ std::vector<Found_Debug_Server> find_debug_servers() {

::LocalFree(thread_name);
});
return debug_servers;
return debug_servers.release_to_span();
}
#endif

#if !QLJS_CAN_FIND_DEBUG_SERVERS
std::vector<Found_Debug_Server> find_debug_servers() {
Span<Found_Debug_Server> find_debug_servers([
[maybe_unused]] Monotonic_Allocator* allocator) {
#warning "--debug-apps is not supported on this platform"
return std::vector<Found_Debug_Server>();
return Span<Found_Debug_Server>();
}
#endif
}
Expand Down
6 changes: 4 additions & 2 deletions src/quick-lint-js/debug/find-debug-server.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
#pragma once

#include <cstdint>
#include <quick-lint-js/container/monotonic-allocator.h>
#include <quick-lint-js/feature.h>
#include <vector>

namespace quick_lint_js {
#if QLJS_FEATURE_DEBUG_SERVER
Expand All @@ -32,8 +32,10 @@ struct Found_Debug_Server {
// This function is inherently racy. By the time this function returns, any
// number of returned debug servers might be dead.
//
// allocator is used to allocate memory for the returned Span.
//
// NOTE[find-debug-server] for implementation details.
std::vector<Found_Debug_Server> find_debug_servers();
Span<Found_Debug_Server> find_debug_servers(Monotonic_Allocator* allocator);
}

// quick-lint-js finds bugs in JavaScript programs.
Expand Down
3 changes: 2 additions & 1 deletion test/test-debug-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,8 @@ TEST_F(Test_Debug_Server, find_debug_servers_finds_running_instance_SLOW) {
ASSERT_TRUE(wait_result.ok()) << wait_result.error_to_string();
std::uint16_t server_port = server->tcp_port_number();

std::vector<Found_Debug_Server> servers = find_debug_servers();
Monotonic_Allocator allocator("Test_Debug_Server");
Span<Found_Debug_Server> servers = find_debug_servers(&allocator);
auto found_server_it =
find_first_if(servers, [&](const Found_Debug_Server &s) -> bool {
return s.port_number == server_port;
Expand Down

0 comments on commit 6c2a407

Please sign in to comment.