Skip to content

Use stack instead of heap in AbstractYuyvBufferDecoder::component_indexes#114251

Open
iker-sr wants to merge 1 commit intogodotengine:masterfrom
iker-sr:component_indexes_stack_fix
Open

Use stack instead of heap in AbstractYuyvBufferDecoder::component_indexes#114251
iker-sr wants to merge 1 commit intogodotengine:masterfrom
iker-sr:component_indexes_stack_fix

Conversation

@iker-sr
Copy link
Copy Markdown

@iker-sr iker-sr commented Dec 20, 2025

Replaces the use of new[] and delete[] with the use of the stack, because the size is constant.

@Calinou
Copy link
Copy Markdown
Member

Calinou commented Dec 21, 2025

Thanks for opening a pull request 🙂

Can you provide before/after benchmarks that show a performance difference? See Optimization guidelines for more information.

If this PR is meant to improve things other than performance, please state so in the PR description.

@clayjohn
Copy link
Copy Markdown
Member

I would say this PR is a net improvement for 2 reasons:

  1. We shouldn't be using new/delete whenever we can avoid it since it avoids our memory allocator entirely and puts us at the mercy of the OS/runtime
  2. It follows our goal of having 0 run time allocations

Ultimately this will never make a noticeable performance difference since the code only runs when the camera feed is activated. Even if it did take a few ms, it would be hidden by the activation of the camera.

That being said, I think it is important to avoid new/delete so that this code doesn't get copied and spread elsewhere since this is a dangerous pattern. Some code patterns are best to simply avoid in new code, but this is one that I think is worth removing from any place we spot it since if we accumulate a lot of new/deletes we will create a huge future headache.

@clayjohn
Copy link
Copy Markdown
Member

Looks great! The final step before merging is to squash all the commits together so that the whole PR only contains 1 big commit with all your changes. We like to merge one commit at a time to keep the git history clean and navigable. It looks like you accidentally added a merge commit on top of this while rebasing

If you don't know how to do that, we have a helpful tutorial in the official documentation https://docs.godotengine.org/en/latest/community/contributing/pr_workflow.html#the-interactive-rebase

@clayjohn clayjohn modified the milestones: 4.x, 4.7 Dec 30, 2025
@iker-sr iker-sr force-pushed the component_indexes_stack_fix branch from 3ce855c to ae3ad75 Compare January 12, 2026 21:29
@iker-sr iker-sr marked this pull request as draft January 12, 2026 21:42
@iker-sr iker-sr marked this pull request as ready for review January 12, 2026 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants