revisit performance on Win10 + Chrome

[pixelzoom]

In #144 (comment), @KatieWoe reported performance on a range of platforms (abbreviated below). Frame rates are for the built version running with ?profiler.

Some of these should be examined before RC testing, to see whether they are OK or require attention. The most curious one is Win10 + Chrome.

OS & version	Browser & version	1.0.0-dev.40 fps
Win 10	Chrome	~35
Win 10	Firefox	~54
Win 10	Edge	60
Win 10	IE	32
Mac 10.14	Chrome	60
Mac 10.14	Safari	~56
Chrome OS	Chrome OS	60
iOS 12	Safari	60
Mac 10.10	Safari	60
iOS 9	Safari	7

[pixelzoom]

@ariel-phet said we can ignore iOS 9 and Win10 + IE.

@KatieWoe can you describe the behavior on Win10 + Chrome? Does it feel jerky? What specifically is the test device? Is it an older or newer model computer?

@arouinfar can you also evaluate on @KatieWoe's test device?

[KatieWoe]

It seemed to be a bit jerkier than the other platforms, but it didn't strike me as being too bad. I probably wouldn't have registered it as a problem if I weren't specifically looking for it. It is my work laptop, so it's relatively new.

[arouinfar]

@pixelzoom @KatieWoe I tested dev.39 and dev.40 out on a brand new gaming PC running Win 10 with pretty beefy specs (AMD 3700X CPU, AMD Radeon 5700XT GPU, 16 GB 3200 MHz RAM) and I am seeing a similar pattern, as described by @KatieWoe. Chrome was noticeably jerkier than Edge, but I wouldn't call the performance unacceptable.

Test case

• Energy screen
• N = 2000 (injected at 200 K so the lid doesn't blow)
• Average speed open
• Histograms open & heavy/light curves on
• TimerNode running
• Remove/add heat keeping P under the max, so particle speeds are constantly changing.
• Eventually allow lid to blow (this is where I took screenshot & where frame rate is the worst)

Edge never budges from 60 FPS. Chrome varied a bit, but hovered around 35-40 FPS. I'm seeing a huge spike in GPU utilization when running the sim in Chrome (up to 90%) compared to Edge (never more than 15%), but no difference in CPU or RAM usage. Screenshots are from dev.39, but the behavior was the same in dev.40.

Chrome

Troubleshooting information (do not edit):
Name: Gas Properties
URL: https://phet-dev.colorado.edu/html/gas-properties/1.0.0-dev.39/phet/gas-properties_en_phet.html?profiler
Version: 1.0.0-dev.39 2019-06-17 03:09:43 UTC
Features missing: touch
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.142 Safari/537.36
Language: en-US
Window: 1711x1294
Pixel Ratio: 1.5/1
WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium)
GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium)
Vendor: WebKit (WebKit WebGL)
Vertex: attribs: 16 varying: 30 uniform: 4096
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 16384x16384
OES_texture_float: true
Dependencies JSON: {}

Edge

Troubleshooting information (do not edit):
Name: Gas Properties
URL: https://phet-dev.colorado.edu/html/gas-properties/1.0.0-dev.39/phet/gas-properties_en_phet.html?profiler
Version: 1.0.0-dev.39 2019-06-17 03:09:43 UTC
Features missing: touch
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.102 Safari/537.36 Edge/18.18362
Language: en-US
Window: 1809x1266
Pixel Ratio: 1.5/1
WebGL: WebGL 1.0
GLSL: WebGL GLSL ES 1.0
Vendor: Microsoft (Microsoft Edge)
Vertex: attribs: 16 varying: 30 uniform: 4096
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 32767x32767
OES_texture_float: true
Dependencies JSON: {}

[pixelzoom]

