From c1efb0c840b7a851177a9e413bd5fe200af50b4e Mon Sep 17 00:00:00 2001 From: Santiago Gimeno Date: Wed, 27 Feb 2013 10:19:46 +0100 Subject: [PATCH] Cache atoms to avoid redundant requests - Once cached we really don't need to send InternAtom and AtonName requests, so we bypass them. - Add atom_names property to XClient: it's basically the atoms object property reversed. It'll be used to cache the atom names. - Add some tests. - Issue #22. --- lib/corereqs.js | 11 +++++- lib/xcore.js | 40 ++++++++++++++++++- package.json | 3 +- test/atoms-cache.js | 96 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 test/atoms-cache.js diff --git a/lib/corereqs.js b/lib/corereqs.js index 6b9e78f..b3a4b84 100644 --- a/lib/corereqs.js +++ b/lib/corereqs.js @@ -350,8 +350,15 @@ module.exports = { return ['CCSSxxa', [16, returnOnlyIfExist ? 1 : 0, 2+padded.length/4, value.length, padded] ]; }, - function(buf) { - var res = buf.unpack('L')[0]; + function(buf, seq_num) { + var res = buf.unpack('L')[0]; + var pending_atom = this.pending_atoms[seq_num]; + if (!this.atoms[pending_atom]) { + this.atoms[pending_atom] = res; + this.atom_names[res] = pending_atom; + } + + delete this.pending_atoms[seq_num]; return res; } ], diff --git a/lib/xcore.js b/lib/xcore.js index 2bc9508..59ddbdd 100644 --- a/lib/xcore.js +++ b/lib/xcore.js @@ -69,6 +69,15 @@ function XClient(stream, displayNum, screenNum, options) // in/out packets indexed by sequence ID this.replies = {}; this.atoms = stdatoms; + this.atom_names = (function() { + var names = {}; + Object.keys(stdatoms).forEach(function(key) { + names[stdatoms[key]] = key; + }); + + return names; + })(); + this.event_consumers = {}; // maps window id to eventemitter TODO: bad name this.eventParsers = {}; @@ -108,6 +117,7 @@ XClient.prototype.close = function(cb) { XClient.prototype.importRequestsFromTemplates = function(target, reqs) { var client = this; + this.pending_atoms = {}; for (var r in reqs) { // r is request name @@ -154,6 +164,18 @@ XClient.prototype.importRequestsFromTemplates = function(target, reqs) if (templateType == 'function') { + if (reqName === 'InternAtom') { + var value = req_proxy.arguments[1]; + if (client.atoms[value]) { + -- client.seq_num; + return process.nextTick(function() { + callback(undefined, client.atoms[value]); + }); + } else { + client.pending_atoms[client.seq_num] = value; + } + } + // call template with input arguments (not including callback which is last argument TODO currently with callback. won't hurt) //reqPack = reqTemplate.call(args); var reqPack = reqTemplate.apply(this, req_proxy.arguments); @@ -168,6 +190,18 @@ XClient.prototype.importRequestsFromTemplates = function(target, reqs) client.pack_stream.flush(); } else if (templateType == 'Array'){ + if (reqName === 'GetAtomName') { + var atom = req_proxy.arguments[0]; + if (client.atom_names[atom]) { + -- client.seq_num; + return process.nextTick(function() { + callback(undefined, client.atom_names[atom]); + }); + } else { + client.pending_atoms[client.seq_num] = atom; + } + } + var format = reqTemplate[0]; var requestArguments = []; @@ -395,7 +429,11 @@ XClient.prototype.expectReplyHeader = function() var handler = client.replies[seq_num]; if (handler) { var unpack = handler[0]; - var result = unpack( data, opt_data ); + if (client.pending_atoms[seq_num]) { + opt_data = seq_num; + } + + var result = unpack.call(client, data, opt_data); var callback = handler[1]; callback(null, result); // TODO: add multiple replies flag and delete handler only after last reply (eg ListFontsWithInfo) diff --git a/package.json b/package.json index e683a7c..fc43e78 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,8 @@ "mocha": "*", "should": "*", "sax": "*", - "async": "*" + "async": "*", + "sinon": "*" } , "scripts": { "test": "node test-runner.js", diff --git a/test/atoms-cache.js b/test/atoms-cache.js new file mode 100644 index 0000000..8d883e3 --- /dev/null +++ b/test/atoms-cache.js @@ -0,0 +1,96 @@ +var x11 = require('../lib'); +var should = require('should'); +var assert = require('assert'); +var sinon = require('sinon'); +var async = require('async'); + +describe('Atoms and atom names cache', function() { + before(function(done) { + var self = this; + var client = x11.createClient(function(err, dpy) { + should.not.exist(err); + self.X = dpy.client; + self.spy = sinon.spy(self.X.pack_stream, 'flush'); + done(); + }); + + client.on('error', done); + }); + + it('should be used directly when requesting std atoms with InternAtom', function(done) { + var self = this; + this.X.InternAtom(true, 'WM_NAME', function(err, atom) { + should.not.exist(err); + atom.should.equal(self.X.atoms.WM_NAME); + sinon.assert.notCalled(self.spy); + done(); + }); + }); + + it('should be used directly when requesting atom names with GetAtomName', function(done) { + var self = this; + var spy = sinon.spy(this.X.GetAtomName[1]); + this.X.GetAtomName(52, function(err, atom_name) { + should.not.exist(err); + atom_name.should.equal('UNDERLINE_THICKNESS'); + sinon.assert.notCalled(self.spy); + done(); + }); + }); + + it('should be used after the first request for non-std atoms', function(done) { + var self = this; + this.X.InternAtom(false, 'My testing atom', function(err, atom) { + should.not.exist(err); + sinon.assert.calledOnce(self.spy); + async.parallel( + [ + function(cb) { + self.X.InternAtom(true, 'My testing atom', cb); + }, + function(cb) { + self.X.GetAtomName(atom, cb); + } + ], + function(err, results) { + should.not.exist(err); + results[0].should.equal(atom); + results[1].should.equal('My testing atom'); + sinon.assert.calledOnce(self.spy); + done(); + } + ); + }); + }); + + it('should be used after the first request for non-std atom_names', function(done) { + var self = this; + this.X.InternAtom(false, 'My testing atom', function(err, atom) { + should.not.exist(err); + sinon.assert.calledOnce(self.spy); + async.parallel( + [ + function(cb) { + self.X.InternAtom(true, 'My testing atom', cb); + }, + function(cb) { + self.X.GetAtomName(atom, cb); + } + ], + function(err, results) { + should.not.exist(err); + results[0].should.equal(atom); + results[1].should.equal('My testing atom'); + sinon.assert.calledOnce(self.spy); + done(); + } + ); + }); + }); + + after(function(done) { + this.X.pack_stream.flush.restore(); + this.X.terminate(); + this.X.on('end', done); + }); +});