Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 572b82f

Browse filesBrowse files
joyeecheungruyadorno
authored andcommitted
src: make BaseObject iteration order deterministic
Previously we just rely on the unordered_set order to iterate over the BaseObjects, which is not deterministic. The iteration is only used in printing, verification, and snapshot generation. In the first two cases the performance overhead of sorting does not matter because they are only used for debugging. In the last case the determinism is more important than the trivial overhead of sorting. So this patch makes the iteration deterministic by sorting the set first, as what is already being done when we drain the queue. PR-URL: #48702 Refs: nodejs/build#3043 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent 9cb18b3 commit 572b82f
Copy full SHA for 572b82f

File tree

Expand file treeCollapse file tree

3 files changed

+13
-2
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+13
-2
lines changed
Open diff view settings
Collapse file

‎src/cleanup_queue-inl.h‎

Copy file name to clipboardExpand all lines: src/cleanup_queue-inl.h
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ void CleanupQueue::Remove(Callback cb, void* arg) {
3939

4040
template <typename T>
4141
void CleanupQueue::ForEachBaseObject(T&& iterator) const {
42-
for (const auto& hook : cleanup_hooks_) {
42+
std::vector<CleanupHookCallback> callbacks = GetOrdered();
43+
44+
for (const auto& hook : callbacks) {
4345
BaseObject* obj = GetBaseObject(hook);
4446
if (obj != nullptr) iterator(obj);
4547
}
Collapse file

‎src/cleanup_queue.cc‎

Copy file name to clipboardExpand all lines: src/cleanup_queue.cc
+8-1Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55

66
namespace node {
77

8-
void CleanupQueue::Drain() {
8+
std::vector<CleanupQueue::CleanupHookCallback> CleanupQueue::GetOrdered()
9+
const {
910
// Copy into a vector, since we can't sort an unordered_set in-place.
1011
std::vector<CleanupHookCallback> callbacks(cleanup_hooks_.begin(),
1112
cleanup_hooks_.end());
@@ -20,6 +21,12 @@ void CleanupQueue::Drain() {
2021
return a.insertion_order_counter_ > b.insertion_order_counter_;
2122
});
2223

24+
return callbacks;
25+
}
26+
27+
void CleanupQueue::Drain() {
28+
std::vector<CleanupHookCallback> callbacks = GetOrdered();
29+
2330
for (const CleanupHookCallback& cb : callbacks) {
2431
if (cleanup_hooks_.count(cb) == 0) {
2532
// This hook was removed from the `cleanup_hooks_` set during another
Collapse file

‎src/cleanup_queue.h‎

Copy file name to clipboardExpand all lines: src/cleanup_queue.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <cstddef>
77
#include <cstdint>
88
#include <unordered_set>
9+
#include <vector>
910

1011
#include "memory_tracker.h"
1112

@@ -66,6 +67,7 @@ class CleanupQueue : public MemoryRetainer {
6667
uint64_t insertion_order_counter_;
6768
};
6869

70+
std::vector<CleanupHookCallback> GetOrdered() const;
6971
inline BaseObject* GetBaseObject(const CleanupHookCallback& callback) const;
7072

7173
// Use an unordered_set, so that we have efficient insertion and removal.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.