[node] switch to NodeRequest member fn callback

For (hopefully) better performance than creating a new v8::Context to
wrap each callback while still avoiding leaking memory with v8::FunctionTemplate.

Adds a JavaScript shim in front of module.exports.Map to wrap the
req.respond API internally and preserve the public callback-passing
API, while still exporting the correct prototype.
This commit is contained in:
Thiago Marcos P. Santos
2016-06-29 21:35:24 +03:00
committed by Mike Morris
parent d6f667a5e7
commit edec56e6c5
8 changed files with 83 additions and 16 deletions

View File

@@ -6,7 +6,7 @@
"mapbox",
"gl"
],
"main": "lib/mapbox-gl-native.node",
"main": "platform/node/index.js",
"repository": {
"type": "git",
"url": "git://github.com/mapbox/mapbox-gl-native.git"

31
platform/node/index.js Normal file
View File

@@ -0,0 +1,31 @@
"use strict";
// Shim to wrap req.respond while preserving callback-passing API
var mbgl = require('../../lib/mapbox-gl-native.node');
var constructor = mbgl.Map.prototype.constructor;
var Map = function(options) {
if (!(options instanceof Object)) {
throw TypeError("Requires an options object as first argument");
}
if (!options.hasOwnProperty('request') || !(options.request instanceof Function)) {
throw TypeError("Options object must have a 'request' method");
}
var request = options.request;
return new constructor(Object.assign(options, {
request: function(req) {
request(req, function() {
req.respond.apply(req, arguments);
});
}
}));
};
Map.prototype = mbgl.Map.prototype;
Map.prototype.constructor = Map;
module.exports = Object.assign(mbgl, { Map: Map });

View File

@@ -43,11 +43,11 @@ static const char* releasedMessage() {
return "Map resources have already been released";
}
NAN_MODULE_INIT(NodeMap::Init) {
void NodeMap::Init(v8::Local<v8::Object> target) {
v8::Local<v8::FunctionTemplate> tpl = Nan::New<v8::FunctionTemplate>(New);
tpl->InstanceTemplate()->SetInternalFieldCount(2);
tpl->SetClassName(Nan::New("Map").ToLocalChecked());
tpl->InstanceTemplate()->SetInternalFieldCount(2);
Nan::SetPrototypeMethod(tpl, "load", Load);
Nan::SetPrototypeMethod(tpl, "loaded", Loaded);

View File

@@ -23,7 +23,7 @@ public:
static Nan::Persistent<v8::Function> constructor;
static NAN_MODULE_INIT(Init);
static void Init(v8::Local<v8::Object> exports);
static void New(const Nan::FunctionCallbackInfo<v8::Value>&);
static void Load(const Nan::FunctionCallbackInfo<v8::Value>&);

View File

@@ -32,6 +32,8 @@ NAN_MODULE_INIT(NodeRequest::Init) {
tpl->InstanceTemplate()->SetInternalFieldCount(1);
tpl->SetClassName(Nan::New("Request").ToLocalChecked());
Nan::SetPrototypeMethod(tpl, "respond", HandleCallback);
constructor.Reset(tpl->GetFunction());
// TODO: Remove this from the public JavaScript API
@@ -49,8 +51,9 @@ void NodeRequest::New(const Nan::FunctionCallbackInfo<v8::Value>& info) {
}
void NodeRequest::HandleCallback(const Nan::FunctionCallbackInfo<v8::Value>& info) {
auto request = Nan::ObjectWrap::Unwrap<NodeRequest>(info.Holder());
// Move out of the object so callback() can only be fired once.
auto request = Nan::ObjectWrap::Unwrap<NodeRequest>(info.Data().As<v8::Object>());
auto callback = std::move(request->callback);
if (!callback) {
return info.GetReturnValue().SetUndefined();
@@ -122,14 +125,9 @@ void NodeRequest::HandleCallback(const Nan::FunctionCallbackInfo<v8::Value>& inf
}
void NodeRequest::Execute() {
// Enter a new v8::Context to avoid leaking v8::FunctionTemplate
// from Nan::New<v8::Function>
v8::Local<v8::Context> context = v8::Context::New(v8::Isolate::GetCurrent());
v8::Context::Scope scope(context);
v8::Local<v8::Value> argv[] = { handle() };
v8::Local<v8::Value> argv[] = { handle(), Nan::New<v8::Function>(NodeRequest::HandleCallback, handle()) };
Nan::MakeCallback(Nan::To<v8::Object>(target->handle()->GetInternalField(1)).ToLocalChecked(), "request", 2, argv);
Nan::MakeCallback(Nan::To<v8::Object>(target->handle()->GetInternalField(1)).ToLocalChecked(), "request", 1, argv);
}
NodeRequest::NodeAsyncRequest::NodeAsyncRequest(NodeRequest* request_) : request(request_) {

View File

@@ -1,13 +1,15 @@
'use strict';
var test = require('tape');
var mbgl = require('../../../../lib/mapbox-gl-native');
var mbgl = require('../../index');
var fs = require('fs');
var path = require('path');
var style = require('../fixtures/style.json');
test('Map', function(t) {
t.test('must be constructed with new', function(t) {
// This test is skipped because of the req.respond shim in index.js
// which will always call new mbgl.Map()
t.skip('must be constructed with new', function(t) {
t.throws(function() {
mbgl.Map();
}, /Use the new operator to create new Map objects/);
@@ -87,6 +89,42 @@ test('Map', function(t) {
t.end();
});
t.test('instanceof mbgl.Map', function(t) {
var options = {
request: function() {},
ratio: 1
};
var map = new mbgl.Map(options);
t.ok(map instanceof mbgl.Map);
t.equal(map.__proto__, mbgl.Map.prototype);
var keys = Object.keys(mbgl.Map.prototype);
t.deepEqual(keys, [
'load',
'loaded',
'render',
'release',
'addClass',
'addSource',
'addLayer',
'removeLayer',
'setLayoutProperty',
'setPaintProperty',
'setFilter',
'dumpDebugLogs',
'queryRenderedFeatures'
]);
for (var key in keys) {
t.equal(map[key], mbgl.Map.prototype[key]);
}
t.end();
});
t.test('.load', function(t) {
var options = {
request: function() {},

View File

@@ -1,7 +1,7 @@
'use strict';
var fs = require('fs');
var mbgl = require('../../../lib/mapbox-gl-native');
var mbgl = require('../index');
var path = require('path');
var test = require('tape');

View File

@@ -1,6 +1,6 @@
'use strict';
var mbgl = require('../../../lib/mapbox-gl-native');
var mbgl = require('../index');
var request = require('request');
mbgl.on('message', function(msg) {