This test's source is located at web.basemark.com/tests/2.1/benchmarks/javascript/array_blur/test.js, running that through a formatter like jsbeautifier.org makes it considerably more readable.
The test's initial setup consists of a single call to initialize a "Bitmap" array with the predefined data, a width, and a height:
this.b = new Bitmap(this.data, 100, 53);
The code of the Bitmap class shows that this class just wraps the "data" array, and interprets its contents as [p1_red, p1_green, p1_blue, p1_alpha, p2_red, p2_green, p2_blue, p2_alpha, ...], in short: there are four color channel values for each pixel. Right there is where we see the first major issue: "data" contains 1500 entries, but 100 * 53 * 4 = 21200. Only about 7% of the required array elements are present! In a sane programming language, this would cause crashes or errors. In JavaScript, however, missing entries in an array are simply "undefined". The JavaScript engine running this, instead of demonstrating how well it can access array elements, will have plenty of fun detecting out-of-bounds accesses and taking the (presumably slower) code path that returns "undefined" in that case -- a code path that correct, well-written code should almost never hit!
Anyway, let's see what happens when the benchmark runs. The main function (with more meaningful names chosen for the parameters) is:
run: function(stop_now, iteration_index) {
this.b.blur(iteration_index % 10) + 2;
counter += (iteration_index % 10) + 1;
It calls the bitmap's "blur" function, which takes a "count" parameter for how often to apply the blurring filter:
function blur(count) {
for (var h = 0; h < count; h++) {
this.applyConvolutionMatrix([1, 1, 1, 1, 1, 1, 1, 1, 1])
}
}
Looks good? Look again. Obviously calling blur(0) does nothing (zero iterations of the for-loop), so they try to avoid that by adding 2 to the result of "iteration_index % 10" (in case you're not familiar with it, '%' means 'modulo', a.k.a. the remainder of the division). However, the "+ 2" is after the function call's closing parenthesis! So what happens is:
In "counter", they keep track of how many blurring iterations were performed. Due to the bug above, it over-counts the amount of work done. Why do they add 1 here, as opposed to adding 2 right above? Beats me. If the parenthesis bug was fixed, this would under-count the amount of work done.
So what does applyConvolutionMatrix() do? What it should be doing is get each pixel's 3x3 neighborhood (i.e. the pixel itself and its eight direct and diagonal neighbors in all directions), and for each color channel compute the average of those nine pixels, weighted by the convolution matrix. What it actually does is it retrieves the nine pixels we would expect (as an array "p") and then:
for (var channel = 0; channel < 3; channel++) {
for (var h = 0; h < count; h++) {
this.applyConvolutionMatrix([1, 1, 1, 1, 1, 1, 1, 1, 1])
}
}
Looks good? Look again. Obviously calling blur(0) does nothing (zero iterations of the for-loop), so they try to avoid that by adding 2 to the result of "iteration_index % 10" (in case you're not familiar with it, '%' means 'modulo', a.k.a. the remainder of the division). However, the "+ 2" is after the function call's closing parenthesis! So what happens is:
- "blur" is called with the result of iteration_index % 10 (which is 0 for one in ten values of iteration_index),
- blur runs and returns no value (which, in JavaScript, means it returns undefined),
- 2 is added to this undefined return value (which, in JavaScript, is perfectly legal),
- the result of this addition is then ignored.
In "counter", they keep track of how many blurring iterations were performed. Due to the bug above, it over-counts the amount of work done. Why do they add 1 here, as opposed to adding 2 right above? Beats me. If the parenthesis bug was fixed, this would under-count the amount of work done.
So what does applyConvolutionMatrix() do? What it should be doing is get each pixel's 3x3 neighborhood (i.e. the pixel itself and its eight direct and diagonal neighbors in all directions), and for each color channel compute the average of those nine pixels, weighted by the convolution matrix. What it actually does is it retrieves the nine pixels we would expect (as an array "p") and then:
for (var channel = 0; channel < 3; channel++) {
for (var neighbor = 0; neighbor < 6; neighbor++) {
n[channel] = n[channel] + matrix[neighbor] * p[neighbor][channel]
}
n[channel] = n[channel] / 6;
It computes the weighted average of the first six out of those nine pixels! The three neighbors below the current pixel are just ignored. Since channel only covers the values 0, 1, 2, which correspond to R-G-B, the alpha-channel is also ignored.
Can it get any better? Well, sure! Considering that the algorithm above needs exactly the first six entries of the "matrix" array, they make sure that it is exactly six elements long:
if (matrix.length == 6) {
It computes the weighted average of the first six out of those nine pixels! The three neighbors below the current pixel are just ignored. Since channel only covers the values 0, 1, 2, which correspond to R-G-B, the alpha-channel is also ignored.
Can it get any better? Well, sure! Considering that the algorithm above needs exactly the first six entries of the "matrix" array, they make sure that it is exactly six elements long:
console.log("Invalid matrix.");
console.log(matrix);
return
}
Except that they're doing it wrong and rejecting arrays with the correct length, whereas any given invalid length is fine! Fortunately the array that they actually do pass in has nine elements (three of which will be ignored), so this bogus check luckily doesn't cause any real damage.
You could also ask why the convolution matrix is all ones. Wouldn't multiplication be more interesting if the right-hand factor wasn't always 1? I think with that question you'd be raising an excellent point.
Besides the egregious bugs described above, the rest of the implementation is just about the most inefficient way I've seen in a long time to implement accesses to an RGBA bitmap. The way it allocates and discards short-lived arrays all the time turns the benchmark mostly into a stress test for object allocation and garbage collection. Getting a pixel's neighbors necessarily requires detecting whether the pixel is at the border of the image; doing this check by accessing a non-existent array element and then comparing if the result is undefined, due to JavaScript's semantics, is much less efficient than comparing the requested index against 0 and the array's length. If I rewrite the blurring function's main loop to be less abjectly inefficient, the score goes up by 5x. With modern browsers usually delivering similar performance, who cares about one browser or another being 10% faster, when every single one of them could do the same work five times as quickly if the program was implemented more efficiently?
Conclusion:
The "Array-Blur" test's code is utter crap. It is horribly buggy and horribly inefficient. While you could argue that this shouldn't matter because a fast browser should handle any task quickly, I think how well browsers handle broken code shouldn't be used for performance comparisons, nor for browser vendors as targets to optimize for.
Except that they're doing it wrong and rejecting arrays with the correct length, whereas any given invalid length is fine! Fortunately the array that they actually do pass in has nine elements (three of which will be ignored), so this bogus check luckily doesn't cause any real damage.
You could also ask why the convolution matrix is all ones. Wouldn't multiplication be more interesting if the right-hand factor wasn't always 1? I think with that question you'd be raising an excellent point.
Besides the egregious bugs described above, the rest of the implementation is just about the most inefficient way I've seen in a long time to implement accesses to an RGBA bitmap. The way it allocates and discards short-lived arrays all the time turns the benchmark mostly into a stress test for object allocation and garbage collection. Getting a pixel's neighbors necessarily requires detecting whether the pixel is at the border of the image; doing this check by accessing a non-existent array element and then comparing if the result is undefined, due to JavaScript's semantics, is much less efficient than comparing the requested index against 0 and the array's length. If I rewrite the blurring function's main loop to be less abjectly inefficient, the score goes up by 5x. With modern browsers usually delivering similar performance, who cares about one browser or another being 10% faster, when every single one of them could do the same work five times as quickly if the program was implemented more efficiently?
Conclusion:
The "Array-Blur" test's code is utter crap. It is horribly buggy and horribly inefficient. While you could argue that this shouldn't matter because a fast browser should handle any task quickly, I think how well browsers handle broken code shouldn't be used for performance comparisons, nor for browser vendors as targets to optimize for.
No comments:
Post a Comment