Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

##Performant code requires a different style.

Performant code requires a different style.

###Some points regarding your code

Some points regarding your code

##Example

Example

##Performant code requires a different style.

###Some points regarding your code

##Example

Performant code requires a different style.

Some points regarding your code

Example

added 163 characters in body
Source Link
Blindman67
  • 22.8k
  • 2
  • 16
  • 40

##Performant code requires a different style.

When you are working with a lot of data, or need it done fast, it pays to optimise every thingeverything. That means that you have to forgo many of the idiomatic coding styles that have come around since ES6.

###Some points regarding your code

  • Don't repeat calculations. Eg you calculate the pixel chanel index 4 times in setPixelColor. Should be once.
  • Destructuring is SLOW, I mean forever slow, until browsers get round to making destructuring as performant as standard assignment dont use it in performance code.
  • Move frequent code and variables close to the scope you use the, You have setPixelColor and getPixelColor outside the function, thus it takes extra time to locate these functions.
  • Most devices are 64bit these days, and low end is 32 bits. Moving a byte takes just as long as moving a 4 bytes as a 32bit long or 8 bytes as 64bit long. Create a 32bit typed array so you can move a pixel in a quarter of the time.
  • Use 32Bit variables to store colors (eg [255, 0, 0] is 0x0000FF (NOTE 32bit pixels are backwards ABGR the opposite to CSS colors RGBA))
  • The source of your GC overload is the line for (const n of [[r+1,c], [r-1,c], [r,c+1], [r,c-1]]) { You create an array that contains 4 populated array. A million pixels in a image is small fry, but 5 million arrays is crazy.
  • Avoid getting values from getters, eg cvs.width is likely a getter. Get the value and store it in a variable scoped as close as possible to where you use it. You
  • You work in rows, and columns (x,y) but the pixel data is a single array. Index directly when you can.
  • Use local scope, rather than pass variables if you must call functions inside performance loops..
  • If you can avoid calling functions, its quicker inline.
  • Use lookup tables to reduce math calculations.
  • Run in strict mode as it give a slight performance increase, and would also have spotted the undeclared variable p. Because you did not declare it, it becomes a global, and thus access would be much slower than a local. As it is in the heart of the function this will cost you many CPU cycles.

Using 32 bit pixels avoiding all the array creation theThe following does it all in about 1/4 of the time.

(function () {
 "use strict"; // Always for performant code
 // Colors as 32bit unsigned ints. Order ABGR
 const black = 0xFF000000;
 const red = 0xFF0000FF;
 const green = 0xFF00FF00;
 const blue = 0xFFFF0000;
 const yellow = red | green;
 const magenta = red | blue;
 const cvs = document.getElementById("paint");
 const width = cvs.width; // Get postencialpotencial slow accessors into vars.
 const w = cvs.width; // alias
 const height = cvs.height;
 const size = width * height;
 const ctx = cvs.getContext('2d');
 // black background
 ctx.fillStyle='black';fillStyle = "black";
 ctx.fillRect(0, 0, width, height);
 const imageData = ctx.getImageData(0, 0, width, height);
 // Use 32bit buffer for single pixel read writes
 const d32 = new Uint32Array(imageData.data.buffer); 
 const start = [
 [40 * w + 40, red], // work in precalculated pixel indexes
 [10 * w + 20, green],
 [23 * w + 42, blue],
 [300 * w +333, yellow],
 [200 * w + 333, magenta]
 ];
 const pixOff = [w, -w, 1, -1]; // lookup for pixels left right top bottom
 const pixOffX = [0, 0, 1, -1]; // lookup for pixel x left right
 const queue = []; // keep queue content as simple as possible.
 for (const pixel of start) { 
 queue.push(pixel[0]); // Populate the queue
 d32[pixel[0]] = pixel[1]; // Write pixel directly to buffer
 }
 
 while (queue.length) {
 const idx = queue.shift();
 const x = idx % w; // Need the x coord for bounds test
 for (let i = 0; i< pixOff.length; i++) {
 const nIdx = idx + pixOff[i]; 
 if (d32[nIdx] === black) { // Pixels off top and bottom 
 // will return undefined
 const xx = x + pixOffX[i];
 if (xx > -1 && xx < w ) {
 d32[nIdx] = d32[idx];
 queue.push(nIdx);
 }
 }
 }
 }
 for (const pixel of start) { d32[pixel[0]] = pixel[1] }
 ctx.putImageData(imageData, 0, 0);
})();
<canvas id="paint" width=500px height=500px />

When you are working with a lot of data it pays to optimise every thing.

  • Don't repeat calculations. Eg you calculate the pixel chanel index 4 times in setPixelColor. Should be once.
  • Destructuring is SLOW, I mean forever slow, until browsers get round to making destructuring as performant as standard assignment dont use it in performance code.
  • Move frequent code and variables close to the scope you use the, You have setPixelColor and getPixelColor outside the function, thus it takes extra time to locate these functions.
  • Most devices are 64bit these days, and low end is 32 bits. Moving a byte takes just as long as moving a 4 bytes as a 32bit long or 8 bytes as 64bit long. Create a 32bit typed array so you can move a pixel in a quarter of the time.
  • Use 32Bit variables to store colors (eg [255, 0, 0] is 0x0000FF (NOTE 32bit pixels are backwards ABGR the opposite to CSS colors RGBA))
  • The source of your GC overload is the line for (const n of [[r+1,c], [r-1,c], [r,c+1], [r,c-1]]) { You create an array that contains 4 populated array. A million pixels in a image is small fry, but 5 million arrays is crazy.
  • Avoid getting values from getters, eg cvs.width is likely a getter. Get the value and store it in a variable scoped as close as possible to where you use it. You work in rows, and columns (x,y) but the pixel data is a single array. Index directly when you can.
  • Use local scope, rather than pass variables if you must call functions inside performance loops..
  • If you can avoid calling functions, its quicker inline.
  • Use lookup tables to reduce math calculations.
  • Run in strict mode as it give a slight performance increase, and would also have spotted the undeclared variable p. Because you did not declare it, it becomes a global, and thus access would be much slower than a local. As it is in the heart of the function this will cost you many CPU cycles.

Using 32 bit pixels avoiding all the array creation the following does it all in about 1/4 of the time.

(function () {
 "use strict"; // Always for performant code
 // Colors as 32bit unsigned ints. Order ABGR
 const black = 0xFF000000;
 const red = 0xFF0000FF;
 const green = 0xFF00FF00;
 const blue = 0xFFFF0000;
 const yellow = red | green;
 const magenta = red | blue;
 const cvs = document.getElementById("paint");
 const width = cvs.width; // Get postencial slow accessors into vars.
 const w = cvs.width; // alias
 const height = cvs.height;
 const size = width * height;
 const ctx = cvs.getContext('2d');
 // black background
 ctx.fillStyle='black';
 ctx.fillRect(0, 0, width, height);
 const imageData = ctx.getImageData(0, 0, width, height);
 // Use 32bit buffer for single pixel read writes
 const d32 = new Uint32Array(imageData.data.buffer); 
 const start = [
 [40 * w + 40, red], // work in precalculated pixel indexes
 [10 * w + 20, green],
 [23 * w + 42, blue],
 [300 * w +333, yellow],
 [200 * w + 333, magenta]
 ];
 const pixOff = [w, -w, 1, -1]; // lookup for pixels left right top bottom
 const pixOffX = [0, 0, 1, -1]; // lookup for pixel x left right
 const queue = []; // keep queue content as simple as possible.
 for (const pixel of start) { 
 queue.push(pixel[0]); // Populate the queue
 d32[pixel[0]] = pixel[1]; // Write pixel directly to buffer
 }
 
 while (queue.length) {
 const idx = queue.shift();
 const x = idx % w; // Need the x coord for bounds test
 for (let i = 0; i< pixOff.length; i++) {
 const nIdx = idx + pixOff[i]; 
 if (d32[nIdx] === black) { // Pixels off top and bottom 
 // will return undefined
 const xx = x + pixOffX[i];
 if (xx > -1 && xx < w ) {
 d32[nIdx] = d32[idx];
 queue.push(nIdx);
 }
 }
 }
 }
 for (const pixel of start) { d32[pixel[0]] = pixel[1] }
 ctx.putImageData(imageData, 0, 0);
})();
<canvas id="paint" width=500px height=500px />

##Performant code requires a different style.

When you are working with a lot of data, or need it done fast, it pays to optimise everything. That means that you have to forgo many of the idiomatic coding styles that have come around since ES6.

###Some points regarding your code

  • Don't repeat calculations. Eg you calculate the pixel chanel index 4 times in setPixelColor. Should be once.
  • Destructuring is SLOW, I mean forever slow, until browsers get round to making destructuring as performant as standard assignment dont use it in performance code.
  • Move frequent code and variables close to the scope you use the, You have setPixelColor and getPixelColor outside the function, thus it takes extra time to locate these functions.
  • Most devices are 64bit these days, and low end is 32 bits. Moving a byte takes just as long as moving a 4 bytes as a 32bit long or 8 bytes as 64bit long. Create a 32bit typed array so you can move a pixel in a quarter of the time.
  • Use 32Bit variables to store colors (eg [255, 0, 0] is 0x0000FF (NOTE 32bit pixels are backwards ABGR the opposite to CSS colors RGBA))
  • The source of your GC overload is the line for (const n of [[r+1,c], [r-1,c], [r,c+1], [r,c-1]]) { You create an array that contains 4 populated array. A million pixels in a image is small fry, but 5 million arrays is crazy.
  • Avoid getting values from getters, eg cvs.width is likely a getter. Get the value and store it in a variable scoped as close as possible to where you use it.
  • You work in rows, and columns (x,y) but the pixel data is a single array. Index directly when you can.
  • Use local scope, rather than pass variables if you must call functions inside performance loops..
  • If you can avoid calling functions, its quicker inline.
  • Use lookup tables to reduce math calculations.
  • Run in strict mode as it give a slight performance increase, and would also have spotted the undeclared variable p. Because you did not declare it, it becomes a global, and thus access would be much slower than a local. As it is in the heart of the function this will cost you many CPU cycles.

The following does it all in about 1/4 of the time.

(function () {
 "use strict"; // Always for performant code
 // Colors as 32bit unsigned ints. Order ABGR
 const black = 0xFF000000;
 const red = 0xFF0000FF;
 const green = 0xFF00FF00;
 const blue = 0xFFFF0000;
 const yellow = red | green;
 const magenta = red | blue;
 const cvs = document.getElementById("paint");
 const width = cvs.width; // Get potencial slow accessors into vars.
 const w = cvs.width; // alias
 const height = cvs.height;
 const size = width * height;
 const ctx = cvs.getContext('2d');
 // black background
 ctx.fillStyle = "black";
 ctx.fillRect(0, 0, width, height);
 const imageData = ctx.getImageData(0, 0, width, height);
 // Use 32bit buffer for single pixel read writes
 const d32 = new Uint32Array(imageData.data.buffer); 
 const start = [
 [40 * w + 40, red], // work in precalculated pixel indexes
 [10 * w + 20, green],
 [23 * w + 42, blue],
 [300 * w +333, yellow],
 [200 * w + 333, magenta]
 ];
 const pixOff = [w, -w, 1, -1]; // lookup for pixels left right top bottom
 const pixOffX = [0, 0, 1, -1]; // lookup for pixel x left right
 const queue = []; // keep queue content as simple as possible.
 for (const pixel of start) { 
 queue.push(pixel[0]); // Populate the queue
 d32[pixel[0]] = pixel[1]; // Write pixel directly to buffer
 }
 
 while (queue.length) {
 const idx = queue.shift();
 const x = idx % w; // Need the x coord for bounds test
 for (let i = 0; i< pixOff.length; i++) {
 const nIdx = idx + pixOff[i]; 
 if (d32[nIdx] === black) { // Pixels off top and bottom 
 // will return undefined
 const xx = x + pixOffX[i];
 if (xx > -1 && xx < w ) {
 d32[nIdx] = d32[idx];
 queue.push(nIdx);
 }
 }
 }
 }
 for (const pixel of start) { d32[pixel[0]] = pixel[1] }
 ctx.putImageData(imageData, 0, 0);
})();
<canvas id="paint" width=500px height=500px />
Source Link
Blindman67
  • 22.8k
  • 2
  • 16
  • 40

When you are working with a lot of data it pays to optimise every thing.

  • Don't repeat calculations. Eg you calculate the pixel chanel index 4 times in setPixelColor. Should be once.
  • Destructuring is SLOW, I mean forever slow, until browsers get round to making destructuring as performant as standard assignment dont use it in performance code.
  • Move frequent code and variables close to the scope you use the, You have setPixelColor and getPixelColor outside the function, thus it takes extra time to locate these functions.
  • Most devices are 64bit these days, and low end is 32 bits. Moving a byte takes just as long as moving a 4 bytes as a 32bit long or 8 bytes as 64bit long. Create a 32bit typed array so you can move a pixel in a quarter of the time.
  • Use 32Bit variables to store colors (eg [255, 0, 0] is 0x0000FF (NOTE 32bit pixels are backwards ABGR the opposite to CSS colors RGBA))
  • The source of your GC overload is the line for (const n of [[r+1,c], [r-1,c], [r,c+1], [r,c-1]]) { You create an array that contains 4 populated array. A million pixels in a image is small fry, but 5 million arrays is crazy.
  • Avoid getting values from getters, eg cvs.width is likely a getter. Get the value and store it in a variable scoped as close as possible to where you use it. You work in rows, and columns (x,y) but the pixel data is a single array. Index directly when you can.
  • Use local scope, rather than pass variables if you must call functions inside performance loops..
  • If you can avoid calling functions, its quicker inline.
  • Use lookup tables to reduce math calculations.
  • Run in strict mode as it give a slight performance increase, and would also have spotted the undeclared variable p. Because you did not declare it, it becomes a global, and thus access would be much slower than a local. As it is in the heart of the function this will cost you many CPU cycles.

##Example

Using 32 bit pixels avoiding all the array creation the following does it all in about 1/4 of the time.

See comments for info relating to above comments.

(function () {
 "use strict"; // Always for performant code
 // Colors as 32bit unsigned ints. Order ABGR
 const black = 0xFF000000;
 const red = 0xFF0000FF;
 const green = 0xFF00FF00;
 const blue = 0xFFFF0000;
 const yellow = red | green;
 const magenta = red | blue;
 const cvs = document.getElementById("paint");
 const width = cvs.width; // Get postencial slow accessors into vars.
 const w = cvs.width; // alias
 const height = cvs.height;
 const size = width * height;
 const ctx = cvs.getContext('2d');
 // black background
 ctx.fillStyle='black';
 ctx.fillRect(0, 0, width, height);
 const imageData = ctx.getImageData(0, 0, width, height);
 // Use 32bit buffer for single pixel read writes
 const d32 = new Uint32Array(imageData.data.buffer); 
 const start = [
 [40 * w + 40, red], // work in precalculated pixel indexes
 [10 * w + 20, green],
 [23 * w + 42, blue],
 [300 * w +333, yellow],
 [200 * w + 333, magenta]
 ];
 const pixOff = [w, -w, 1, -1]; // lookup for pixels left right top bottom
 const pixOffX = [0, 0, 1, -1]; // lookup for pixel x left right
 const queue = []; // keep queue content as simple as possible.
 for (const pixel of start) { 
 queue.push(pixel[0]); // Populate the queue
 d32[pixel[0]] = pixel[1]; // Write pixel directly to buffer
 }
 
 while (queue.length) {
 const idx = queue.shift();
 const x = idx % w; // Need the x coord for bounds test
 for (let i = 0; i< pixOff.length; i++) {
 const nIdx = idx + pixOff[i]; 
 if (d32[nIdx] === black) { // Pixels off top and bottom 
 // will return undefined
 const xx = x + pixOffX[i];
 if (xx > -1 && xx < w ) {
 d32[nIdx] = d32[idx];
 queue.push(nIdx);
 }
 }
 }
 }
 for (const pixel of start) { d32[pixel[0]] = pixel[1] }
 ctx.putImageData(imageData, 0, 0);
})();
<canvas id="paint" width=500px height=500px />

default

AltStyle によって変換されたページ (->オリジナル) /