Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iojs: introduce internal modules #848

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/_http_common.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const FreeList = require('freelist').FreeList;
const FreeList = require('internal/freelist').FreeList;
const HTTPParser = process.binding('http_parser').HTTPParser;

const incoming = require('_http_incoming');
Expand Down
25 changes: 1 addition & 24 deletions lib/freelist.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,3 @@
'use strict';

// This is a free list to avoid creating so many of the same object.
exports.FreeList = function(name, max, constructor) {
this.name = name;
this.constructor = constructor;
this.max = max;
this.list = [];
};


exports.FreeList.prototype.alloc = function() {
//debug("alloc " + this.name + " " + this.list.length);
return this.list.length ? this.list.shift() :
this.constructor.apply(this, arguments);
};


exports.FreeList.prototype.free = function(obj) {
//debug("free " + this.name + " " + this.list.length);
if (this.list.length < this.max) {
this.list.push(obj);
return true;
}
return false;
};
module.exports = require('internal/freelist');
24 changes: 24 additions & 0 deletions lib/internal/freelist.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';

// This is a free list to avoid creating so many of the same object.
exports.FreeList = function(name, max, constructor) {
this.name = name;
this.constructor = constructor;
this.max = max;
this.list = [];
};


exports.FreeList.prototype.alloc = function() {
return this.list.length ? this.list.shift() :
this.constructor.apply(this, arguments);
};


exports.FreeList.prototype.free = function(obj) {
if (this.list.length < this.max) {
this.list.push(obj);
return true;
}
return false;
};
6 changes: 3 additions & 3 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ Module._nodeModulePaths = function(from) {


Module._resolveLookupPaths = function(request, parent) {
if (NativeModule.exists(request)) {
if (NativeModule.nonInternalExists(request)) {
return [request, []];
}

Expand Down Expand Up @@ -262,7 +262,7 @@ Module._load = function(request, parent, isMain) {
return cachedModule.exports;
}

if (NativeModule.exists(filename)) {
if (NativeModule.nonInternalExists(filename)) {
// REPL is a special case, because it needs the real require.
if (filename == 'repl') {
var replModule = new Module('repl');
Expand Down Expand Up @@ -299,7 +299,7 @@ Module._load = function(request, parent, isMain) {
};

Module._resolveFilename = function(request, parent) {
if (NativeModule.exists(request)) {
if (NativeModule.nonInternalExists(request)) {
return request;
}

Expand Down
2 changes: 2 additions & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@
'lib/v8.js',
'lib/vm.js',
'lib/zlib.js',

'lib/internal/freelist.js',
],
},

Expand Down
3 changes: 3 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3133,6 +3133,9 @@ static void ParseArgs(int* argc,
} else if (strncmp(arg, "--icu-data-dir=", 15) == 0) {
icu_data_dir = arg + 15;
#endif
} else if (strcmp(arg, "--expose-internals") == 0 ||
strcmp(arg, "--expose_internals") == 0) {
// consumed in js
} else {
// V8 option. Pass through as-is.
new_v8_argv[new_v8_argc] = arg;
Expand Down
21 changes: 21 additions & 0 deletions src/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,27 @@
return NativeModule._source.hasOwnProperty(id);
};

const EXPOSE_INTERNALS = process.execArgv.some(function(arg) {
return arg.match(/^--expose[-_]internals$/);
});

if (EXPOSE_INTERNALS) {
NativeModule.nonInternalExists = NativeModule.exists;

NativeModule.isInternal = function(id) {
return false;
};
} else {
NativeModule.nonInternalExists = function(id) {
return NativeModule.exists(id) && !NativeModule.isInternal(id);
};

NativeModule.isInternal = function(id) {
return id.startsWith('internal/');
};
}


NativeModule.getSource = function(id) {
return NativeModule._source[id];
};
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/internal-modules/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('internal/freelist');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this file be called freelist.js or something? Avoids unnecessary churn when another module is added.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what do you mean. js2c drops extensions so this is how we require internal modules. This particular test checks that if we try it in userland module and there is node_modules/internal/freelist.js everything works as before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The directory is called test/fixtures/internal-modules but it's really only about the freelist module. Maybe move index.js to test/fixtures/internal-modules/freelist?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't change a lot. we only need to test this once with any module. it could be anything other than freelist

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions test/parallel/test-internal-modules-expose.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Flags: --expose_internals

var common = require('../common');
var assert = require('assert');

assert.equal(typeof require('internal/freelist').FreeList, 'function');
8 changes: 8 additions & 0 deletions test/parallel/test-internal-modules.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
var common = require('../common');
var assert = require('assert');

assert.throws(function() {
require('internal/freelist');
});

assert(require('../fixtures/internal-modules') === 42);
27 changes: 20 additions & 7 deletions tools/js2c.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,11 @@ def ReadMacros(lines):


NATIVE_DECLARATION = """\
{ "%(id)s", %(id)s_native, sizeof(%(id)s_native)-1 },
{ "%(id)s", %(escaped_id)s_native, sizeof(%(escaped_id)s_native)-1 },
"""

SOURCE_DECLARATION = """\
const char %(id)s_native[] = { %(data)s };
const char %(escaped_id)s_native[] = { %(data)s };
"""


Expand Down Expand Up @@ -293,16 +293,29 @@ def JS2C(source, target):
lines = ExpandMacros(lines, macros)
lines = CompressScript(lines, do_jsmin)
data = ToCArray(s, lines)
id = os.path.basename(str(s)).split('.')[0]
id = '/'.join(re.split('/|\\\\', s)[1:]).split('.')[0]
if delay: id = id[:-6]
if delay:
delay_ids.append((id, len(lines)))
else:
ids.append((id, len(lines)))
source_lines.append(SOURCE_DECLARATION % { 'id': id, 'data': data })
source_lines_empty.append(SOURCE_DECLARATION % { 'id': id, 'data': 0 })
native_lines.append(NATIVE_DECLARATION % { 'id': id })


escaped_id = id.replace('/', '$')
source_lines.append(SOURCE_DECLARATION % {
'id': id,
'escaped_id': escaped_id,
'data': data
})
source_lines_empty.append(SOURCE_DECLARATION % {
'id': id,
'escaped_id': escaped_id,
'data': 0
})
native_lines.append(NATIVE_DECLARATION % {
'id': id,
'escaped_id': escaped_id
})

# Build delay support functions
get_index_cases = [ ]
get_script_source_cases = [ ]
Expand Down