Test for memory leaks prior to starting 1.6 release process

[jbphet]

This sim is getting close to the point where a new release will need to go out. It should be tested for memory leaks before that process is started. This GitHub issue will be the place to track the results.

[jbphet]

Here are the results of a memory leak test on the master RequireJS version. All testing was done in a new incognito window in Chrome. The basic test procedure is to record heap snapshot was recorded every minute for 10 minutes, though the very first one (time=0) was done by loading the sim without the fuzz query parameter.

Results:

Time (minutes)	Memory Usage (MB)
0	19.5
1	26.1
2	28.3
3	29.0
4	29.1
5	29.4
6	29.6
7	29.9
8	29.7
9	30.0
10	30.1

These values look pretty reasonable, and there are mostly increases, though there is a small decrease between minutes 7 and 8. I did some comparisons of snapshots in an effort to determine why there appears to be a slow but steady increase in memory usage, and everything I saw appeared to be either performance related items in arrays (many were labeled as deoptimization_data) and allocations into scenery pools. Notably, I did not see any increase in anything easily traceable to specific code in the sim.

I think this looks like acceptable behavior, but will double check with other developers.

Next up: test the memory behavior of the phet-io brand version.

[jbphet]

It has been requested that we don't wait for phet-io to be ready to support a release before doing the next RIAW release, so I'm not going to collect memory usage data for the phet-io version.

[jbphet]

I showed this to @pixelzoom at a recent meeting and compared it to some other sims, and the behavior seems reasonable. Closing.

[pixelzoom]

Reopening in light of https://github.com/phetsims/tandem/issues/71. If RIAW release branch was created after IO types were moved to tandem, then RIAW is almost certainly using more memory than it should be. Up to @jbphet to decide if that's an issue. If it's were me and RC hadn't started yet, I'd grab a fresh set of master repos.

[zepumph]

It's many commits, but the following greatly reduced the memory footprint. Up to JB if RIAW's current footprint is too big:

Emitter and Property fixes: https://github.com/phetsims/axon/commit/4a9762e882ea9862af96fdf0e736754e6e6c35d8 https://github.com/phetsims/axon/commit/eef4587cc2c4eefdbde35955854c6596aed7a682 https://github.com/phetsims/scenery-phet/commit/6a746d36f6e3123d737103509c8d5ebcc8ccd10d https://github.com/phetsims/axon/commit/8b02d30e7f4b4c082ff1fc245506ce38527a8e00 https://github.com/phetsims/scenery-phet/commit/77b9f41b4dbabcdcf74eff2a1aceeb5adfda0c5d

Press/DragListener https://github.com/phetsims/scenery/commit/3d75e4f3d5b29f5099533ce76aaff4b1f4ca04f6 https://github.com/phetsims/scenery/commit/6988a3ac68f5cdb3a0bc69b32ec8c8ab9db7b0dc https://github.com/phetsims/scenery/commit/773ec5e30e87b0db8e04f4c8a429d45a3a19b052

And some others https://github.com/phetsims/tandem/commit/93822898043cbeaf0875d41570d56fbd291b33e5 https://github.com/phetsims/axon/commit/036865d639c3404167c04fe62fad660fa839abf0 https://github.com/phetsims/sun/commit/968bf3c38cf862433a68c1710bd4fb2119de7b27

Hopefully this would be easy enough to cherrypick

[jbphet]

Hopefully this would be easy enough to cherrypick

Well, it would mean creating 5 branches and doing 11 cherry picks, so it's not exactly trivial.

[jbphet]

Here is the data from the same memory test performed on the currently published version, which is v1.4.4:

Time (minutes)	Memory Usage (MB)
0	19.1
1	23.0
2	23.6
3	23.8
4	24.1
5	24.3
6	24.4
7	24.7
8	24.7
9	25.0
10	25.1

...and here is one from the current master version, which would include the changes @zepumph references above.

Time (minutes)	Memory Usage (MB)
0	14.7
1	19.1
2	20.4
3	20.8
4	21.3
5	21.7
6	22.1
7	22.2
8	22.4
9	22.4
10	22.5

This data indicates that the 1.6 RC branch is currently using roughly 5/25 = 20% more memory than the published version and that the changes made on master after the 1.6 release branch was created improves memory usage significantly, making it even better than the published version.

Based on this data, I'm reluctantly deciding that I should go ahead and create the branches for the common code and try to integrate these modifications into the 1.6 release branch. It will be a PITA, but it would likely improve performance and feels to me like it would be a bit negligent to ignore.

[pixelzoom]

@jbphet said:

Based on this data, I'm reluctantly deciding that I should go ahead and create the branches for the common code and try to integrate these modifications into the 1.6 release branch. It will be a PITA, but it would likely improve performance and feels to me like it would be a bit negligent to ignore.

If it were me, I'd delete the 1.6 release branch and start over.

[jbphet]

If it were me, I'd delete the 1.6 release branch and start over.

I think it's worth the time to at least try the items described above. There has been a round of RC testing completed on the 1.6 branch, I'd rather not completely throw that away if it can be avoided.

[jbphet]

If it were me, I'd delete the 1.6 release branch and start over.

Tempting, but since there has been a round of RC testing on the 1.6 branch, I think it makes sense to at least take a shot at the changes listed above and see if it helps. Otherwise, I feel like we'd be throwing away the previous testing.

[jbphet]

I did the updates, and it seems to have gone reasonably well. I just did a quick memory test on a locally built version with these changes, and saw the following:

Time (minutes)	Memory Usage (MB)
0	14.5
5	21.7
10	22.5

This is a big improvement, so I'm going to call it good and have it retested in the next RC.

[jbphet]

The RCs for the 1.6 have passed testing, closing.