phetsims / fourier-making-waves

"Fourier: Making Waves" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 3 forks source link

Vector2 pooling for data sets #61

Closed pixelzoom closed 3 years ago

pixelzoom commented 3 years ago

bamboo plots data sets, which are {Vector2[]}. This sim requires a large number of points (Vector2 instances) to plot the harmonics and sum. For the highest-frequency harmonic, at least 2000 points are needed.

The sim seems to be responsive on laptop/desktop class machines. But there is an occassional hitch in "space & time" animation, probably due to GC. And the main thing that is dynamically allocated is Vector2.

So... Investigate using Vector2's Poolable features.

pixelzoom commented 3 years ago

createFromPool and freeToPool are the methods that I'll need to use.

I discovered that Poolable has a maxSize option. When calling freeToPool, an instance is only returne to the pool if pool.length < options.maxSize.

For Vector2, maxSize is 100, and there's currently no way to adjust it. That seems like an API problem - one size is not appropriate for all sims. A Vector2 pool limited to maxSize: 100 is of no use to me in Fourier. But number of points needed in Fourier may be a serious memory leak in another sim.

pixelzoom commented 3 years ago

After Slack discussion in developer channel, @jonathanolson is going to make Poolable's maxSize adjustable. See https://github.com/phetsims/phet-core/issues/89#issuecomment-844398711.

pixelzoom commented 3 years ago

Another way to address this that does not involve pooling... Data sets typiclly have a constant number of points, at least in the first 2 screens. I could populate the data sets with Vector2 instances on initialization, then mutate those Vector2 instances as the Fourier series changes. I could probably make that work with bamboo since I’m using the Canvas implementation, which requires the client to call update. This would be a major change from the current implementation. And I'm more than a little concerned about not knowing the future PhET-iO requirements for these data sets (which are currently Properties.)

pixelzoom commented 3 years ago

I'm thinking that we might want to use pooling only for the Discrete screen, when domain is "space & time". That's the only case where there's animaton, and any perceptible issue with GC.

pixelzoom commented 3 years ago

Unassigning until I work on this.

pixelzoom commented 3 years ago

I'm getting close to when I'll need to start working on this again.

I'd like to start by using Vector2 pooling only in the Discrete screen, where there's animation, and a reasonable pool size, ~1K Vector2 instances.

The Wave Packet screen would require a huge pool, ~100K Vector2 instances. In the worst case, there are 97 Fourier components, each of which require 1K points to smoothly render its waveform. In addition to that, there are plots for the sum, envelope, and continuous waveform - each of which require 1K points.

pixelzoom commented 3 years ago

@samreid and I were discussing whether to create Canvas versions of BarPlot and AreaPlot, and whether it would make a performance difference. The Components (center) chart in the Wave Packet screen is clearly the pain point. We noticed that (a) performance is not great on Chromebook (Lenovo 300e Chromebook 81QC) and (b) 40% of time is spent in createFromPool for Vector2 allocation.

pixelzoom commented 3 years ago

An alternative to pooling would be to always use the same number of points in each data set, and reuse the Vector2 instance in those data sets. This might be preferrable to dealing with the hassles of using Poolable -- its maxSize, having to call createFromPool, then remember to call freeToPool, etc.

pixelzoom commented 3 years ago

In the commit above, I removed Vector2 pooling from this sim. There is so much that is shared between screens that pooling was leaking into the other screens, where it was sucking the pool dry, making the pool ineffective in the first screen.

In case I decide to revisit pooling, here's the bit that might be worth salvaging:

  // Vector2 pooling is used only in the Discrete screen. That screen has animation in the 'space & time' domain,
  // and we want to avoid the animation pause that occurs due to garbage collection.  So we'll need a large enough
  // pool for the Discrete screen.  That screen has 11 harmonic plots, 1 sum plot, and 1 infinite harmonic plot.
  // The number of points in a harmonic plots varies based on the frequency of the harmonic (higher frequency requires
  // more points for a smooth plot) by the maximum is FMWConstants.MAX_HARMONICS.  So here we compute what should
  // be a safe maximum.
  Vector2.maxPoolSize = ( FMWConstants.MAX_HARMONICS + 2 ) * FMWConstants.MAX_POINTS_PER_DATA_SET;
pixelzoom commented 3 years ago

Unless GC causes animation pauses that we can't tolerate in the Discrete screen, I'm going to avoid Vector2 pooling. Even then, I think I'd investigate recycling fixed-length arrays for each data set in the Discrete screen.

pixelzoom commented 3 years ago

Having gotten performance sign-off in #131, I'm closing this issue.