[p5.js 2.0 Bug Report]: noise() is laggier in 2.x

[mngyuan]

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• WebGL
• Build process
• Unit testing
• Internationalization
• Friendly errors
• Other (specify if possible)

p5.js version

2.1.1

Web browser and version

Zen 1.17.7b

Operating system

MacOSX 15.6.1

Steps to reproduce this

Steps:

1. Open this sketch https://editor.p5js.org/mngyuan/sketches/oTOMucuCM
2. Change library version to p5 2.x
3. Run the sketch

Snippet:

function setup() {
  createCanvas(300, 600);
}

function draw() {
  // let's save frameCount into a time variable t, and scale it slower for
  // smoother motion
  let t = frameCount * 0.005;
  
  background(220);
  noStroke();
  // let's iterate over the canvas but for speed we'll jump by 3 instead of
  // by one
  for (let i = 0; i < width; i += 3) {
    for (let j = 0; j < height; j += 3) {
      // use noise to color this "pixel", and use t to change over time
      let n = noise(i * 0.005 + t, j * 0.005, t);
      fill(n * 255);
      // draw the "pixel"
      rect(i, j, 3);
    }
  }
}

Expected Behavior

Same framerate as p5.js 1.x

Actual Behavior

p5.js 2.x displays noticeable jittery-ness and lag when rendering this sketch

Notes

I use this sketch to introduce noise and the concept of perlin noise to my students, but I just noticed it seems to be laggier (tested in both Safari and Firefox/Zen on macOS) when using p5.js 2.

[Vikas4245]

I would like to work on this issue please assign this to me .

[reshma045]

Hi @mngyuan, I would like to work on this issue. Can you please assign this to me?

[davepagurek]

Hi! Before jumping into implementation, we have to identify what the cause is. The first thing to check is whether this is unrelated to noise, and is actually a result of the friendly error system checking parameters more slowly than in 1.x. If you add p5.disableFriendlyErrors = true to the top of the file (or load the .min.js version of p5 instead of just .js) do you notice the same problem?

[sidwellr]

I have replicated this using Brave 1.85.118 (Chromium 143.0.7499.169) on Linux Mint 22.1. Using the .min.js version makes the sketch run faster both in 1.11.11 and 2.1.1, but 2.1.1 is still much slower and jittery.

But I'm not convinced the issue is just with noise(). I changed the line let n = noise(i * 0.005 + t, j * 0.005, t); to let n = sin(i * 0.01 + j * 0.01 + t) / 2 + 0.5; (which displays diagonal bars rather than noise). I also changed the framecount multiplier from 0.005 to 0.05 to make it move faster so the difference is more noticable. The sketch is still slower and jittery with versions 2.1.1 and 2.1.2 (beta).

[davepagurek]

Thanks @sidwellr! To narrow it down further, if you don't draw anything and just calculate the sin values, is that still slower? If it is, then it's likely something to do with how p5's global function calls work. Otherwise, it may have to do with how drawing works. (Of course if we're not drawing anything, checking how fast it runs will have to be done differently. Maybe you could grab the time before/after using an operation in a loop?)

[davepagurek]

did a quick test of @mngyuan's initial example. Just setting fill() seems to cause the slowdown, drawing rect()s without filling doesn't seem to be a significant issue: https://editor.p5js.org/davepagurek/sketches/2f8VpUSPU

Some profiling results:

I'm not certain there's much we can do about _setFill, which is also there in the 1.11.1 perf results, but in those everything afterwards is related to noise:

[davepagurek]

Also it looks like I get a crash if I leave it running too long:

p5.js:9941 Uncaught (in promise) RangeError: Too many properties to enumerate
    at Color.toString (p5.js:9941:31)
    at Renderer2D.fill (p5.js:70486:27)
    at fn.fill (p5.js:34784:22)
    at p5.fill (p5.js:88116:25)
    at draw (62438745-9fea-48e5-beac-bcde8f49c7fc:20:7)
    at fn.redraw (p5.js:3787:27)
    at async p5._draw (p5.js:71641:9)

That's coming from the cache of serialized colors getting too big:

p5.js/src/color/p5.Color.js

Lines 321 to 332 in f557d24

	toString(format) {
	const key = `${this._color.space.id}-${this._color.coords.join(',')}-${this._color.alpha}-${format}`;
	let colorString = serializationMap[key];
	if(!colorString){
	colorString = serialize(this._color, {
	format
	});
	serializationMap[key] = colorString;
	}
	return colorString;
	}

Aside from the main issue, maybe this needs to be replaced with a Map and something like:

  toString(format) {
    const key = `${this._color.space.id}-${this._color.coords.join(',')}-${this._color.alpha}-${format}`;
    let colorString = serializationMap.get(key);

    if(!colorString){
      colorString = serialize(this._color, {
        format
      });
      if (serializationMap.size > 1000) {
        serializationMap.delete(serializationMap.keys().next().value)
      }
      serializationMap.set(key, colorString);
    }
    return colorString;
  }

[davepagurek]

Just put up a PR doing the above + a few other optimizations:

• When you construct a color with a string, it passes that string directly to the 2D renderer
• When you construct a color with RGB numbers, it creates a hex string out of it instead of using the default colorjs conversion
• It only lazily creates colorjs objects so that if no conversion is necessary, we can skip extra parsing

@limzykenneth let me know if that approach seems OK to you. It's just optimizing a few common code paths so it does introduce some more complexity and some cases where using different modes may still see reduced performance. Possibly this can be updated to include more manual conversions for performance where we feel it's necessary.

[davepagurek]

Going to close this for now since that PR is merged, while not 100% at original perf it should be better next version.