Improve performance for the ConcentrationMeterNode wire curve when running with mirror.html

[samreid]

When running with together/examples/mirror.html, redrawing the curve takes up 90% of the CPU. The vector instances have the same values, but are different instances which makes axon think it needs to trigger a change notification, and the updateCode body executes. Hence it would be good to check if the values actually changed before computing cubic curves and redrawing them.

Proposed fix forthcoming.

[samreid]

Oops I guess this should be a beers-law-lab issue, since that is where the code lives. Anyways, reassigning to @pixelzoom for review.

[pixelzoom]

Discussed with @samreid. While this fix works in Concentration, it's a general problem that we don't want to have to address in this manner on a case-by-case basis in sim code. Axon.Property uses referential equality to decide whether a Property's value has changed. And doing playback of state via together creates new instances at each step. So there will be Property change notifications firing that normally would not occur in the sim. Rather than add checks like the commit above, we should address this either in together or axon.Property.

[pixelzoom]

Also worth asking... For playback, is this performance issue really an issue that needs to be addressed? For example, if a teacher is observing playback of a student session, does it really matte that performance is not the same same as the student experienced? The teacher is not interacting with the sim, simply observing what happened, so perhaps the performance hit caused by these Property notifications is a non-issue.

[samreid]

Here is some code that @pixelzoom & I discuss for implementing equals:

  var equals = function( a, b ) {
    if ( a === b ) {
      return true;
    }
    else {

      // We have to check both a.equals(b) and b.equals(a) to handle cases like this (even though it takes twice as long to run)
      // a = Vector2(1,2)
      // b = Vector3(1,2,7)
      return ( a && b && a.equals && b.equals && a.equals( b ) && b.equals( a ) );
    }
  };

[samreid]

At Tuesday's developer meeting, we discussed moving the "is it different" check to together.js.

[pixelzoom]

@samreid What is the status of this? Can I back out the change that you made in https://github.com/phetsims/beers-law-lab/commit/953155e72abcbb820994e5e36abf53163f592e98?

[samreid]

Done above, closing.