Skip to content

Conversation

@DaniPopes
Copy link
Collaborator

@DaniPopes DaniPopes commented Jul 23, 2025

I believe this change was made to reduce memory consumption, however the initial problem was a memory leak which was fixed. So I don't think this is necessary anymore, and it should slightly improve performance because of cache locality of the frames inside of the Vec, as well as removing an initial allocation when creating the frame.

@codspeed-hq
Copy link

codspeed-hq bot commented Jul 23, 2025

CodSpeed Performance Report

Merging #2761 will not alter performance

Comparing DaniPopes:frame-unbox (f10d438) with main (549ca96)

Summary

✅ 171 untouched benchmarks

@DaniPopes DaniPopes marked this pull request as ready for review July 23, 2025 10:49
@DaniPopes DaniPopes requested a review from rakita July 23, 2025 10:49
Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, allocating multiple frames at once (as it is one vec) will be slightly faster when the frame is used for the first time.

@rakita rakita merged commit f501d00 into bluealloy:main Jul 23, 2025
29 checks passed
This was referenced Jul 23, 2025
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

Successfully merging this pull request may close these issues.

2 participants