Why does using `color` have such bad performance

nlooije

I am using the following piece of code to set the color of each pixel:

colorMode(RGB,1); for (var x = 0; x < nX; x++) { for (var y = 0; y < nY; y++) { var id = getIdx(x,y); var v = vArray[id]; //range=[0,1] var c = color(v, 0, 1-v); set(x, y, c); } } updatePixels();

It is very straightforward; i get a value from an array vArray which is in the range [0,1]. A color c is is calculated using color() and pixel [x,y] is set to c. Unfortunately, this has very bad performance compared to the rest of my code; color() takes approximately 60% of the CPU time. It is a huge bottleneck compared to the rest of the code which should be the bottleneck considering it is doing heavy work. If i move color() out of the loop the code runs efficiently again.

Does anyone know why color() is so inefficient? How can I avoid this inefficiency, either by modifying the use of color() or perhaps by setting the pixel color in some other way?

GoToLoop

http://forum.processing.org/two/discussion/13074/performance-of-p5-js-when-using-color-and-set

Does anyone know why color() is so inefficient?

This is part of the long & tortuous path your color(v, 0, 1-v); takes: >:)

https://GitHub.com/processing/p5.js/blob/master/src/color/creating_reading.js#L244

p5.prototype.color = function() {
  var args = new Array(arguments.length);

  for (var i = 0; i < args.length; ++i) {
    args[i] = arguments[i];
  }

  if (args[0] instanceof p5.Color) {
    return args[0];
  } else if (args[0] instanceof Array) {
    return new p5.Color(this, args[0]);
  } else {
    return new p5.Color(this, args);
  }
};

https://GitHub.com/processing/p5.js/blob/master/src/color/p5.Color.js#L18

p5.Color = function (pInst, vals) {
  this.mode = pInst._renderer._colorMode;
  this.maxes = pInst._renderer._colorMaxes;
  var isHSB = this.mode === constants.HSB,
      isRGB = this.mode === constants.RGB,
      isHSL = this.mode === constants.HSL;

  if (isRGB) {
    this._array = p5.Color._getFormattedColor.apply(pInst, vals);
  } else if (isHSB) {
    this.hsba = p5.Color._getFormattedColor.apply(pInst, vals);
    this._array = color_utils.hsbaToRGBA(this.hsba);
  } else if (isHSL){
    this.hsla = p5.Color._getFormattedColor.apply(pInst, vals);
    this._array = color_utils.hslaToRGBA(this.hsla);
  } else {
    throw new Error(pInst._renderer._colorMode + ' is an invalid colorMode.');
  }

  this.rgba = [ Math.round(this._array[0] * 255),
                Math.round(this._array[1] * 255),
                Math.round(this._array[2] * 255),
                Math.round(this._array[3] * 255)];
  return this;
};

https://GitHub.com/processing/p5.js/blob/master/src/color/p5.Color.js#L449

p5.Color._getFormattedColor = function () {
  var numArgs = arguments.length;
  var mode    = this._renderer._colorMode;
  var maxArr  = this._renderer._colorMaxes[this._renderer._colorMode];
  var results = [];

  // Handle [r,g,b,a] or [h,s,l,a] color values
  if (numArgs >= 3) {
    results[0] = arguments[0] / maxArr[0];
    results[1] = arguments[1] / maxArr[1];
    results[2] = arguments[2] / maxArr[2];
    results[3] = typeof arguments[3] === 'number' ?
              arguments[3] / maxArr[3] : 1;
  // Handle strings: named colors, hex values, css strings
  } else if (numArgs === 1 && typeof arguments[0] === 'string') {
    var str = arguments[0].trim().toLowerCase();

    if (namedColors[str]) {
      // Handle named color values
      return p5.Color._getFormattedColor.apply(this, [namedColors[str]]);
    }

    // Work through available string patterns to determine how to proceed
    if (colorPatterns.HEX3.test(str)) {
      results = colorPatterns.HEX3.exec(str).slice(1).map(function(color) {
        // Expand #RGB to #RRGGBB
        return parseInt(color + color, 16) / 255;
      });
      results[3] = 1;
    } else if (colorPatterns.HEX6.test(str)) {
      results = colorPatterns.HEX6.exec(str).slice(1).map(function(color) {
        return parseInt(color, 16) / 255;
      });
      results[3] = 1;
    } else if (colorPatterns.RGB.test(str)) {
      results = colorPatterns.RGB.exec(str).slice(1).map(function(color) {
        return color / 255;
      });
      results[3] = 1;
    } else if (colorPatterns.RGB_PERCENT.test(str)) {
      results = colorPatterns.RGB_PERCENT.exec(str).slice(1)
        .map(function(color) {
          return parseFloat(color) / 100;
        });
      results[3] = 1;
    } else if (colorPatterns.RGBA.test(str)) {
      results = colorPatterns.RGBA.exec(str).slice(1)
        .map(function(color, idx) {
          if (idx === 3) {
            return parseFloat(color);
          }
          return color / 255;
        });
    } else if (colorPatterns.RGBA_PERCENT.test(str)) {
      results = colorPatterns.RGBA_PERCENT.exec(str).slice(1)
        .map(function(color, idx) {
          if (idx === 3) {
            return parseFloat(color);
          }
          return parseFloat(color) / 100;
        });
    }
    // convert RGBA result to correct color space
    if( results.length ){
      if( mode === constants.RGB ){
        return results;
      }
      else if( mode === constants.HSL ){
        return color_utils.rgbaToHSLA(results);
      }
      else if( mode === constants.HSB ){
        return color_utils.rgbaToHSBA(results);
      }
    }

    // test string HSLA format
    if (colorPatterns.HSL.test(str)) {
      results = colorPatterns.HSL.exec(str).slice(1)
        .map(function(color, idx) {
        if( idx === 0 ) {
          return parseInt(color, 10) / 360;
        }
        return parseInt(color, 10) / 100;
      });
      results[3] = 1;
    } else if (colorPatterns.HSLA.test(str)) {
      results = colorPatterns.HSLA.exec(str).slice(1)
        .map(function(color, idx) {
        if( idx === 0 ){
          return parseInt(color, 10) / 360;
        }
        else if( idx === 3 ) {
          return parseFloat(color);
        }
        return parseInt(color, 10) / 100;
      });
    }
    // convert HSLA result to correct color space
    if( results.length ){
      if( mode === constants.RGB ){
        return color_utils.hslaToRGBA(results);
      }
      else if( mode === constants.HSL ){
        return results;
      }
      else if( mode === constants.HSB ){
        return color_utils.hslaToHSBA(results);
      }
    }

    // test string HSBA format
    if (colorPatterns.HSB.test(str)) {
      results = colorPatterns.HSB.exec(str).slice(1)
        .map(function(color, idx) {
        if( idx === 0 ) {
          return parseInt(color, 10) / 360;
        }
        return parseInt(color, 10) / 100;
      });
      results[3] = 1;
    } else if (colorPatterns.HSBA.test(str)) {
      results = colorPatterns.HSBA.exec(str).slice(1)
        .map(function(color, idx) {
        if( idx === 0 ){
          return parseInt(color, 10) / 360;
        }
        else if( idx === 3 ) {
          return parseFloat(color);
        }
        return parseInt(color, 10) / 100;
      });
    }
    // convert HSBA result to correct color space
    if( results.length ){
      if( mode === constants.RGB ){
        return color_utils.hsbaToRGBA(results);
      }
      else if( mode === constants.HSB ){
        return results;
      }
      else if( mode === constants.HSL ){
        return color_utils.hsbaToHSLA(results);
      }
    }

    // Input did not match any CSS Color pattern: Default to white
    results = [1, 1, 1, 1];
  } // Handle greyscale color mode
  else if((numArgs === 1 || numArgs === 2)&& typeof arguments[0] === 'number')
  {
    // When users pass only one argument, they are presumed to be
    // working in grayscale mode.
    if (mode === constants.RGB) {
      results[0] = arguments[0] / maxArr[0];
      results[1] = arguments[0] / maxArr[1];
      results[2] = arguments[0] / maxArr[2];
      results[3] = typeof arguments[1] === 'number' ?
                     arguments[1] / maxArr[3] : 1;
    } else {
      results[0] = arguments[0];
      results[1] = arguments[0];
      results[2] = arguments[0] / maxArr[2];
      results[3] = typeof arguments[1] === 'number' ?
                     arguments[1] / maxArr[3] : 1;
    }
  } else {
    throw new Error (arguments + 'is not a valid color representation.');
  }

  return results;
};

blindfish

@GoToLoop said:

This is part of the long & tortuous path your color(v, 0, 1-v); takes

Ouch! :((

This is slightly painful to see:

var args = new Array(arguments.length);

  for (var i = 0; i < args.length; ++i) {
    args[i] = arguments[i];
  }

Why not:

var argsLen = arguments.length,
      args = new Array(argsLen);

  for (var i = 0; i < argsLen; ++i) {
    args[i] = arguments[i];
  }

That's loop optimisation 101 no?

(Edit:) ...or even, since arguments is already an array:

var args = [].slice.call(arguments);

(Edited again - see optimization killers)

To be fair a lot of the rest is helper code to account for more esoteric colour modes. It looks to me as though if you stick to RGB mode and pass separate RGB values it's not quite as bad as it looks.

Still you could try and take a shortcut by creating a p5.Color object directly instead of relying on color():

var foo;
function setup() {
    createCanvas(600, 400);
    foo = new p5.Color(this, [255,0,0]);
}

function draw() {
    background(foo);
}

nlooije

Thanks for your replies, i posted the same question to SO and there i was given a look at the source for color() and it is an unoptimized mess.

Fortunately, since i am not interested in 'esoteric' color modes, i was able to improve the code significantly by passing in an RGBA array to set rather than calculate a Color object:

for (var x = 0; x < nX; x++) { for (var y = 0; y < nY; y++) { var id = getIdx(x,y); var v = vArray[id]; //range=[0,1] var c = [255*v,0,255*(1-v),255]; //rgba set(x, y, c); } } updatePixels();

no need to set colorMode().

GoToLoop

Your excerpt still got lots to be improved. I'll try to cook something up here w/ pixels[].

Just so you know, every single time we call color(), background(), set(), stroke() and fill(), a new p5.Color is created unless we pass an already created 1 to them!

And you've already witnessed the monstrosity that happens in order to get each p5.Color! :-S

EDIT: set() doesn't create another instance of p5.Color like the others! :bz
Although much more limited to accepted types of color parameters, its parser is much lighter! \m/

nlooije

I am very interested in any improvements you can suggest. I considered directly setting the pixels[] and i know that the documentation states that this can be 'slightly' faster than set() but figured it would be micro-optimizing compared to the rest of the code and 'slightly' less readable.

GoToLoop

Still you could try and take a shortcut by creating a p5.Color object directly instead of relying on color().

Directly instantiating a p5.Color rather than calling color() just cut that useless array instantiation.

However, the most frightful things happen inside "hidden" method _getFormattedColor(). :-O

The only way to guarantee performance is to always pre-instantiate all the p5.Color we're gonna need.

GoToLoop

... or even, since arguments is already an array: var args = [].slice.call(arguments);

Actually arguments is "array-like" b/c it lacks its actual methods. :-B

Nonetheless there's no need to transform it into a proper Array within color() b/c later on p5.Color's constructor already calls _getFormattedColor() using apply(). :-@

Here's my shorter implementation for color() btW: :ar!

p5.prototype.color = function (c) {
  if (c.constructor === p5.Color)  return c;
  const isArrayLike = typeof c === 'object' && c.length >= 0;
  return new p5.Color(this, isArrayLike? c : arguments);
};

blindfish

However, the most frightful things happen inside "hidden" method _getFormattedColor().

I didn't look properly beyond the first condition since if you pass it arguments of length >= 3 it should complete quickly no?

Actually arguments is "array-like" b/c it lacks its actual methods.

You're of course correct (and pedantic) as usual :P

Nonetheless there's no need to transform it into a proper Array within color() b/c later on p5.Color's constructor already calls _getFormattedColor() using apply().

You might get away with that for now, but I'd file it as an over-optimisation. From what I can gather it's bad practice to 'leak' the arguments object outside the calling function. What happens if someone comes to p5.Color later and expects a real array?

Also it transpires my suggested use of slice is not good for optimisation: Mozilla docs makes reference to Optimization killers; which explains the use of the seemingly backwards loop; though don't know why the array length isn't cached; presumably as this is now typically done by the JS engine.

Anyway - if the above link is correct it suggests that your attempts to write code as succinctly (and illegibly) as possible don't always result in the best performance :P

GoToLoop

... since if you pass it arguments of length >= 3 it should complete quickly no?

Indeed it's a much better case than passing a string. But still it does a lotta division operations.

From what I can gather it's bad practice to 'leak' the arguments object outside the calling function. What happens if someone comes to p5.Color later and expects a real array?

Function color() is merely an entry-point to create p5.Color objects, not arrays.

And if you examine p5.Color's expected parameter types, you'll see that the 2nd 1 gotta be an arraylike/pseudoarray b/c it passes it to _getFormattedColor() via apply():
this._array = p5.Color._getFormattedColor.apply(pInst, vals);

And apply()'s 2nd argument gotta be an arraylike structure. Otherwise it CRASHES! 8-X
https://developer.Mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply

And going back to color(), it commits 2 sins:

1st, It calls if (args[0] instanceof p5.Color) return args[0]; after:

var args = new Array(arguments.length);
for (var i = 0; i < args.length; ++i)   args[i] = arguments[i];

Even though returning the passed p5.Color, which is arguments[0], doesn't need that whole thing!

2nd, like I've mentioned, no need to transform arguments into a real array just yet.
p5.Color's constructor already takes care of that! :P

And my color() implementation takes care of both bottlenecks cited above:

p5.prototype.color = function (c) {
  if (c.constructor === p5.Color)  return c;
  const isArrayLike = typeof c === 'object' && c.length >= 0;
  return new p5.Color(this, isArrayLike? c : arguments);
};

Besides accepting any arraylike objects. Not just restricted to "real" arrays. :bz

You might get away with that for now, but I'd file it as an over-optimization.

JavaScript isn't as fast or as smart as the much older & mature Java.
It doesn't have the benefit of a pre-compiled bytecode either.
Nor the certainty of which datatype a variable is holding at a certain moment.

The whole color parser is frequently used by many drawing functions.
So any tiny optimization counts a lot for it! :-\"

GoToLoop

I considered directly setting the pixels[] and I know that the documentation states that this can be 'slightly' faster than set()...

Indeed set() is reasonably fast. And a very important reason it's b/c it got a simpler color parser rather than relying on _getFormattedColor() and other colorMode() calculations.
It's always RGBA w/ 0-255 range for each of its 4 channels. B-)

Although not much needed, I said I was cooking something up to be even faster... :^o
Finally I've come to a solution w/ pixels[] + Uint32Array, but incompatible w/ "retina" as set() is. :-$
Although adding some more complexity, "retina" is easy to support too: :ar!

// forum.processing.org/two/discussion/13114/
// why-does-using-color-have-such-bad-performance

// GoToLoop (2015-Oct-20)

pixels.buffer || loadPixels();

const dots = new Uint32Array(pixels.buffer);
var row, idx, v, c;

for (var x, y = 0; y < nY; ++y)  for (row = y * width, x = 0; x < nX; ++x) {
  idx = row + x, v = vArray[idx];
  c = 0xFF*v << 030 | 0xFF*(1-v) << 010 | 0xFF;
  dots[idx] = c;
}

updatePixels();

nlooije

I appreciate the effort you put into making it faster! can you indicate approximately how much faster this is compared to my solution?

GoToLoop

Actually I haven't even run it once. Just blind typing! 8-}
Why don't you try it out and tell us whether you got less CPU or not? O:-)