quantified-uncertainty / squiggle

An estimation language

https://squiggle-language.com

MIT License

150 stars 23 forks source link

"Simulation #x in nS" should account for rendered items #3121

Open OAGr opened 6 months ago

OAGr commented 6 months ago

_ Is refactor
x Is new feature
_ Concerns documentation

Description of suggestion or shortcoming:

In this model, most of the time is spent rendering the Plot, but this isn't captured in the simulation count.

https://squigglehub.org/models/benthamite/vote-trading

After this, it would be useful to also show long things like initial calculator renders take, and Spec/other tests.

berekuk commented 6 months ago

This feature is mostly doable now, but I expect that it'll become useless over time, because of its global semantics:

widgets could load lazily, e.g. depending on whether the widget is on the screen
or depending on whether the widget is collapsed
simulations will move to web workers relatively soon
we'll be streaming top-level results (this is ~easy, btw), this also blurs the semantics of what "global simulation time" means

Also, it's not clear how we could collect these timings from widgets; I guess const f = useReportRunningTime(); f(value.context.path, ms); is possible...

So, my alternative proposal is to measure widget render timings individually, and find a way to display them on individual items. (maybe show a number or a yellow "!" icon when the time is >10ms)

OAGr commented 6 months ago

Good points!

At the same time, here are some things I'm thinking of:

When we run externally, it would be good to show the "total time it takes for regular users". So then, it would be useful to have some way of loading at least "everything needed for one view".
For tests and stuff, we might want some of this to load after the initial loading - but it would still be useful to have one page that lists "What are all the loaded items, and for each, how long did it take?"

We could wait for a more advanced system - at the same time I don't want to let the perfect be the enemy of the good.

I think this could be a useful feature if it's a really quick thing to do before the more thorough jobs, but it's not particularly important.

berekuk commented 5 months ago

Related: #3257. (Btw, I'm not even sure how to detect when React commits the background-rendered DOM tree to the main tree, to measure the full time, but there must be a way?)