Issues with `pointsProperty` validation

[pixelzoom]

For https://github.com/phetsims/qa/issues/924 ...

In Curve.ts:

    const initialPoints: CurvePoint[] = [];
    for ( let i = 0; i < this.numberOfPoints; i++ ) {
      const xNormalized = i / ( this.numberOfPoints - 1 ); // in the range [0,1]
      const x = this.xRange.expandNormalizedValue( xNormalized );
      initialPoints.push( new CurvePoint( x, 0 ) );
    }

    this.pointsProperty = new Property( initialPoints, {
      isValidValue: points => isValidPoints( initialPoints, points ),

Two PhET-iO problems with the last line of code above:

(1) initialPoints will never be garbage collected, because it's included in the closure for the isValidValue function. That means that 13,761 (11 x 1251) instances of CurvePoint will hang around forever.

(2) isValidPoints compares every element in points to initialPoints, to verify that they have the same x-coordinate. This seems like a potential performance problem. Perhaps this would be sufficient?

    this.pointsProperty = new Property( initialPoints, {
-      isValidValue: points => isValidPoints( initialPoints, points ),
+      isValidValue: points => ( initialPoints.length === points.length ),

@veillette thoughts?

[veillette]

I don't know how expensive these checks are. My understanding of Property is that it always keep tracks of its initial value for Reset purposes, such that initialPoints would not be GC regardless of isValidValue.

We can try to do some performance comparisons by eliminating altogether isValidValue and go from there.

[pixelzoom]

My understanding of Property is that it always keep tracks of its initial value for Reset purposes, such that initialPoints would not be GC regardless of isValidValue.

Ah, right you are. So nothing to do for that.

We can try to do some performance comparisons by eliminating altogether isValidValue and go from there.

In the State Wrapper console, I see ~900 ms to set state, with and without isValidValue. (macOS 13.2.1 + Chrome 111) So there's no practical benefit to eliminiating the comparison of x-coordinates, and we should keep it in case a client violates #326.

Closing.