From 2c87a5654f514b5adcc71656a01323c88b3d510c Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 24 Oct 2018 21:15:22 +0800 Subject: [PATCH 1/4] lib: fix code cache generation e7f710c1 broke the code cache generation since internalBinding is now passed in through the wrapper and cannot be redeclared. This patch fixes that. --- lib/internal/bootstrap/cache.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/bootstrap/cache.js b/lib/internal/bootstrap/cache.js index 41fe1e3a914ba0..734c6d10c2d3b3 100644 --- a/lib/internal/bootstrap/cache.js +++ b/lib/internal/bootstrap/cache.js @@ -6,7 +6,7 @@ // cannot be tampered with even with --expose-internals const { - NativeModule, internalBinding + NativeModule } = require('internal/bootstrap/loaders'); function getCodeCache(id) { From 135472a8194a581fae781910e545f8ceb1aa412e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 24 Oct 2018 23:24:31 +0800 Subject: [PATCH 2/4] test: run code cache test by default and test generator - Add the code cache tests to the default test suite, and test the bookkeeping when the binary is not built with the code cache. - Test the code cache generator to make sure we do not accidentally break it - until we enable code cache in the CI. --- test/code-cache/test-code-cache-generator.js | 58 ++++++++++++++++++++ test/code-cache/test-code-cache.js | 46 ++++++++++------ tools/test.py | 1 - 3 files changed, 88 insertions(+), 17 deletions(-) create mode 100644 test/code-cache/test-code-cache-generator.js diff --git a/test/code-cache/test-code-cache-generator.js b/test/code-cache/test-code-cache-generator.js new file mode 100644 index 00000000000000..972d89629c0a23 --- /dev/null +++ b/test/code-cache/test-code-cache-generator.js @@ -0,0 +1,58 @@ +'use strict'; + +// This test verifies that the binary is compiled with code cache and the +// cache is used when built in modules are compiled. + +const common = require('../common'); + +const tmpdir = require('../common/tmpdir'); +const { spawnSync } = require('child_process'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); +const readline = require('readline'); + +const generator = path.join( + __dirname, '..', '..', 'tools', 'generate_code_cache.js' +); +tmpdir.refresh(); +const dest = path.join(tmpdir.path, 'cache.cc'); + +// Run tools/generate_code_cache.js +const child = spawnSync( + process.execPath, + ['--expose-internals', generator, dest] +); +assert.ifError(child.error); +if (child.status !== 0) { + console.log(child.stderr.toString()); + assert.strictEqual(child.status, 0); +} + +// Verifies that: +// - node::DefineCodeCache() +// - node::DefineCodeCacheHash() +// are defined in the generated code. +// See src/node_code_cache_stub.cc for explanations. + +const rl = readline.createInterface({ + input: fs.createReadStream(dest), + crlfDelay: Infinity +}); + +let hasCacheDef = false; +let hasHashDef = false; + +rl.on('line', common.mustCallAtLeast((line) => { + if (line.includes('DefineCodeCache(')) { + hasCacheDef = true; + } + if (line.includes('DefineCodeCacheHash(')) { + hasHashDef = true; + } +}, 2)); + +rl.on('close', common.mustCall(() => { + assert.ok(hasCacheDef); + assert.ok(hasHashDef); +})); diff --git a/test/code-cache/test-code-cache.js b/test/code-cache/test-code-cache.js index b05e764e8ad290..c1bfa80f4342f3 100644 --- a/test/code-cache/test-code-cache.js +++ b/test/code-cache/test-code-cache.js @@ -1,8 +1,9 @@ 'use strict'; // Flags: --expose-internals -// This test verifies that the binary is compiled with code cache and the -// cache is used when built in modules are compiled. +// This test verifies that if the binary is compiled with code cache, +// and the cache is used when built in modules are compiled. +// Otherwise, verifies that no cache is used when compiling builtins. require('../common'); const assert = require('assert'); @@ -18,23 +19,36 @@ const { compiledWithoutCache } = require('internal/bootstrap/cache'); -assert.strictEqual( - typeof process.config.variables.node_code_cache_path, - 'string' -); - -assert.deepStrictEqual(compiledWithoutCache, []); - const loadedModules = process.moduleLoadList .filter((m) => m.startsWith('NativeModule')) .map((m) => m.replace('NativeModule ', '')); -for (const key of loadedModules) { - assert(compiledWithCache.includes(key), - `"${key}" should've been compiled with code cache`); -} +// The binary is not configured with code cache, verifies that the builtins +// are all compiled without cache and we are doing the bookkeeping right. +if (process.config.variables.node_code_cache_path === undefined) { + assert.deepStrictEqual(compiledWithCache, []); + assert.notStrictEqual(compiledWithoutCache.length, 0); + + for (const key of loadedModules) { + assert(compiledWithoutCache.includes(key), + `"${key}" should not have been compiled with code cache`); + } -for (const key of cachableBuiltins) { - assert(isUint8Array(codeCache[key]) && codeCache[key].length > 0, - `Code cache for "${key}" should've been generated`); +} else { + // The binary is configured with code cache. + assert.strictEqual( + typeof process.config.variables.node_code_cache_path, + 'string' + ); + assert.deepStrictEqual(compiledWithoutCache, []); + + for (const key of loadedModules) { + assert(compiledWithCache.includes(key), + `"${key}" should've been compiled with code cache`); + } + + for (const key of cachableBuiltins) { + assert(isUint8Array(codeCache[key]) && codeCache[key].length > 0, + `Code cache for "${key}" should've been generated`); + } } diff --git a/tools/test.py b/tools/test.py index 3d0ba43beca8b5..cd361196653043 100755 --- a/tools/test.py +++ b/tools/test.py @@ -1498,7 +1498,6 @@ def PrintCrashed(code): IGNORED_SUITES = [ 'addons', 'addons-napi', - 'code-cache', 'doctool', 'internet', 'pummel', From c95e31e09ba93993082459acd9ae4ebfff7fbc06 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 29 Oct 2018 15:14:41 +0800 Subject: [PATCH 3/4] fixup! lib: fix code cache generation --- lib/internal/bootstrap/cache.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/internal/bootstrap/cache.js b/lib/internal/bootstrap/cache.js index 734c6d10c2d3b3..b27a00cd05b800 100644 --- a/lib/internal/bootstrap/cache.js +++ b/lib/internal/bootstrap/cache.js @@ -42,6 +42,11 @@ const cannotUseCache = [ 'internal/bootstrap/node' ].concat(depsModule); +if (process.config.variables.v8_enable_inspector !== 1) { + cannotUseCache.push('inspector'); + cannotUseCache.push('internal/util/inspector'); +} + module.exports = { cachableBuiltins: Object.keys(NativeModule._source).filter( (key) => !cannotUseCache.includes(key) From def5fc06e73e189b40b75e9336807844e51e9128 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 29 Oct 2018 19:22:20 +0800 Subject: [PATCH 4/4] fixup! lib: fix code cache generation --- lib/internal/bootstrap/cache.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/internal/bootstrap/cache.js b/lib/internal/bootstrap/cache.js index b27a00cd05b800..8e20672b71e489 100644 --- a/lib/internal/bootstrap/cache.js +++ b/lib/internal/bootstrap/cache.js @@ -8,6 +8,7 @@ const { NativeModule } = require('internal/bootstrap/loaders'); +const { hasTracing } = process.binding('config'); function getCodeCache(id) { const cached = NativeModule.getCached(id); @@ -47,6 +48,10 @@ if (process.config.variables.v8_enable_inspector !== 1) { cannotUseCache.push('internal/util/inspector'); } +if (!hasTracing) { + cannotUseCache.push('trace_events'); +} + module.exports = { cachableBuiltins: Object.keys(NativeModule._source).filter( (key) => !cannotUseCache.includes(key)