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

Unnecessary allocations per frame #494

Open
itsjamie opened this issue Jun 21, 2023 · 3 comments
Open

Unnecessary allocations per frame #494

itsjamie opened this issue Jun 21, 2023 · 3 comments
Assignees
Labels
performance Issue is related to a performance problem

Comments

@itsjamie
Copy link
Contributor

Doing some memory allocations timeline testing in Chrome has revealed a few spots where Lightning is performing a lot of heap allocations unnecessarily leading to a lot of garbage being generated per frame.

The Rounded Rectangle Shader is a key part where this occurs.

Each uniform is set using the variadic function _setUniform and the values are passed as individual parameters, but then put into an array to be passed to uniform4fv, etc. The issue with that, is putting those values into an array allocates an array of those floats. So in our application that leads to ~300kb/s of GC pressure.

Providing the ability to disable the WebGLStateManager and refactoring the uniforms to directly use functions like uniform4f when the parameters are not already in a vector leads to far less GC pressure, and in our experience can drastically improve performance.

Understanding that it may be a design of tradeoffs, wanted to open an issue for discussion here to see if there was anything we could be missing.

@erikhaandrikman erikhaandrikman self-assigned this Jun 23, 2023
@itsjamie
Copy link
Contributor Author

In addition, to the allocations from the use of setUniform, a shader like RoundedRectangle allocate on every frame in the map function converting the radius to the final int32's.

@uguraslan uguraslan added the performance Issue is related to a performance problem label Oct 2, 2023
@elsassph
Copy link
Contributor

Ugh that's bad. For some shaders the arrays could potentially be "cached"?

@itsjamie
Copy link
Contributor Author

For some shaders the arrays can definitely be cached, but in a lot of cases they can also be entirely avoided if you add the exact functions you wanted to call rather than going through the API of setUniform that only allows a single value parameter

https://github.com/rdkcentral/Lightning/blob/master/src/renderer/webgl/shaders/RoundedRectangleShader.mjs#L145-L151
Those Float32Arrays are only necessary because of the setUniform API of currying the GL function to use only supports a single value for the glFunction.

If instead you had a setUniform[2,3,4]f, you could avoid all that heap allocation and just stack alloc making GC pressure a lot lower.

This largely doesn't matter until you have something animating on screen and then every rounded rect allocs 6 floats of garbage per frame.

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
None yet
Development

No branches or pull requests

4 participants