Versions <= 1.0.0-dev.40 did not have the performance improvement that was make to repo kite to address Canvas performance issues in pH Scale (see phetsims/ph-scale#83). So I'd like to see if that improvement resulted in any improvement to Gas Properties, since it also uses Canvas for drawing particles.

@KatieWoe please test 1.0.0-dev.43?profiler on Win10 + Chrome. Test on your personal machine and on the "brand new gaming PC" mentioned above. Let me know what the frame rate is. Thanks.

[KatieWoe]

I can't really test the gaming pc since I believe that belongs to @arouinfar. I will test mine and @arouinfar, would you be willing to do so on your machine?

[arouinfar]

The earliest I'll be able to test is Friday. If it's been resolved on @KatieWoe's machine, then that's good enough for me.

[KatieWoe]

@pixelzoom still seems to be about 35-36

[pixelzoom]

OK, thanks @KatieWoe.

@arouinfar @ariel-phet Do we need to address this issue for 1.0.0 release? I have no immediate ideas for improving performance, and I don't have a Win10+Chrome test machine handy (would need to borrow one from PhET). This is also likely to require some of @jonathanolson's time.

[arouinfar]

@pixelzoom I personally do not think this needs to be addressed before the 1.0.0 release. While the performance is smoother on other browsers, the simulation still performs reasonably on Win10/Chrome. Since @ariel-phet is a Win10 user, I think it would be good for him to test it out on his machine and see how it feels. I'll defer final judgement to him.

[ariel-phet]

@pixelzoom @arouinfar tested on my machine (Win 10 Chrome), and I agree performance is acceptable.

That being said, my machine did seem to be getting pretty darn hot...and...

Edge will be moving to a "chromium" based browser. This is also by far one of our most popular sims, so it at least seems worthy of some investigation and some of @jonathanolson time.

We do not necessarily need to fix, but while the sim is still fresh in people's minds and is getting some attention, it seems worthy of a bit of investigation. Especially since chrome is by far our most popular browser. @pixelzoom I think you could also ask an on campus dev to do some quick profiling. @chrisklus might be open to it.

[ariel-phet]

Marking as medium, since as mentioned above, I don't think this blocks publication, merely worth of investigation.

[pixelzoom]

Performance issues can typically involve substantial rewriting or reorganization of code. So while I appreciate that it's not blocking publication, I'd like to understand this issue a little bit (at a minimum, identify where the time is being spent) before moving into RC testing.

[kathy-phet]

I tested this link https://phet-dev.colorado.edu/html/gas-properties/1.0.0-dev.43/phet/gas-properties_en_phet.html?profiler
on my Chrome + Window 10 device and got in the high 40s low 50s, hovering right around 50 average at load. I do see a GPU spike.

Test device: Kathy's computer
Operating System: MS Windows Enterprise 10, Version 10.0.17763 Build 17763
Browser: Chrome
Troubleshooting information (do not edit):
Name: ‪Gas Properties‬
URL: https://phet-dev.colorado.edu/html/gas-properties/1.0.0-dev.43/phet/gas-properties_en_phet.html?profiler
Version: 1.0.0-dev.43 2019-07-22 18:36:42 UTC
Features missing: generatedcontent, touch
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.142 Safari/537.36
Language: en-US
Window: 1138x706
Pixel Ratio: 1.25/1
WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium)
GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium)
Vendor: WebKit (WebKit WebGL)
Vertex: attribs: 16 varying: 30 uniform: 4096
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 16384x16384
OES_texture_float: true
Dependencies JSON: {}

[pixelzoom]

@jonathanolson this is the issue mentioned at 7/25/19 status meeting that needs investigation. Thanks in advance for your help.

[jonathanolson]

There was originally a single Canvas for all particles (inside and outside the container). Consulting with @jonathanolson on 5/20/19, he told me that it would be more performant to have 2 size-optimized Canvases, rather than the single canvas. So IdealGasLawParticleSystemNode was reimplemented in 9f98e53. Should I undo that work?

It would benefit only if the "outside" CanvasNode isn't included with its bounds all the time. Right now, even if there are no particles outside, Scenery still needs to have its actual Canvas covert the entire upper region. I'm happy to collaborate on this, and ?showFittedBlockBounds should display where the actual Canvas is.

Good to know that there's some performance to be gained in the histograms. But @KatieWoe's report that resulted in this issue was for the Ideal screen, which does not involve histograms, see #144 (comment).

I had been testing the reproduction instructions in this issue, specifically #146 (comment) (which has the histograms on)

