MPD Performance

In the introducing Volumio 2 topic in the forums Michelangelo suggested that the performance of the MPD plugin needed looking at.

I am still learning how all this hangs together but my developer spidey sense is making me think the the usage of the Node string library may be a bit of an overhead. For example currently the searchFor method looks like this

[code]ControllerMpd.prototype.searchFor = function (lines, startFrom, beginning) {
var self = this;

var count = lines.length;
var i = 0;

while (startFrom + i < count) {
	var line = s(lines[startFrom + i]);

	if (line.startsWith(beginning))
		return line.chompLeft(beginning).trimLeft().s;
	else if (line.startsWith('file:'))
		return '';
	else if (line.startsWith('directory:'))
		return '';

	i++;
}

};[/code]

I do not know whether or not this is used a lot, but it shows what I think might be unnecessary work being done

I haven’t tested it yet, but it could be done without the string library (and therefore letting the V8 engine get on with things - a bit like this

[code]ControllerMpd.prototype.searchFor = function (lines, startFrom, beginning) {

var count = lines.length;
var i = startFrom;

while (i < count) {
	var line = lines[i];

	if (line.indexOf(beginning)===0)
		return line.slice(beginning.length);
	else if (line.indexOf('file:')===0 ||line.indexOf('directory:'==0)
		return '';
	i++;
}

};[/code]

in this case , the string library does a lot of extra stuff in order to do what the string library can already do…

But I may be wrong
If I get some time today I will look into this more.

I have now performed a benchmark - see the code below.

Without taking into consideration the construction/destruction of string objects it is obvious that the ‘string’ library is much slower than native string functions.

Running on OS X using node 4.2.3, when searching through 5001 lines for the last line I get the following results

/usr/local/bin/node StringTests.js
|This is a test 1 |
Microseconds Taken using string: 110019
|This is a test 1 |
Microseconds Taken using native: 3495

which is 31 times faster (WOW)

This figure does of course vary from test to test, but it is always way faster not using the string library.

I have used the native trimleft function in this benchmark, which seems to work, but that is only being called once - most of the time is the difference between indexOf and beginsWith. I have no idea if this is supported with Volumio’s node version.

I think if time is taken to replace the string library usage with native string functions we will see an improvement in the mpd performance.

I have also done this test where the string construction is factored into the benchmark (code not shown), and the speedup is around 40 times consistently.

This won’t fix all the issues I am sure, but it is definitely an overhead we could do without.

Please let me know if my benchmarks are wrong.

[code]var S = require(‘string’);
var microtime = require(‘microtime’);

var stringLines=[];
var lines=[];
var i;
var N=5000;

for (i=0;i<N;i++) {
stringLines.push(S(‘Strings Test 2: This is a test 2’));
stringLines.push(S(‘Strings Test 3: This is a test 3’));
}
stringLines.push(S('Strings Test 1: This is a test 1 '));

for (i=0;i<N;i++) {
lines.push(‘Strings Test 2: This is a test 2’);
lines.push(‘Strings Test 3: This is a test 3’);
}
lines.push('Strings Test 1: This is a test 1 ');

var searchForString = function (lines, startFrom, beginning) {

var count = lines.length;
var i = 0;

while (startFrom + i < count) {
	var line = S(lines[startFrom + i]);

	if (line.startsWith(beginning)) {
		console.log("|"+line.chompLeft(beginning).trimLeft().s+"|");
		return line.chompLeft(beginning).trimLeft().s;
	}
	else if (line.startsWith('file:'))
		return '';
	else if (line.startsWith('directory:'))
		return '';

	i++;
}

};

var searchFor = function (lines, startFrom, beginning) {

var count = lines.length;
var i = 0;

while (startFrom + i < count) {
	var line = lines[startFrom + i];

	if (line.indexOf(beginning)===0) {
		console.log("|"+line.slice(beginning.length).trimLeft()+"|");
		return line.slice(beginning.length);
	}
	else if (line.indexOf('file:')===0)
		return '';
	else if (line.indexOf('directory:')==0)
		return '';

	i++;
}

};

var start=microtime.now();
searchForString(stringLines,0,‘Strings Test 1:’);
var end=microtime.now();

console.log('Microseconds Taken using string: '+ (end-start));

start=microtime.now();
searchFor(lines,0,‘Strings Test 1:’);
end=microtime.now();

console.log('Microseconds Taken using native: '+ (end-start));

[/code]

I was already thinking the same: string library = unnecessary overhead…
So I absolutely agree with you, and I’m ok with refactoring the offending parts . Do we need an upgraded node version to do that?
If not, you can go and change it in a new branch, we test it for a while and if everything is ok we merge! what do you think?

If we upgrade the node version the code changes are much smaller (the method names like beginsWith and trimLeft are the same in the new V8 engine). This would mean that the overall code change would be smaller and therefore less likely to be wrong :slight_smile:

We have the proper version now… Want to replace the string functions with native?

I will be testing the image on my raspberry pi 2 while I am at work today (real job :frowning: )

Then this evening I have a chance to try the string removal.

I will let you know.

:wink: