Skip to content
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

Investigate: Tyrian apps seem CPU intensive #254

Closed
davesmith00000 opened this issue Jan 30, 2024 · 4 comments
Closed

Investigate: Tyrian apps seem CPU intensive #254

davesmith00000 opened this issue Jan 30, 2024 · 4 comments

Comments

@davesmith00000
Copy link
Member

If you run the standard g8 template, and then check the task manager in Chrome, you can see the Tyrian is using a surprising amount of CPU even when idling.

If may by an unavoidable overhead of the architecture, but maybe we just introduced a regression somewhere. Worth looking into.

@davesmith00000
Copy link
Member Author

We speculate that this is due to the hot render loop.

#255

@davesmith00000
Copy link
Member Author

I've spent some time butchering the runtime and trying out different things.

My own linux machine seems to show even higher CPU load than I recall the original reporter talking about.

Some findings:

  1. If I remove the render loop, CPU usage when the app is at rest, is zero as expected. So that's good.
  2. If I remove the normal render loop, and bolt in a "does nothing" loop based on pure Scala.js (no Cats Effect, does not render with Snabbdom), then the CPU load is back.

So fundamentally, the CPU load seems to be coming from requestAnimationFrame. Below you can see the "does nothing" version in the screenshot, note the CPU load.

image

Surely this can't be the case?! How does Elm cope? Well on my machine... Elm has almost the same problem. Maybe slightly better, depending on the type of app your running, but not much. Certainly when I'm interacting with an app there is no real difference

Still, is there anything we can do?

Well, we render on frame tick if there has been a model change. We do that because if you're doing heavy updates you don't want to be re-rendering for nothing, but it adds constant load. If you render only on Msg then you can have zero overhead while the app is idle, but it can go nuts during use - for instance it would render even on a No-Op.

The only other thing I can think of is to do a hybrid. In this model you render on requestAnimationFrame, as long as you saw a message in the last, say, ~2 seconds. So if your app is idle, the rendering will go idle, but while stuff is happening (whether triggered by the user or anything else) the rendering continues.

@armanbilge
Copy link
Collaborator

The only other thing I can think of is to do a hybrid. In this model you render on requestAnimationFrame, as long as you saw a message in the last

That is what my PR #255 attempted to do. I got side-tracked but I intend to take a second look and fix it :)

@davesmith00000
Copy link
Member Author

No worries @armanbilge.

I've been refactoring / toying with the runtime on and off over the last couple of days, a) because it feels like the only bit of the code base I'm slightly uncomfortable with - I'd like to fix that, and b) because I'd like to see if an improvement is possible before I cut another release.

I am looking at it, but if you feel like you really want to jump back on it, do let me know. Your time is always appreciated. Equally I'm sure you're doing a bazillion other things, so don't worry over it either. 😄

davesmith00000 added a commit that referenced this issue Apr 5, 2024
Remove ModelHolder

A model update triggers redraw

Remove unused argument

Call redraw on a rendering class

Record last triggered time

Minor refactoring

Anemic Renderer

Revert "Anemic Renderer"

This reverts commit f297cb7.

Imperative version, but it kinda works.

Tidying up comments

Use the model Ref to use all updates

Replaced imperative impl with CE
davesmith00000 added a commit that referenced this issue Apr 9, 2024
Remove ModelHolder

A model update triggers redraw

Remove unused argument

Call redraw on a rendering class

Record last triggered time

Minor refactoring

Anemic Renderer

Revert "Anemic Renderer"

This reverts commit f297cb7.

Imperative version, but it kinda works.

Tidying up comments

Use the model Ref to use all updates

Replaced imperative impl with CE
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants