Skip to content

Commit

Permalink
Revert "src: modernize cleanup queue to use c++20"
Browse files Browse the repository at this point in the history
This reverts commit 581b444.

PR-URL: nodejs#56846
Refs: nodejs#56063
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
richardlau authored Feb 1, 2025
1 parent 0a4672e commit ad84558
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 25 deletions.
14 changes: 0 additions & 14 deletions src/cleanup_queue-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include <compare>

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

Expand All @@ -31,18 +29,6 @@ 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: 13 additions & 7 deletions src/cleanup_queue.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#include "cleanup_queue.h" // NOLINT(build/include_inline)
#include <algorithm>
#include <ranges>
#include <vector>
#include "cleanup_queue-inl.h"

Expand All @@ -9,20 +8,27 @@ 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 callbacks(cleanup_hooks_.begin(), cleanup_hooks_.end());
std::vector<CleanupHookCallback> 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.

// Sort in descending order so that the most recently inserted callbacks are
// run first.
std::ranges::sort(callbacks, std::greater());
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_;
});

return callbacks;
}

void CleanupQueue::Drain() {
for (const CleanupHookCallback& cb : GetOrdered()) {
if (!cleanup_hooks_.contains(cb)) {
std::vector<CleanupHookCallback> callbacks = GetOrdered();

for (const CleanupHookCallback& cb : callbacks) {
if (cleanup_hooks_.count(cb) == 0) {
// 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: 0 additions & 4 deletions src/cleanup_queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include <compare>
#include <cstddef>
#include <cstdint>
#include <unordered_set>
Expand Down Expand Up @@ -42,9 +41,6 @@ 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 ad84558

Please sign in to comment.