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

Using scaleX/scaleY instead of alpha/visible to control visibility performs better #466

Open
hiratariq opened this issue Mar 13, 2023 · 12 comments
Labels
performance Issue is related to a performance problem

Comments

@hiratariq
Copy link

Hi,

In a very complex render tree, I experimented with scaling the components down to nothing instead of hiding their visibility,
link to example

It seems to me that scaleX/scaleY is significantly faster than alpha/visible to mainly hide and show. It makes quite a difference on our slower device as well. I haven't come across any penalty from Lightning that comes with it, that is, patching is faster comparatively, and the drawFrame right after seems to take about the same amount of time.

Is there something I am overlooking, if not, why is scaling faster than visibility?

Thanks!

@imtiaz101325
Copy link

This is interesting! Which devices specifically did you try this on? @hiratariq
Did you also check FPS and memory usage?

@hiratariq hiratariq changed the title Using scaleX/scaleY instead of alpha/visible to control visibility performs better on slower device Using scaleX/scaleY instead of alpha/visible to control visibility performs better Mar 13, 2023
@hiratariq
Copy link
Author

The device is a STB, FPS generally is pretty down on it anyway, same with memory usage.

@imtiaz101325
Copy link

@hiratariq is your STB running RDK software like RDK-V?
If not could you share the browser the STB uses?

@uguraslan uguraslan added the performance Issue is related to a performance problem label Mar 13, 2023
@hiratariq
Copy link
Author

It's not running RDK.
Browser is Chrome 39. To make our code work on it we transpile it using babel.
We limit memory pressure to 3.8e6 of GPU memory usage in pixels for the device during lightning stage initialisation.

However the scale vs visibility performance comparison is not just limited to this device, its just more noticeable on the device as it is quite slow.

@imtiaz101325
Copy link

Hi @hiratariq

I am not seeing the performance benefits on my machine(M1 Mac).

I copied your code here.

Screenshot 2023-03-20 at 10 45 04 PM

Attaching the profile in case you wanna check the difference on your end:
scaleIsMorePerfPOC_profile.json.zip

Maybe you could try to capture the performance profile on your end and share?

@hiratariq
Copy link
Author

hiratariq commented May 5, 2023

Edit: Please ignore this comment

Hi @imtiaz101325 apologies I've taken this long to check for updates on github.

I think at the time I posted the codepen I forgot to disable image worker. But I've updated it to not use it. In my codebase I completely removed alpha/visible property and used scale throughout which made significant difference on the STB.

In the codepen however, at the time I posted it here I saw the difference I was talking about, but rerunning today the results are inconsistent and changing order of the lines of code also effects the time. I'll have to write an elaborate example instead. I've tweaked the codepen a little which basically after showing and hiding the Element I repeat it again. So previously I had
hide (alpha),
show (alpha),
hide (scale),
show (scale)

Now I have
hide (alpha),
show (alpha),
hide again (alpha),
show again (alpha),
hide (scale),
show (scale),
hide again (scale),
show again (scale),

But really changing the order of lines changes everything and the codepen doesn't do a good enough job to prove my findings in the actual codebase I am working with.

I'll give the code you've attached a go and get back to you with better example.

@hiratariq
Copy link
Author

hiratariq commented May 9, 2023

Edit: Please ignore the striked through bit

These are screenshots from my profiling session. But I have gone above and beyond to prove this in isolation and timing the execution of the two methods in my codebase. It turns out it is only an issue when I am running the debugger. So this really isn't an issue and can be closed. Apologies for the confusion.

using_alpha
using_scale

@hiratariq hiratariq closed this as not planned Won't fix, can't repro, duplicate, stale May 9, 2023
@github-project-automation github-project-automation bot moved this to Done in Lightning May 9, 2023
@hiratariq hiratariq reopened this May 9, 2023
@hiratariq
Copy link
Author

Apologies, but after having slept on the issue I realised patching without rendering anything was the missing piece in the codePen so I have now updated it https://codepen.io/hiratariq/pen/wvErXmb

Now it does show that scaling instead of changing visibility does perform better, so the issue is valid thats why reopened.

@imtiaz101325
Copy link

