src: better encapsulate native immediate list

Refactor for clarity and reusability. Make it more obvious that the list is a FIFO queue. PR-URL: #31386 Refs: openjs-foundation/summit#240 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>
nodejs · addaleax · Sep 28, 2019 · Sep 28, 2019 · Nov 12, 2019 · Nov 12, 2019
commit 973f8ef12ea2a7355611ffdaab6d117c23c8ee07
diff --git a/src/env-inl.h b/src/env-inl.h
@@ -732,18 +732,45 @@ inline void IsolateData::set_options(
   options_ = std::move(options);
 }
 
-template <typename Fn>
-void Environment::CreateImmediate(Fn&& cb, bool ref) {
-  auto callback = std::make_unique<NativeImmediateCallbackImpl<Fn>>(
-      std::move(cb), ref);
-  NativeImmediateCallback* prev_tail = native_immediate_callbacks_tail_;
+std::unique_ptr<Environment::NativeImmediateCallback>
+Environment::NativeImmediateQueue::Shift() {
+  std::unique_ptr<Environment::NativeImmediateCallback> ret = std::move(head_);
+  if (ret) {
+    head_ = ret->get_next();
+    if (!head_)
+      tail_ = nullptr;  // The queue is now empty.
+  }
+  return ret;
+}
+
+void Environment::NativeImmediateQueue::Push(
+    std::unique_ptr<Environment::NativeImmediateCallback> cb) {
+  NativeImmediateCallback* prev_tail = tail_;
 
-  native_immediate_callbacks_tail_ = callback.get();
+  tail_ = cb.get();
   if (prev_tail != nullptr)
-    prev_tail->set_next(std::move(callback));
+    prev_tail->set_next(std::move(cb));
   else
-    native_immediate_callbacks_head_ = std::move(callback);
+    head_ = std::move(cb);
+}
+
+void Environment::NativeImmediateQueue::ConcatMove(
+    NativeImmediateQueue&& other) {
+  size_ += other.size_;
+  if (tail_ != nullptr)
+    tail_->set_next(std::move(other.head_));
+  else
+    head_ = std::move(other.head_);
+  tail_ = other.tail_;
+  other.tail_ = nullptr;
+  other.size_ = 0;
+}
 
+template <typename Fn>
+void Environment::CreateImmediate(Fn&& cb, bool ref) {
+  auto callback = std::make_unique<NativeImmediateCallbackImpl<Fn>>(
+      std::move(cb), ref);
+  native_immediates_.Push(std::move(callback));
   immediate_info()->count_inc(1);
 }
 

diff --git a/src/env.cc b/src/env.cc
@@ -665,14 +665,14 @@ void Environment::RunAndClearNativeImmediates(bool only_refed) {
                               "RunAndClearNativeImmediates", this);
   size_t ref_count = 0;
   size_t count = 0;
-  std::unique_ptr<NativeImmediateCallback> head;
-  head.swap(native_immediate_callbacks_head_);
-  native_immediate_callbacks_tail_ = nullptr;
+
+  NativeImmediateQueue queue;
+  queue.ConcatMove(std::move(native_immediates_));
 
   auto drain_list = [&]() {
     TryCatchScope try_catch(this);
-    for (; head; head = head->get_next()) {
-      DebugSealHandleScope seal_handle_scope(isolate());
+    DebugSealHandleScope seal_handle_scope(isolate());
+    while (std::unique_ptr<NativeImmediateCallback> head = queue.Shift()) {
       count++;
       if (head->is_refed())
         ref_count++;
@@ -684,15 +684,12 @@ void Environment::RunAndClearNativeImmediates(bool only_refed) {
         if (!try_catch.HasTerminated() && can_call_into_js())
           errors::TriggerUncaughtException(isolate(), try_catch);
 
-        // We are done with the current callback. Move one iteration along,
-        // as if we had completed successfully.
-        head = head->get_next();
         return true;
       }
     }
     return false;
   };
-  while (head && drain_list()) {}
+  while (queue.size() > 0 && drain_list()) {}
 
   DCHECK_GE(immediate_info()->count(), count);
   immediate_info()->count_dec(count);

diff --git a/src/env.h b/src/env.h
@@ -1425,8 +1425,20 @@ class Environment : public MemoryRetainer {
     Fn callback_;
   };
 
-  std::unique_ptr<NativeImmediateCallback> native_immediate_callbacks_head_;
-  NativeImmediateCallback* native_immediate_callbacks_tail_ = nullptr;
+  class NativeImmediateQueue {
+   public:
+    inline std::unique_ptr<NativeImmediateCallback> Shift();
+    inline void Push(std::unique_ptr<NativeImmediateCallback> cb);
+    // ConcatMove adds elements from 'other' to the end of this list, and clears
+    // 'other' afterwards.
+    inline void ConcatMove(NativeImmediateQueue&& other);
+
+   private:
+    std::unique_ptr<NativeImmediateCallback> head_;
+    NativeImmediateCallback* tail_ = nullptr;
+  };
+
+  NativeImmediateQueue native_immediates_;
 
   void RunAndClearNativeImmediates(bool only_refed = false);
   static void CheckImmediate(uv_check_t* handle);