Skip to content

Commit

Permalink
lib: remove use of Debug.MakeMirror()
Browse files Browse the repository at this point in the history
This paves the way for removing `vm.runInDebugContext()`.  Inspection
of Map and Set iterators is now done through V8 instrinsics.

Fixes: #11875
PR-URL: #13295
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Jefe Lindstaedt <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
  • Loading branch information
bnoordhuis committed Nov 23, 2017
1 parent 0028241 commit 05948d8
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 20 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
lib/internal/v8.js
lib/internal/v8_prof_polyfill.js
lib/punycode.js
test/addons/??_*
Expand Down
15 changes: 15 additions & 0 deletions lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
// do this good and early, since it handles errors.
setupProcessFatal();

setupV8();
setupProcessICUVersions();

setupGlobalVariables();
Expand Down Expand Up @@ -414,6 +415,20 @@
};
}

function setupV8() {
// Warm up the map and set iterator preview functions. V8 compiles
// functions lazily (unless --nolazy is set) so we need to do this
// before we turn off --allow_natives_syntax again.
const v8 = NativeModule.require('internal/v8');
v8.previewMapIterator(new Map().entries(), 1);
v8.previewSetIterator(new Set().entries(), 1);
// Disable --allow_natives_syntax again unless it was explicitly
// specified on the command line.
const re = /^--allow[-_]natives[-_]syntax$/;
if (!process.execArgv.some((s) => re.test(s)))
process.binding('v8').setFlagsFromString('--noallow_natives_syntax');
}

function setupProcessICUVersions() {
const icu = process.binding('config').hasIntl ?
process.binding('icu') : undefined;
Expand Down
21 changes: 21 additions & 0 deletions lib/internal/v8.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';

function take(it, n) {
const result = [];
for (const e of it) {
if (--n < 0)
break;
result.push(e);
}
return result;
}

function previewMapIterator(it, n) {
return take(%MapIteratorClone(it), n);
}

function previewSetIterator(it, n) {
return take(%SetIteratorClone(it), n);
}

module.exports = { previewMapIterator, previewSetIterator };
39 changes: 19 additions & 20 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const { TextDecoder, TextEncoder } = require('internal/encoding');
const { isBuffer } = require('buffer').Buffer;

const { errname } = process.binding('uv');
const { previewMapIterator, previewSetIterator } = require('internal/v8');

const {
getPromiseDetails,
Expand Down Expand Up @@ -77,7 +78,6 @@ const dateToISOString = Date.prototype.toISOString;
const errorToString = Error.prototype.toString;

var CIRCULAR_ERROR_MESSAGE;
var Debug;

/* eslint-disable */
const strEscapeSequencesRegExp = /[\x00-\x1f\x27\x5c]/;
Expand Down Expand Up @@ -356,17 +356,6 @@ function stylizeNoColor(str, styleType) {
return str;
}

function ensureDebugIsInitialized() {
if (Debug === undefined) {
const runInDebugContext = require('vm').runInDebugContext;
// a workaround till this entire method is removed
const originalValue = process.noDeprecation;
process.noDeprecation = true;
Debug = runInDebugContext('Debug');
process.noDeprecation = originalValue;
}
}

function formatValue(ctx, value, recurseTimes, ln) {
// Primitive types cannot have properties
if (typeof value !== 'object' && typeof value !== 'function') {
Expand Down Expand Up @@ -474,10 +463,10 @@ function formatValue(ctx, value, recurseTimes, ln) {
formatter = formatTypedArray;
} else if (isMapIterator(value)) {
braces = ['MapIterator {', '}'];
formatter = formatCollectionIterator;
formatter = formatMapIterator;
} else if (isSetIterator(value)) {
braces = ['SetIterator {', '}'];
formatter = formatCollectionIterator;
formatter = formatSetIterator;
} else {
// Check for boxed strings with valueOf()
// The .valueOf() call can fail for a multitude of reasons
Expand Down Expand Up @@ -782,17 +771,27 @@ function formatMap(ctx, value, recurseTimes, keys) {
return output;
}

function formatCollectionIterator(ctx, value, recurseTimes, keys) {
ensureDebugIsInitialized();
const mirror = Debug.MakeMirror(value, true);
const vals = mirror.preview();
const output = [];
function formatCollectionIterator(preview, ctx, value, recurseTimes,
visibleKeys, keys) {
var nextRecurseTimes = recurseTimes === null ? null : recurseTimes - 1;
var vals = preview(value, 100);
var output = [];
for (const o of vals) {
output.push(formatValue(ctx, o, recurseTimes));
output.push(formatValue(ctx, o, nextRecurseTimes));
}
return output;
}

function formatMapIterator(ctx, value, recurseTimes, visibleKeys, keys) {
return formatCollectionIterator(previewMapIterator, ctx, value, recurseTimes,
visibleKeys, keys);
}

function formatSetIterator(ctx, value, recurseTimes, visibleKeys, keys) {
return formatCollectionIterator(previewSetIterator, ctx, value, recurseTimes,
visibleKeys, keys);
}

function formatPromise(ctx, value, recurseTimes, keys) {
var output;
const [state, result] = getPromiseDetails(value);
Expand Down
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@
'lib/internal/http2/core.js',
'lib/internal/http2/compat.js',
'lib/internal/http2/util.js',
'lib/internal/v8.js',
'lib/internal/v8_prof_polyfill.js',
'lib/internal/v8_prof_processor.js',
'lib/internal/streams/lazy_transform.js',
Expand Down
6 changes: 6 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4288,6 +4288,12 @@ void Init(int* argc,
const char no_typed_array_heap[] = "--typed_array_max_size_in_heap=0";
V8::SetFlagsFromString(no_typed_array_heap, sizeof(no_typed_array_heap) - 1);

// Needed for access to V8 intrinsics. Disabled again during bootstrapping,
// see lib/internal/bootstrap_node.js.
const char allow_natives_syntax[] = "--allow_natives_syntax";
V8::SetFlagsFromString(allow_natives_syntax,
sizeof(allow_natives_syntax) - 1);

// We should set node_is_initialized here instead of in node::Start,
// otherwise embedders using node::Init to initialize everything will not be
// able to set it and native modules will not load for them.
Expand Down

0 comments on commit 05948d8

Please sign in to comment.