fixup! src: use STL containers instead of v8 values for static module…

… data
nodejs · joyeecheung · Nov 14, 2018 · Nov 16, 2018 · Nov 19, 2018 · 161c9f80f261b5e2260f2737036247484bb11807
commit 161c9f80f261b5e2260f2737036247484bb11807
diff --git a/lib/internal/bootstrap/cache.js b/lib/internal/bootstrap/cache.js
@@ -9,11 +9,12 @@ const {
   NativeModule
 } = require('internal/bootstrap/loaders');
 const {
-  source,
+  getSource,
   compileCodeCache
 } = internalBinding('native_module');
 const { hasTracing } = process.binding('config');
 
+const source = getSource();
 const depsModule = Object.keys(source).filter(
   (key) => NativeModule.isDepsModule(key) || key.startsWith('internal/deps')
 );

diff --git a/src/node.cc b/src/node.cc
@@ -1781,7 +1781,7 @@ void LoadEnvironment(Environment* env) {
   // generated in node_javascript.cc by node_js2c.
 
   // TODO(joyeecheung): use NativeModuleLoader::Compile
-  // We duplicate the string literals here since once we refactor the boostrap
+  // We duplicate the string literals here since once we refactor the bootstrap
   // compilation out to NativeModuleLoader none of this is going to matter
   Isolate* isolate = env->isolate();
   Local<String> loaders_name =

diff --git a/src/node_native_module.cc b/src/node_native_module.cc
@@ -19,9 +19,7 @@ using v8::Isolate;
 using v8::Local;
 using v8::Maybe;
 using v8::MaybeLocal;
-using v8::Name;
 using v8::Object;
-using v8::PropertyCallbackInfo;
 using v8::Script;
 using v8::ScriptCompiler;
 using v8::ScriptOrigin;
@@ -30,6 +28,7 @@ using v8::String;
 using v8::Uint8Array;
 using v8::Value;
 
+// TODO(joyeecheung): make these more general and put them into util.h
 Local<Object> MapToObject(Local<Context> context,
                           const NativeModuleRecordMap& in) {
   Isolate* isolate = context->GetIsolate();
@@ -41,7 +40,8 @@ Local<Object> MapToObject(Local<Context> context,
   return out;
 }
 
-Local<Set> ToJsSet(Local<Context> context, const std::set<std::string>& in) {
+Local<Set> ToJsSet(Local<Context> context,
+                   const std::set<std::string>& in) {
   Isolate* isolate = context->GetIsolate();
   Local<Set> out = Set::New(isolate);
   for (auto const& x : in) {
@@ -52,8 +52,8 @@ Local<Set> ToJsSet(Local<Context> context, const std::set<std::string>& in) {
 }
 
 void NativeModuleLoader::GetCacheUsage(
-    Local<Name> property, const PropertyCallbackInfo<Value>& info) {
-  Environment* env = Environment::GetCurrent(info);
+    const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();
   Local<Context> context = env->context();
   Local<Object> result = Object::New(isolate);
@@ -67,13 +67,13 @@ void NativeModuleLoader::GetCacheUsage(
             OneByteString(isolate, "compiledWithoutCache"),
             ToJsSet(context, env->native_modules_without_cache))
       .FromJust();
-  info.GetReturnValue().Set(result);
+  args.GetReturnValue().Set(result);
 }
 
 void NativeModuleLoader::GetSourceObject(
-    Local<Name> property, const PropertyCallbackInfo<Value>& info) {
-  Environment* env = Environment::GetCurrent(info);
-  info.GetReturnValue().Set(per_process_loader.GetSourceObject(env->context()));
+    const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  args.GetReturnValue().Set(per_process_loader.GetSourceObject(env->context()));
 }
 
 Local<Object> NativeModuleLoader::GetSourceObject(
@@ -306,22 +306,11 @@ void NativeModuleLoader::Initialize(Local<Object> target,
                                     Local<Value> unused,
                                     Local<Context> context) {
   Environment* env = Environment::GetCurrent(context);
-  Isolate* isolate = env->isolate();
 
-  CHECK(target
-            ->SetAccessor(env->context(),
-                          FIXED_ONE_BYTE_STRING(isolate, "source"),
-                          GetSourceObject,
-                          nullptr,
-                          env->as_external())
-            .FromJust());
-  CHECK(target
-            ->SetAccessor(env->context(),
-                          FIXED_ONE_BYTE_STRING(isolate, "cacheUsage"),
-                          GetCacheUsage,
-                          nullptr,
-                          env->as_external())
-            .FromJust());
+  env->SetMethod(
+      target, "getSource", NativeModuleLoader::GetSourceObject);
+  env->SetMethod(
+      target, "getCacheUsage", NativeModuleLoader::GetCacheUsage);
   env->SetMethod(
       target, "compileFunction", NativeModuleLoader::CompileFunction);
   env->SetMethod(

diff --git a/src/node_native_module.h b/src/node_native_module.h
@@ -13,8 +13,8 @@
 namespace node {
 namespace native_module {
 
-typedef typename std::map<std::string, UnionBytes> NativeModuleRecordMap;
-typedef typename std::map<std::string, std::string> NativeModuleHashMap;
+using NativeModuleRecordMap = std::map<std::string, UnionBytes>;
+using NativeModuleHashMap = std::map<std::string, std::string>;
 
 // The native (C++) side of the native module compilation.
 // This class should not depend on Environment
@@ -28,25 +28,23 @@ class NativeModuleLoader {
   v8::Local<v8::String> GetSource(v8::Isolate* isolate, const char* id) const;
 
  private:
-  static void GetCacheUsage(v8::Local<v8::Name> property,
-                            const v8::PropertyCallbackInfo<v8::Value>& info);
+  static void GetCacheUsage(const v8::FunctionCallbackInfo<v8::Value>& args);
   // For legacy process.binding('natives') which is mutable, and for
   // internalBinding('native_module').source for internal use
-  static void GetSourceObject(v8::Local<v8::Name> property,
-                              const v8::PropertyCallbackInfo<v8::Value>& info);
+  static void GetSourceObject(const v8::FunctionCallbackInfo<v8::Value>& args);
   // Compile code cache for a specific native module
   static void CompileCodeCache(const v8::FunctionCallbackInfo<v8::Value>& args);
   // Compile a specific native module as a function
   static void CompileFunction(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   // Generated by tools/js2c.py as node_javascript.cc
   void LoadJavaScriptSource();  // Loads data into source_
-  void LoadJavaScriptHash();  // Loads data into source_hash_
+  void LoadJavaScriptHash();    // Loads data into source_hash_
 
   // Generated by tools/generate_code_cache.js as node_code_cache.cc when
   // the build is configured with --code-cache-path=.... They are noops
   // in node_code_cache_stub.cc
-  void LoadCodeCache();  // Loads data into code_cache_
+  void LoadCodeCache();      // Loads data into code_cache_
   void LoadCodeCacheHash();  // Loads data into code_cache_hash_
 
   v8::ScriptCompiler::CachedData* GetCachedData(const char* id) const;

diff --git a/test/code-cache/test-code-cache-generator.js b/test/code-cache/test-code-cache-generator.js
@@ -30,10 +30,10 @@ if (child.status !== 0) {
 }
 
 // Verifies that:
-// - node::DefineCodeCache()
-// - node::DefineCodeCacheHash()
+// - node::LoadCodeCache()
+// - node::LoadCodeCacheHash()
 // are defined in the generated code.
-// See src/node_code_cache_stub.cc for explanations.
+// See src/node_native_module.h for explanations.
 
 const rl = readline.createInterface({
   input: fs.createReadStream(dest),
@@ -44,10 +44,10 @@ let hasCacheDef = false;
 let hasHashDef = false;
 
 rl.on('line', common.mustCallAtLeast((line) => {
-  if (line.includes('DefineCodeCache(')) {
+  if (line.includes('LoadCodeCache(')) {
     hasCacheDef = true;
   }
-  if (line.includes('DefineCodeCacheHash(')) {
+  if (line.includes('LoadCodeCacheHash(')) {
     hasHashDef = true;
   }
 }, 2));

diff --git a/test/code-cache/test-code-cache.js b/test/code-cache/test-code-cache.js
@@ -18,6 +18,9 @@ const {
 const {
   internalBinding
 } = require('internal/test/binding');
+const {
+  getCacheUsage
+} = internalBinding('native_module');
 
 for (const key of cachableBuiltins) {
   if (!isMainThread && key === 'trace_events') {
@@ -28,11 +31,9 @@ for (const key of cachableBuiltins) {
 
 // The computation has to be delayed until we have done loading modules
 const {
-  cacheUsage: {
-    compiledWithoutCache,
-    compiledWithCache
-  }
-} = internalBinding('native_module');
+  compiledWithoutCache,
+  compiledWithCache
+} = getCacheUsage();
 
 const loadedModules = process.moduleLoadList
   .filter((m) => m.startsWith('NativeModule'))

diff --git a/tools/generate_code_cache.js b/tools/generate_code_cache.js
@@ -13,6 +13,12 @@ const {
   cachableBuiltins
 } = require('internal/bootstrap/cache');
 
+const {
+  types: {
+    isUint8Array
+  }
+} = require('util');
+
 function hash(str) {
   if (process.versions.openssl) {
     return require('crypto').createHash('sha256').update(str).digest('hex');
@@ -61,17 +67,16 @@ function getInitalizer(key, cache) {
                      `${cache.join(',')}\n};`;
   const source = getSource(key);
   const sourceHash = hash(source);
-  const bytes = `UnionBytes(${defName}, arraysize(${defName}))`;
   const initializer =
-    'code_cache_.insert(std::make_pair(\n' +
-    `  std::string("${key}"),\n` +
-    `  ${bytes}\n` +
-    '));';
+    'code_cache_.emplace(\n' +
+    `  "${key}",\n` +
+    `  UnionBytes(${defName}, arraysize(${defName}))\n` +
+    ');';
   const hashIntializer =
-    'code_cache_hash_.insert(std::make_pair(\n' +
-    `  std::string("${key}"),\n` +
-    `  std::string("${sourceHash}")\n` +
-    '));';
+    'code_cache_hash_.emplace(\n' +
+    `  "${key}",\n` +
+    `  "${sourceHash}"\n` +
+    ');';
   return {
     definition, initializer, hashIntializer, sourceHash
   };
@@ -94,20 +99,20 @@ function lexical(a, b) {
 
 for (const key of cachableBuiltins.sort(lexical)) {
   const cachedData = getCodeCache(key);
-  if (!(cachedData instanceof Uint8Array)) {
+  if (!isUint8Array(cachedData)) {
     console.error(`Failed to generate code cache for '${key}'`);
     process.exit(1);
   }
 
-  const length = cachedData.byteLength;
-  totalCacheSize += length;
+  const size = cachedData.byteLength;
+  totalCacheSize += size;
   const {
     definition, initializer, hashIntializer, sourceHash
   } = getInitalizer(key, cachedData);
   cacheDefinitions.push(definition);
   cacheInitializers.push(initializer);
   cacheHashInitializers.push(hashIntializer);
-  console.log(`Generated cache for '${key}', size = ${formatSize(length)}` +
+  console.log(`Generated cache for '${key}', size = ${formatSize(size)}` +
               `, hash = ${sourceHash}, total = ${formatSize(totalCacheSize)}`);
 }
 

diff --git a/tools/js2c.py b/tools/js2c.py
@@ -203,17 +203,17 @@ def ReadMacros(lines):
 
 
 INITIALIZER = """
-source_.insert(std::make_pair(
-    std::string("{module}"),
+source_.emplace(
+    "{module}",
     UnionBytes({var}, arraysize({var}))
-));
+);
 """
 
 HASH_INITIALIZER = """\
-source_hash_.insert(std::make_pair(
-    std::string("{module}"),
-    std::string("{hash_value}")
-));
+source_hash_.emplace(
+    "{module}",
+    "{hash_value}"
+);
 """
 
 DEPRECATED_DEPS = """\