src: fixup for embedding - reduce dependecy

* move `arraysize` to util.h to reduce the need for `node_intrnals.h` * make sure options_parser singletons init in order * deoptimize NodeTraceWriter
nodejs · refack · Oct 12, 2018 · Oct 12, 2018 · Oct 13, 2018 · Oct 13, 2018
commit 0a17e279fb4165731188ec9fd3999303681a9795
diff --git a/src/env-inl.h b/src/env-inl.h
@@ -330,10 +330,6 @@ inline bool Environment::profiler_idle_notifier_started() const {
   return profiler_idle_notifier_started_;
 }
 
-inline v8::Isolate* Environment::isolate() const {
-  return isolate_;
-}
-
 inline Environment* Environment::from_timer_handle(uv_timer_t* handle) {
   return ContainerOf(&Environment::timer_handle_, handle);
 }
@@ -893,16 +889,6 @@ void Environment::ForEachBaseObject(T&& iterator) {
 #undef VY
 #undef VP
 
-#define V(PropertyName, TypeName)                                             \
-  inline v8::Local<TypeName> Environment::PropertyName() const {              \
-    return StrongPersistentToLocal(PropertyName ## _);                        \
-  }                                                                           \
-  inline void Environment::set_ ## PropertyName(v8::Local<TypeName> value) {  \
-    PropertyName ## _.Reset(isolate(), value);                                \
-  }
-  ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V)
-#undef V
-
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

diff --git a/src/env.cc b/src/env.cc
@@ -876,4 +876,14 @@ bool BaseObject::IsRootNode() const {
   return !persistent_handle_.IsWeak();
 }
 
+#define V1(PropertyName, TypeName)                                            \
+  v8::Local<TypeName> Environment::PropertyName() const {                     \
+    return StrongPersistentToLocal(PropertyName ## _);                        \
+  }                                                                           \
+  void Environment::set_ ## PropertyName(v8::Local<TypeName> value) {         \
+    PropertyName ## _.Reset(isolate(), value);                                \
+  }
+  ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V1)
+#undef V1
+
 }  // namespace node
