-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve performance on slower devices #191
Comments
1.0.0-dev.16 iPad testingTest 1: Idle first screen All numbers are in frames per second (min: 0, max: 60). Each number was recorded once the sim had settled out to a constant value for each respective state. If a constant value was never reached (within a tolerance of 1-2 FPS), the observed range of values was recorded. Any unclear or questionable results were re-tested for clarity. Tested on Safari with Jemison device, MD510LL/A, iOS version 10.3.3
|
A number of optimizations have been implemented, and I just did a quick test and saw substantial improvements for tests 1 through 5. There are still some situations where the sim doesn't feel very usable on an iPad 4 (Jemison), so there is further work to be done here. From my observations, I think the next steps should be (and this is here mostly for my own benefit):
|
I reran the tests from #191 (comment) to get some numbers recorded from @jbphet's optimizations, and to have a new baseline for future optimizations. Testing on the latest dev version, https://phet-dev.colorado.edu/html/energy-forms-and-changes/1.0.0-dev.17/phet/energy-forms-and-changes_en_phet.html
|
I've looked into a few of @jbphet's next steps and tested any possibilities I found on Jemison.
For starters, I changed the loop that iterates over all of the steam bubbles to c-style, since there are ~126 bubbles being rendered at max steaming. This helped a little on my device (from a variable 54-60 FPS -> constant 60 for Test 5), but I didn't notice any performance increase on Jemison. The majority of the steam's computation load is in this loop, but I didn't see any obvious things that could be done to our current implementation. Redrawing all of the bubbles every step seems like the most expensive part. I'd like to hear @jbphet's thoughts on this.
I turned off the text nodes on energy chunks to see if implementing this would help, but didn't notice a performance increase.
I tried this, and did not notice a performance increase. I think the energy chunk redistribution algorithm is going to be an important place to get improvements from. I will look into it a bit now, but @jbphet will likely have better ideas on how to do this. |
…hunks was calculated, did some code cleanup, made minor changes to distribution params, see #191
Based on profiling on Chrome, the biggest user of CPU cycles in the energy chunk distributor. Right now, it works through a repulsion algorithm where each energy chunk is repelled by the walls of the "slice" that it is in as well as by all other energy chunks in the thermal model element. This looks quite good, but it's an order n-squared algorithm that runs at every time step for each model element whose number of energy chunks has changed. The best way to address this may well be to come up with an alternative algorithm that we either use all the time or just use on lower performance devices. |
I had tried some optimizations to the repulsive algorithm in the optimizedECDistribution branch, but they changed the way the algorithm operated, and not in a way that seemed like a visual improvement. I just took some time and carefully propagated these optimizations to the main branch, taking care to make sure that the visual appearance didn't change. It seemed like it worked, and it got a 9 FPS on the test case, which is cool! For the record, the test procedure was to run a RequireJS version with the profiler on, full screen on the main display of my Win 10 Dell laptop, and to stack the water beaker on the brick and heat it to steady-state steaming. Before the optimizations it was at 31 FPS, after it was 40 FPS. |
This should be re-tested on the the next RC, but other than that, I think the development work is done. |
The alternate distribution has not been showing up in the rc. Including on iOS 9. Going back to dev 4 shows that this platform was still running the alternate distribution at that time. #238 also happens to a very large degree on this platform but does not occur in dev 4 |
Same with iOS 10. |
I'm still not getting the alternate distribution in 1.2.0-dev.1. Is this supposed to be the case? @jbphet @chrisklus |
Good to know, thanks. For all test devices @KatieWoe? From the dev test issue:
So, I'd say that if the normal algorithm is working well enough in all the places that it's not showing up then that is a good thing. If looks terrible on some devices and you would expect the spiral algorithm to kick in, but it's not, then that seems like a problem. |
Well, I would expect it to kick in for iOS 9 and 10 at least, maybe Edge as well, but it doesn't. |
Gotcha, hmm. What frame rate range are you seeing for for those platforms? |
(Heating up the water and watching the energy chunks float up.) |
Thanks, yeah it should definitely be kicking in for performance that poor. I think we tried to set the cutoff to be ~8-10 FPS. @jbphet do you have any time to look into this in the next few days? If not, I might be able to get my dad to bring home his old iPad 2 home this weekend - I think that's the only device to debug this issue with that I have access to. Even at 6x CPU throttling on my computer, I can only get it down to 16 FPS. |
I just ran a test using an iOS 9 device (Tycho), and found that the performance is good enough that the alternative algorithm just isn't kicking in. I suspect that the optimizations to the "regular" distribution algorithm made enough of a difference that this threshold is not reached. I tried testing with the alternative algorithm on using the I tested on Edge, and it looked great, and it didn't even get close to the threshold. I'm guessing that the new version has come out that uses a lot of Chrome internals, so performance of the browser is way better. By the way, I think that the reason the frame rate is so slow on the iOS devices has as much or more to do with the steam animation than the energy chunk distribution. @chrisklus - I don't think it's worth lowering the threshold in order to make this algorithm kick in if we're only going to get 1 FPS improvement. Agreed? If so, I think we just leave it as is, unless we want to dig into improving it on iOS 10. |
Agreed! Thanks for looking into it. Sounds good to me, ready to close. |
@jbphet and I will track and commit our latest performance improvements in this issue.
The text was updated successfully, but these errors were encountered: