Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

Commit

Permalink
Replace heartbeat with ping-pong
Browse files Browse the repository at this point in the history
Node is now waiting for Phantomjs to respond with a 'pong' before sending the next 'ping'.
This could fix performance issues when Phantomjs' stdin was cluttered with heartbeat messages.

Fixes #12
  • Loading branch information
jhnns committed Sep 3, 2014
1 parent 72e34d8 commit 0c3f5a3
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 25 deletions.
44 changes: 27 additions & 17 deletions lib/Phantom.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ var phantomMethods = require("./phantom/methods.js");

var pageId = 0;
var slice = Array.prototype.slice;
var heartbeatInterval = 10;
var pingInterval = 100;
var nextRequestId = 0;

/**
Expand All @@ -29,12 +29,12 @@ function Phantom(childProcess) {
Phantom.prototype.childProcess = null;

/**
* The current active heartbeat interval id as returned by setInterval()
* The current scheduled ping id as returned by setTimeout()
*
* @type {*}
* @private
*/
Phantom.prototype._heartbeatIntervalId = null;
Phantom.prototype._pingTimeoutId = null;

/**
* The number of currently pending requests. This is necessary so we can stop the interval
Expand Down Expand Up @@ -62,7 +62,6 @@ Phantom.prototype._pendingDeferreds = null;
Phantom.prototype.constructor = function (childProcess) {
var self = this;

this._sendHeartbeat = this._sendHeartbeat.bind(this);
this._receive = this._receive.bind(this);

this.childProcess = childProcess;
Expand Down Expand Up @@ -148,7 +147,7 @@ Phantom.prototype.dispose = function () {
self.run(phantomMethods.exitPhantom).catch(reject);

self.childProcess = null;
clearInterval(self._heartbeatIntervalId);
clearTimeout(self._pingTimeoutId);
});
};

Expand All @@ -174,8 +173,8 @@ Phantom.prototype._send = function (message, fnIsSync) {
resolve: resolve,
reject: reject
};
if (!fnIsSync && self._heartbeatIntervalId === null) {
self._heartbeatIntervalId = setInterval(self._sendHeartbeat, heartbeatInterval);
if (!fnIsSync) {
self._schedulePing();
}
self._pending++;

Expand All @@ -201,6 +200,18 @@ Phantom.prototype._receive = function (message) {
// we have no chance to map it back to a pending promise.
// Luckily this JSON can't be invalid because it has been JSON.stringified by PhantomJS.
message = JSON.parse(message);

// pong messages are special
if (message.status === "pong") {
this._pingTimeoutId = null;

// If we're still waiting for a message, we need to schedule a new ping
if (this._pending > 0) {
this._schedulePing();
}
return;
}

deferred = this._pendingDeferreds[message.id];

// istanbul ignore next because this is tested in a separated process and thus isn't recognized by istanbul
Expand All @@ -216,11 +227,6 @@ Phantom.prototype._receive = function (message) {
delete this._pendingDeferreds[message.id];
this._pending--;

if (this._pending === 0) {
clearInterval(this._heartbeatIntervalId);
this._heartbeatIntervalId = null;
}

if (message.status === "success") {
deferred.resolve(message.data);
} else {
Expand All @@ -229,17 +235,21 @@ Phantom.prototype._receive = function (message) {
};

/**
* Sends an heartbeat action to the PhantomJS process. This function is passed to setInterval().
* Check out lib/phantom/start.js for an explanation of the heartbeat action.
* Sends a ping to the PhantomJS process after a given delay.
* Check out lib/phantom/start.js for an explanation of the ping action.
*
* @private
*/
Phantom.prototype._sendHeartbeat = function () {
write(this.childProcess, { action: "heartbeat" });
Phantom.prototype._schedulePing = function () {
// Don't schedule ping when we're already disposed or another timeout is already set
if (!this.childProcess || this._pingTimeoutId) {
return;
}
this._pingTimeoutId = setTimeout(write, pingInterval, this.childProcess, { action: "ping" });
};

/**
* Helpers function that stringifies the given message-object, appends an end of line character
* Helper function that stringifies the given message-object, appends an end of line character
* and writes it to childProcess.stdin.
*
* @param {child_process.ChildProcess} childProcess
Expand Down
30 changes: 22 additions & 8 deletions lib/phantom/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ function createResolver(message) {
* @param {Object} data
*/
function resolve(data) {
system.stdout.writeLine("message to node: " + JSON.stringify({
write({
status: "success",
id: message.done? null : message.id,
data: data
}));
});
message.done = true;
}

Expand Down Expand Up @@ -82,11 +82,11 @@ function createRejecter(message) {
};
}

system.stdout.writeLine("message to node: " + JSON.stringify({
write({
status: "fail",
id: message.done? null : message.id,
data: data
}));
});
message.done = true;
}

Expand Down Expand Up @@ -123,6 +123,15 @@ function evalSrc(src, context, resolve, reject) {
eval(src);
}

/**
* Helper function that stringifies the given object and writes it to system.stdout
*
* @param {Object} message
*/
function write(message) {
system.stdout.writeLine("message to node: " + JSON.stringify(message));
}

/**
* Collection of request-able commands (as defined in the action-property of the message).
*
Expand All @@ -131,11 +140,16 @@ function evalSrc(src, context, resolve, reject) {
commandHandlers = {

/**
* The heartbeat command is a neat trick so phantomjs isn't stuck in the stdin.readLine()-loop
* when waiting for an asynchronous event. A heartbeat-command is sent by node every 10m while waiting
* for a result. It is necessary because phantomjs' stdin is completely synchronous.
* The ping command is a neat trick so phantomjs isn't stuck in the stdin.readLine()-loop
* while waiting for an asynchronous event. A ping-command is sent by node as long as it
* waits for PhantomJS to respond. We're responding with a pong to tell node that we're waiting
* for the next ping.
*/
heartbeat: noop,
ping: function () {
write({
status: "pong"
});
},

/**
* Runs message.data.src in the default context.
Expand Down

0 comments on commit 0c3f5a3

Please sign in to comment.