diff --git a/src/env.h b/src/env.h
@@ -629,7 +629,9 @@ class Environment {
   void StopProfilerIdleNotifier();
   inline bool profiler_idle_notifier_started() const;
 
-  inline v8::Isolate* isolate() const;
+  v8::Isolate* isolate() const {
+    return isolate_;
+  }
   inline uv_loop_t* event_loop() const;
   inline uint32_t watched_providers() const;
 
@@ -813,8 +815,8 @@ class Environment {
 #undef VP
 
 #define V(PropertyName, TypeName)                                             \
-  inline v8::Local<TypeName> PropertyName() const;                            \
-  inline void set_ ## PropertyName(v8::Local<TypeName> value);
+  v8::Local<TypeName> PropertyName() const;                                   \
+  void set_ ## PropertyName(v8::Local<TypeName> value);
   ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V)
 #undef V
 

diff --git a/src/node_code_cache.h b/src/node_code_cache.h
@@ -3,7 +3,8 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-#include "node_internals.h"
+#include "v8.h"
+#include "env.h"
 
 namespace node {
 

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -33,6 +33,8 @@
 #include "v8.h"
 #include "tracing/trace_event.h"
 #include "node_api.h"
+#include "util.h"
+
 
 #include <stdint.h>
 #include <stdlib.h>
@@ -182,14 +184,6 @@ extern std::shared_ptr<PerProcessOptions> per_process_opts;
 // Forward declaration
 class Environment;
 
-// If persistent.IsWeak() == false, then do not call persistent.Reset()
-// while the returned Local<T> is still in scope, it will destroy the
-// reference to the object.
-template <class TypeName>
-inline v8::Local<TypeName> PersistentToLocal(
-    v8::Isolate* isolate,
-    const Persistent<TypeName>& persistent);
-
 // Convert a struct sockaddr to a { address: '1.2.3.4', port: 1234 } JS object.
 // Sets address and port properties on the info object and returns it.
 // If |info| is omitted, a new object is returned.
@@ -231,9 +225,6 @@ bool SafeGetenv(const char* key, std::string* text);
 std::string GetHumanReadableProcessName();
 void GetHumanReadableProcessName(char (*name)[1024]);
 
-template <typename T, size_t N>
-constexpr size_t arraysize(const T(&)[N]) { return N; }
-
 #ifndef ROUND_UP
 # define ROUND_UP(a, b) ((a) % (b) ? ((a) + (b)) - ((a) % (b)) : (a))
 #endif

diff --git a/src/node_javascript.h b/src/node_javascript.h
@@ -24,7 +24,8 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-#include "node_internals.h"
+#include "v8.h"
+#include "env.h"
 
 namespace node {
 

diff --git a/src/node_options.cc b/src/node_options.cc
@@ -44,6 +44,13 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
 
 namespace options_parser {
 
+// Make sure dynamic instantiation of singletons is done in the right order.
+DebugOptionsParser DebugOptionsParser::instance = {};
+EnvironmentOptionsParser EnvironmentOptionsParser::instance = {};
+PerIsolateOptionsParser PerIsolateOptionsParser::instance = {};
+PerProcessOptionsParser PerProcessOptionsParser::instance = {};
+
+
 // XXX: If you add an option here, please also add it to doc/node.1 and
 // doc/api/cli.md
 // TODO(addaleax): Make that unnecessary.
@@ -82,8 +89,6 @@ DebugOptionsParser::DebugOptionsParser() {
 #endif
 }
 
-DebugOptionsParser DebugOptionsParser::instance;
-
 EnvironmentOptionsParser::EnvironmentOptionsParser() {
   AddOption("--experimental-modules",
             "experimental ES Module support and caching modules",
@@ -193,7 +198,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
          &EnvironmentOptions::get_debug_options);
 }
 
-EnvironmentOptionsParser EnvironmentOptionsParser::instance;
 
 PerIsolateOptionsParser::PerIsolateOptionsParser() {
   AddOption("--track-heap-objects",
@@ -216,8 +220,6 @@ PerIsolateOptionsParser::PerIsolateOptionsParser() {
          &PerIsolateOptions::get_per_env_options);
 }
 
-PerIsolateOptionsParser PerIsolateOptionsParser::instance;
-
 PerProcessOptionsParser::PerProcessOptionsParser() {
   AddOption("--title",
             "the process title to use on startup",
@@ -320,7 +322,6 @@ PerProcessOptionsParser::PerProcessOptionsParser() {
          &PerProcessOptions::get_per_isolate_options);
 }
 
-PerProcessOptionsParser PerProcessOptionsParser::instance;
 
 inline std::string RemoveBrackets(const std::string& host) {
   if (!host.empty() && host.front() == '[' && host.back() == ']')

diff --git a/src/node_postmortem_metadata.cc b/src/node_postmortem_metadata.cc
@@ -1,7 +1,6 @@
 #include "env.h"
-#include "base_object-inl.h"
 #include "handle_wrap.h"
-#include "util-inl.h"
+#include "util.h"
 #include "req_wrap.h"
 #include "v8abbr.h"
 #include "node_context_data.h"

diff --git a/src/tracing/agent.cc b/src/tracing/agent.cc
@@ -4,7 +4,6 @@
 #include "trace_event.h"
 #include "tracing/node_trace_buffer.h"
 #include "debug_utils.h"
-#include "env-inl.h"
 
 namespace node {
 namespace tracing {

diff --git a/src/tracing/node_trace_writer.h b/src/tracing/node_trace_writer.h
@@ -11,6 +11,8 @@
 namespace node {
 namespace tracing {
 
+constexpr size_t kTracesPerFile = 1 << 19;
+
 using v8::platform::tracing::TraceObject;
 using v8::platform::tracing::TraceWriter;
 
@@ -23,7 +25,6 @@ class NodeTraceWriter : public AsyncTraceWriter {
   void AppendTraceEvent(TraceObject* trace_event) override;
   void Flush(bool blocking) override;
 
-  static const int kTracesPerFile = 1 << 19;
 
  private:
   struct WriteRequest {
@@ -61,7 +62,7 @@ class NodeTraceWriter : public AsyncTraceWriter {
   int highest_request_id_completed_ = 0;
   int total_traces_ = 0;
   int file_num_ = 0;
-  const std::string& log_file_pattern_;
+  std::string log_file_pattern_;
   std::ostringstream stream_;
   std::unique_ptr<TraceWriter> json_trace_writer_;
   bool exited_ = false;

diff --git a/src/util-inl.h b/src/util-inl.h
@@ -141,82 +141,6 @@ typename ListHead<T, M>::Iterator ListHead<T, M>::end() const {
   return Iterator(const_cast<ListNode<T>*>(&head_));
 }
 
-template <typename Inner, typename Outer>
-constexpr uintptr_t OffsetOf(Inner Outer::*field) {
-  return reinterpret_cast<uintptr_t>(&(static_cast<Outer*>(0)->*field));
-}
-
-template <typename Inner, typename Outer>
-ContainerOfHelper<Inner, Outer>::ContainerOfHelper(Inner Outer::*field,
-                                                   Inner* pointer)
-    : pointer_(
-        reinterpret_cast<Outer*>(
-            reinterpret_cast<uintptr_t>(pointer) - OffsetOf(field))) {}
-
-template <typename Inner, typename Outer>
-template <typename TypeName>
-ContainerOfHelper<Inner, Outer>::operator TypeName*() const {
-  return static_cast<TypeName*>(pointer_);
-}
-
-template <typename Inner, typename Outer>
-constexpr ContainerOfHelper<Inner, Outer> ContainerOf(Inner Outer::*field,
-                                                      Inner* pointer) {
-  return ContainerOfHelper<Inner, Outer>(field, pointer);
-}
-
-template <class TypeName>
-inline v8::Local<TypeName> PersistentToLocal(
-    v8::Isolate* isolate,
-    const Persistent<TypeName>& persistent) {
-  if (persistent.IsWeak()) {
-    return WeakPersistentToLocal(isolate, persistent);
-  } else {
-    return StrongPersistentToLocal(persistent);
-  }
-}
-
-template <class TypeName>
-inline v8::Local<TypeName> StrongPersistentToLocal(
-    const Persistent<TypeName>& persistent) {
-  return *reinterpret_cast<v8::Local<TypeName>*>(
-      const_cast<Persistent<TypeName>*>(&persistent));
-}
-
-template <class TypeName>
-inline v8::Local<TypeName> WeakPersistentToLocal(
-    v8::Isolate* isolate,
-    const Persistent<TypeName>& persistent) {
-  return v8::Local<TypeName>::New(isolate, persistent);
-}
-
-inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
-                                           const char* data,
-                                           int length) {
-  return v8::String::NewFromOneByte(isolate,
-                                    reinterpret_cast<const uint8_t*>(data),
-                                    v8::NewStringType::kNormal,
-                                    length).ToLocalChecked();
-}
-
-inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
-                                           const signed char* data,
-                                           int length) {
-  return v8::String::NewFromOneByte(isolate,
-                                    reinterpret_cast<const uint8_t*>(data),
-                                    v8::NewStringType::kNormal,
-                                    length).ToLocalChecked();
-}
-
-inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
-                                           const unsigned char* data,
-                                           int length) {
-  return v8::String::NewFromOneByte(isolate,
-                                    reinterpret_cast<const uint8_t*>(data),
-                                    v8::NewStringType::kNormal,
-                                    length).ToLocalChecked();
-}
-
 void SwapBytes16(char* data, size_t nbytes) {
   CHECK_EQ(nbytes % 2, 0);
 

diff --git a/src/util.cc b/src/util.cc
@@ -34,6 +34,32 @@ using v8::Local;
 using v8::String;
 using v8::Value;
 
+Local<String> OneByteString(Isolate* isolate, const char* data, int length) {
+  return String::NewFromOneByte(isolate,
+                                reinterpret_cast<const uint8_t*>(data),
+                                v8::NewStringType::kNormal,
+                                length).ToLocalChecked();
+}
+
+Local<String> OneByteString(Isolate* isolate,
+                            const signed char* data,
+                            int length) {
+  return String::NewFromOneByte(isolate,
+                                reinterpret_cast<const uint8_t*>(data),
+                                v8::NewStringType::kNormal,
+                                length).ToLocalChecked();
+}
+
+Local<String> OneByteString(Isolate* isolate,
+                            const unsigned char* data,
+                            int length) {
+  return String::NewFromOneByte(isolate,
+                                reinterpret_cast<const uint8_t*>(data),
+                                v8::NewStringType::kNormal,
+                                length).ToLocalChecked();
+}
+
+
 template <typename T>
 static void MakeUtf8String(Isolate* isolate,
                            Local<Value> value,