Memory Leak Test

[marlitas]

In preparation for code review we are running a few memory leak tests.

[marlitas]

I did a memory leak test in phet bran and it indicates there's a leak. I'll investigate.

[marlitas]

Some investigation has led me to investigating MixedFractionNode. I am now removing the calculationNode's children at the top of the multilink that creates the MixedFractionNode, but it seems to still be hanging onto some layout listeners. I'm not sure why since I would assume removing it from the calculationNode would sever all ties.

MixedFractionNode is also used in fraction-common, so I'm going to investigate to see if this is a sim-specific memory leak, or something that is happening with MixedFractionNode in general.

[marlitas]

I looked at Fractions: Mixed Numbers and the sim does not create a new MixedFractionNode every time the fraction changes. This makes me think that the memory leak might be part of how MixedFractionNode is working. However, it also made me wonder if there's a better way to use MixedFractionNode. Do we actually have to create a new one each time or can we modify the original?

[marlitas]

I did another memory test with this patch:

```diff Subject: [PATCH] MixedFractionNode --- Index: js/common/view/MeanCalculationPanel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/MeanCalculationPanel.ts b/js/common/view/MeanCalculationPanel.ts --- a/js/common/view/MeanCalculationPanel.ts (revision 4e62a4ccf10f0b042540e5f3cc14724eb587a401) +++ b/js/common/view/MeanCalculationPanel.ts (date 1716483497536) @@ -161,6 +161,26 @@ alignBounds: alignBounds } ); + const unreducedFraction = new MixedFractionNode( { + numerator: 0, + denominator: 0, + fractionNumbersFont: FRACTION_NUMBER_FONT, + vinculumLineWidth: VINCULUM_LINE_WIDTH, + visibleProperty: calculationsVisibleProperty + } ); + + // The value can be represented as either a decimal, mixed fraction, or a whole number with a remainder. + // We instantiate as a MixedFractionNode so that we can update the whole number, numerator, and denominator + // in MixedFraction representations rather than creating a new MixedFractionNode each time. + let valueRepresentation: Node = new MixedFractionNode( { + numerator: 0, + denominator: 0, + wholeNumberFont: WHOLE_NUMBER_FONT, + fractionNumbersFont: FRACTION_NUMBER_FONT, + vinculumLineWidth: VINCULUM_LINE_WIDTH, + visibleProperty: calculationsVisibleProperty + } ); + // Monitor the dependencies and update the equations as changes occur. Multilink.multilinkAny( [ ...calculationDependencies ], () => { @@ -188,17 +208,10 @@ const additionDenominatorText = new Text( numberOfActiveDataObjects, fractionNumberOptions ); const additionFraction = new VBox( { children: [ additionText, additionFractionLine, additionDenominatorText ] } ); - // Create the Node that shows the total value on top and the number of items to divide by on the bottom. - const unreducedFraction = new MixedFractionNode( { - numerator: totalValues, - denominator: numberOfActiveDataObjects, - fractionNumbersFont: FRACTION_NUMBER_FONT, - vinculumLineWidth: VINCULUM_LINE_WIDTH, - visibleProperty: calculationsVisibleProperty - } ); + // Update the unreducedFraction to show the total value on top and the number of items to divide by on the bottom. + unreducedFraction.numerator = totalValues; + unreducedFraction.denominator = numberOfActiveDataObjects; - // The value can be represented as either a decimal, mixed fraction, or a whole number with a remainder. - let valueRepresentation; if ( options.calculatedMeanDisplayMode === 'decimal' ) { const decimalOptions = { font: DECIMAL_FONT, @@ -226,16 +239,10 @@ fraction.reduce(); } - // Create a node that represents the mean as a mixed fraction. - valueRepresentation = new MixedFractionNode( { - whole: ( meanWholePart > 0 || totalValues === 0 ) ? meanWholePart : null, - numerator: fraction ? fraction.numerator : null, - denominator: fraction ? fraction.denominator : null, - wholeNumberFont: WHOLE_NUMBER_FONT, - fractionNumbersFont: FRACTION_NUMBER_FONT, - vinculumLineWidth: VINCULUM_LINE_WIDTH, - visibleProperty: calculationsVisibleProperty - } ); + // Update the valueRepresentation that represents the mean as a mixed fraction. + valueRepresentation.whole = meanWholePart > 0 || totalValues === 0 ? meanWholePart : null; + valueRepresentation.numerator = fraction ? fraction.numerator : null; + valueRepresentation.denominator = fraction ? fraction.denominator : null; } calculationNode.rows = [ ```

And it looks like the sim is doing better, but the memory leak is not completely gone:

[marlitas]

JO pointed out some super helpful things in https://github.com/phetsims/scenery-phet/issues/857. I went ahead and changed the logic in MeanCalculationPanel so that we are not passing through a visibleProperty on each multilink call. Looks like it's working well.

[marlitas]

New memory test is looking good in phet-brand.

[marlitas]

Test on phet-io brand looks good as well. Closing.