Closed gwhitney closed 3 weeks ago
You said in the desc: "this change avoids reloading that specimen when the URL is updated to reflect what specimen is being viewed"
Can you elaborate? If I'm looking at Thue Trellis and I delete the URL back to the base URL (http://localhost:5173
), it does indeed restart the visualizer (the Thue Trellis turtle restarts the path).
Ok there's definitely a performance issue on my machine. To reproduce, first try this in ui2 and then try it in this PR and see the difference:
(1) go to polyfactors, and scrub the mouse watching the immediate response of the mouseover highlight/info (2) go to gallery and load vfib (3) go to gallery and load thue trellis (4) go to gallery and load wait for it (5) go to gallery and load beatty dna (6) return to polyfactors, and scrub the mouse watching the response. In this PR the response is laggy at this point.
Is it possible that some visualizers are not being destroyed as well as they used to be? So that the cpu is still trying to deal with all those webgl specimens?
Can you elaborate? If I'm looking at Thue Trellis and I delete the URL back to the base URL (
http://localhost:5173
), it does indeed restart the visualizer (the Thue Trellis turtle restarts the path).
Yes, I agree. The difference is that in ui2, when you visit the base URL, whatever the current query is gets loaded twice (in quick succession, so the first one may not be noticeable). Now it only happens once, because only one URL ever gets to the scope -- the one with the query filled in.
Is it possible that some visualizers are not being destroyed as well as they used to be? So that the cpu is still trying to deal with all those webgl specimens?
I agree something is up. Will investigate.
@katestange can you verify that the performance hit does not occur if rather than going through the Gallery, you just cut and paste the proper URLs into the address bar in turn?
I am pretty stymied to know what is going on here. Everything seems to be cleaning itself up in the scope. I can't get any draws that shouldn't be happening to console.log(), and if I add a message in the scope cleanup it seems always to be printed. So now I am wondering if it could be the Gallery itself that is causing the performance gap...
Trying to follow up on the idea that maybe the Gallery is the problem, Kate can you check whether if you start from a polyfactors that is responding OK, and then go to the Gallery and hit the back button (i.e., don't do a different scope), click on Gallery again and then do a back button, etc maybe 3 or 4 cycles, are you then seeing a performance hit when you get back to polyfactors after say 4 roundtrips? Thanks for letting me know. Have not had any success in tracking this down yet.
Kate can you check whether if you start from a polyfactors that is responding OK, and then go to the Gallery and hit the back button (i.e., don't do a different scope), click on Gallery again and then do a back button, etc maybe 3 or 4 cycles, are you then seeing a performance hit when you get back to polyfactors after say 4 roundtrips?
Yes, I am definitely seeing a hit when I do this, but I see a similar hit doing this in ui2.
@katestange can you verify that the performance hit does not occur if rather than going through the Gallery, you just cut and paste the proper URLs into the address bar in turn?
Yes, I can verify this.
(1) go to polyfactors, and scrub the mouse watching the immediate response of the mouseover highlight/info (2) go to gallery and load vfib (3) go to gallery and load thue trellis (4) go to gallery and load wait for it (5) go to gallery and load beatty dna (6) return to polyfactors, and scrub the mouse watching the response. In this PR the response is laggy at this point.
If I do all this except that I return to polyfactors by loading the URL instead of clicking the gallery icon, I don't have the performance problem.
except that I return to polyfactors by loading the URL instead of clicking the gallery icon, I don't have the performance problem.
Oh that makes it hard to debug because I guess changing the url manually in the url bar must be akin to doing a reload, which of course releases all the phantom visualizers (if indeed that is what's causing the performance issue). I will start by seeing if the Gallery seems to be leaving behind a mess, and if so, clean that up, and then take things from there.
Indeed, the Thumbnails in the Gallery were leaving behind a mess of visualizers, because their canvases were being removed from the DOM before the onUnmounted call was occurring, so the handle the code was using for the visualizer.depart()
call was no longer valid at that time. Worked around this by saving a valid handle elsewhere.
And yes, this problem was present in ui2, but all of the reloading that was being triggered every time the specimen changed was masking it.
Correcting that leak of active visualizers fixes all the performance issues for me. You?
Yes! The performance issues are gone!! I wonder if this will correct other performance issues elsewhere that we've come across. Excellent! I've tested this pretty extensively now, so I'm ready to merge.
Prior to this change, the browser 'back' button would always update the URL, but the visualization would typically not change if you were going from one scope URL (
/?name=Blah&...
) to another.This PR remedies the situation by watching for route changes in the Scope view, and reloading the new visualizer and sequence into the specimen already being viewed. Implementing this tactic requires the ability to completely change the sequence and visualizer of a specimen, which previously did not exist.
The change also enables delaying specimen initialization from Vue setup() time to just before the component is mounted. As a result, the Vue setup is not asynchronous, so the experimental Vue
<Suspense>
tag is no longer needed.Adds a test that the back button actually took effect, to the existing test that changes a parameter. (The extended test definitely failed in ui2 prior to this PR.)
Uses the router to redirect from an empty URL to the last-viewed specimen, rather than the Scope component; this change avoids reloading that specimen when the URL is updated to reflect what specimen is being viewed.
Finally, fixes a small previously-existing bug in which the Differences visualizer could never display more terms than were available in its last parameter settings, even if you changed the parameters to make more terms available.
Resolves #414.
Given the large number of internal changes to Specimen.ts, please in review do play with a veriety of visualizations to make sure all seems to be working well.
By submitting this PR, I am indicating to the Numberscope maintainers that I have read and understood the contributing guidelines and that this PR follows those guidelines to the best of my knowledge. I have also read the pull request checklist and followed the instructions therein.