Hi @hiratariq
The results are indeed interesting. I have quickly set it up and observed the results.
Screenshot 2023-05-14 at 14 14 25
Will look into this further and update here and on my public workbook https://imtiaz101325.notion.site/Investigation-rdkcentral-Lightning-issues-466-1a98a0de12374ad88c6f39eab4912e04

@erikhaandrikman
Copy link
Contributor

erikhaandrikman commented May 15, 2023

Hi, i will try to explain why setting scaleX: 0 results in a better and faster execution compared to alpha: 0
( TLDR at the bottom )

From an engine perspective it needs to execute the following steps when updating scaleX property:

  • if value is not equal to current scaleX value
  • update value
  • update its local transformation
  • flag the component that it has transformation recalculation updates
  • flag the parent chain that it has updates
  • If the component has a parent flag that we have render updates, since a change in descendants needs to re-create any render textures and re-execute the shader ( if the part of the render-tree has alpha > 0 )
  • recalculate it’s local translation

Steps for updating alpha property:

  • test for any potential rounding errors
  • test if value is not equal to current alpha value
  • update value
  • update local alpha
  • Test if component world alpha > 0 and the parent world alpha > 0
  • flag the component that it has recalculation updates
  • flag the parent chain that it has alpha updates
  • If the component has a parent, flag that we have render updates, since a change in descendants needs to re-create any render textures and re-execute the shader ( if the part of the render-tree has alpha > 0 )
  • test for alpha rounding error

Currently, we are essentially executing similar logic when modifying the scaleX property and the alpha property. However, Lightning allows us to listen to changes in alpha and visibility, additional logic is required to update the 'enabled' flag for that specific section of the branch. It's important to note that this code is not invoked when we solely update the scaleX property, even though it produces the same rendering output.

  • Execute logic to update enabled flag
  • Test if the element & parent are enabled ( visible true & alpha > 0)
  • if alpha === 0 we execute by Lightning.Component overridable _disable() lifecycle event
  • if alpha > 0 we execute by Lightning.Component overridable _enable() lifecycle event
  • flag component to be enabled / disabled

    If we’re setting alpha: 0, so effectively disabling the component, the core needs to: 

  • As multiple elements can utilize the same texture for sampling, it is necessary to remove the instance of [this] component from the Texture elements property. This step is crucial because when the Texture is no longer in use (i.e., no component is utilizing it for rendering), Lightning's texture garbage collector can release the associated memory. This improves performance and prevents any memory leaks from occurring.
  • Since multiple elements can be renderer with the same shader, we need to remove [ this ] component instance from the bound Shader.
  • remove [ this ] component instance from any render to texture.
  • flag disabled
  • Iterate over the children
  • repeat the above for each child.

For setting alpha: 1, it involves an even more complex set of steps that need to be taken. I will provide a quick overview for clarity:

  • Update texture dimensions because dimensions might have changed
  • Update texture coordinates
  • Update the Texture to add [ this ] to elements using the texture to sample from during rendering ( fragment shader )
  • Test if the component is with in the renderable viewport ( on screen ) and execute active flag logic
  • Add [ this ] instance to Shader instance elements

Since Lightning is build with embedded in mind, there is a lot more logic being executed when you update the alpha property of a component, it will eagerly try to clean-up after itself to free up resources.

TLDR; The reason why setting scaleX: 0 leads to better performance because it avoids executing unnecessary tree enabling/disabling logic. When aiming for a stable 60 frames per second in our app, we have a limited time of 16.7 milliseconds to construct a frame. By skipping the execution of code that is not required, we can speed up this process and achieve better overall frames per second (FPS). However, it's important to note that simply updating all visibility or alpha properties to scaleX may cause memory issues over time, as it doesn't free up memory and can result in rendering problems.

@imtiaz101325
Copy link

Thanks so much @erikhaandrikman 🙏🏾
This is incredible insight ✅💪🏾
From this explanation we can now try to follow the code path and get a deeper understanding of the engine 😍

@hiratariq
Copy link
Author

hiratariq commented May 22, 2023

Thanks @erikhaandrikman I did dive a little into the code to realise the recursion is where the two methods differ. Thanks for the detailed explanation. Definitely shader and texture management matters in our codebase. But is there an alternate method, that produces the visual results faster and then cleanups later when convenient to us?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Issue is related to a performance problem
Projects
Status: Done
Development

No branches or pull requests

4 participants