Skip to content

Commit

Permalink
src: modernize cleanup queue to use c++20
Browse files Browse the repository at this point in the history
  • Loading branch information
anonrig committed Nov 30, 2024
1 parent 4cf6fab commit f416d40
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 13 deletions.
14 changes: 14 additions & 0 deletions src/cleanup_queue-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include <compare>

#include "cleanup_queue.h"
#include "util.h"

Expand All @@ -29,6 +31,18 @@ void CleanupQueue::Remove(Callback cb, void* arg) {
cleanup_hooks_.erase(search);
}

constexpr std::strong_ordering CleanupQueue::CleanupHookCallback::operator<=>(
const CleanupHookCallback& other) const noexcept {
if (insertion_order_counter_ > other.insertion_order_counter_) {
return std::strong_ordering::greater;
}

if (insertion_order_counter_ < other.insertion_order_counter_) {
return std::strong_ordering::less;
}
return std::strong_ordering::equivalent;
}

} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down
20 changes: 7 additions & 13 deletions src/cleanup_queue.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "cleanup_queue.h" // NOLINT(build/include_inline)
#include <algorithm>
#include <ranges>>
#include <vector>
#include "cleanup_queue-inl.h"

Expand All @@ -8,27 +9,20 @@ namespace node {
std::vector<CleanupQueue::CleanupHookCallback> CleanupQueue::GetOrdered()
const {
// Copy into a vector, since we can't sort an unordered_set in-place.
std::vector<CleanupHookCallback> callbacks(cleanup_hooks_.begin(),
cleanup_hooks_.end());
std::vector callbacks(cleanup_hooks_.begin(), cleanup_hooks_.end());
// We can't erase the copied elements from `cleanup_hooks_` yet, because we
// need to be able to check whether they were un-scheduled by another hook.

std::sort(callbacks.begin(),
callbacks.end(),
[](const CleanupHookCallback& a, const CleanupHookCallback& b) {
// Sort in descending order so that the most recently inserted
// callbacks are run first.
return a.insertion_order_counter_ > b.insertion_order_counter_;
});
// Sort in descending order so that the most recently inserted callbacks are
// run first.
std::ranges::sort(callbacks, std::greater());

return callbacks;
}

void CleanupQueue::Drain() {
std::vector<CleanupHookCallback> callbacks = GetOrdered();

for (const CleanupHookCallback& cb : callbacks) {
if (cleanup_hooks_.count(cb) == 0) {
for (const CleanupHookCallback& cb : GetOrdered()) {
if (!cleanup_hooks_.contains(cb)) {
// This hook was removed from the `cleanup_hooks_` set during another
// hook that was run earlier. Nothing to do here.
continue;
Expand Down
4 changes: 4 additions & 0 deletions src/cleanup_queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include <compare>
#include <cstddef>
#include <cstdint>
#include <unordered_set>
Expand Down Expand Up @@ -41,6 +42,9 @@ class CleanupQueue : public MemoryRetainer {
arg_(arg),
insertion_order_counter_(insertion_order_counter) {}

constexpr std::strong_ordering operator<=>(
const CleanupHookCallback& other) const noexcept;

// Only hashes `arg_`, since that is usually enough to identify the hook.
struct Hash {
size_t operator()(const CleanupHookCallback& cb) const;
Expand Down

0 comments on commit f416d40

Please sign in to comment.