Please provide details of which specific Path(s) you're referring to, and how you recommend overriding computeShapeBounds.

I didn't fully dig in to investigate which Paths were causing the most overhead. The pattern followed so far is path.computeShapeBounds = with a function that will return a Bounds2 that should always include the path's contents.

Let me know if it would help to collaborate on the Canvas or Path issues.

[pixelzoom]

I reviewed HistogramModel and don't see any obvious opportunities for optimizing algorithms. I'm guessing that most of the cost is in array allocation for samples and bin counts. And changing that (e.g. preallocating and reusing arrays) would be nontrivial, because we have arrays of arrays that are smoothed over a sample period.

[pixelzoom]

I'm also wondering about whether I should be using Shape.makeImmutable. From Slack:

Chris Malley 11:25 AM
I see a lot of calls to Shape.makeImmutable in vector-addition. Is this a performance optimization that I’m not aware of? If I know that a Shape will not be mutated, should I be calling makeImmutable?

Jonathan Olson:house_with_garden: 1:17 PM
If it won't be mutated AND will have a longer lifetime than (or be used by) multiple Paths, then it's probably good. Otherwise, a Path needs to add a listener to the Shape to see if it will be changed (which isn't necessarily a huge amount of overhead, but could be a source of memory leaks)

I don't totally understand @jonathanolson's response, so will add this to the list of things to discuss with him.

[pixelzoom]

I discussed on Zoom with @jonathanolson. I'm going to do these optimizations, in this order, to verify that they have a positive impact:

(1) Path.computeShapeBounds

• BarPlotNode
• LinePlotNode
• HistogramNode.intervalLines (factor out IntervalLineNodes.js)

(2) ParticlesNode

• don't repeatedly call invalidatePaint when number of particles is 0

(3) Array optimization for HistogramModel

• use Float32Array or Float64Array. This might hurt performance, or cause other problems.

(4) ParticlesNode

• layerSplit: true, so that each CanvasNode has its own block. This might hurt performance.

Optimizations (1) and (2) will benefit the Energy screen only, since they are related to histograms.

[pixelzoom]

Also noting that we discussed Shape.makeImmutable. It's not relevant here. It's useful when you have a persistent Shape that is used with multiple Paths.

[pixelzoom]

The commits above implement optimizations (1) and (2) described above, in master and 1.0 branches.

@KatieWoe could you please verify whether there is a performance improvement on Win10+Chrome? Please compare 1.1.0-dev.1 to 1.0.0, running with ?profiler, using the Energy screen with both histograms expanded and all checkboxes checked.

I'm seeing ~60fps in both versions on macOS + Chrome (MacBookPro15,1), so it's difficult for me to tell whether these optimizations had any impact.

[chrisklus]

I'm seeing ~60fps in both versions on macOS + Chrome (MacBookPro15,1), so it's difficult for me to tell whether these optimizations had any impact.

For quick performance iteration of problems only affected by slower devices, I often throttle Chrome's CPU x4 under the performance tab of dev tools.

[KatieWoe]

I'm actually getting 55-60 for both now as well. Tried another situation too, just in case. I wonder if another Chrome update helped.

[pixelzoom]

Thanks @chrisklus. With 4x CPU throttling, I'm still seeing 60fps with both versions on macOS + Chrome (MacBookPro15,1).

Thanks @KatieWoe. Unfortunately we have no Chrome version numbers for any of our previously reported results. I don't know what else could be responsible for the 1.0.0 improvement that you're reporting - a Chrome update is a good guess.

So... Since the Win10+Chrome performance is now on par with other platforms, I'm going to refrain from making the remaining optimizations that were proposed in #146 (comment), since they are expected to yield negligible improvement.

[pixelzoom]

To verify in RC testing:

• Run on Win10+Chrome with ?profiler and verify that the performance is acceptable. The worst case performance is the the Energy screen, both histograms expanded, all checkboxes checked for the histograms.
• Spot check on a fe platforms to verify that the histograms are rendered correctly.
• Spot check on a few platforms to verify that particles inside and outside the container are correctly rendered.

[KatieWoe]

All looks good to me rc.1