-
Notifications
You must be signed in to change notification settings - Fork 22
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
struct FrameTileThreadData::lowest_pixel_mem
: Make into Vec
#878
Conversation
Also remove `FrameTileThreadData` because it only had a single field and was only used in one place. `lowest_pixel_mem` is now a field of `Rav1dFrameData`.
ef0fcf1
to
d393291
Compare
FrameTileThreadData::lowest_pixel_mem
: Make into VecFrameTileThreadData::lowest_pixel_mem
: Make into Vec
FrameTileThreadData::lowest_pixel_mem
: Make into Vec
struct FrameTileThreadData::lowest_pixel_mem
: Make into Vec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM besides using .resize_with
instead.
let mut lowest_pixel_ptr = f.tile_thread.lowest_pixel_mem; | ||
// TODO: Fallible allocation | ||
f.lowest_pixel_mem | ||
.resize(lowest_pixel_mem_sz as usize, Default::default()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.resize(lowest_pixel_mem_sz as usize, Default::default()); | |
.resize_with(lowest_pixel_mem_sz as usize, Default::default); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why though? What's the difference between these two? The data is Copy
so the end result is the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs recommend it for Default::default
and it optimizes a little bit better, since .resize
does a write for n - 1
, cloning them, and then writes the last one without the clone. .resize_with
just write all n
at once, so the codegen is better, and the source is a bit cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted. I'll use resize_with
going forward.
Also remove
FrameTileThreadData
because it only had a single field and was only used in one place.lowest_pixel_mem
is now a direct field ofRav1dFrameData
.@rinon let me know if this looks like it'll conflict with your changes. From what I saw it doesn't look like you changed
FrameTileThreadData
so I'm not expecting it to conflict, but we can wait to merge this until after your changes are merged if that'll help.FrameTileThreadData
: Make fields safe #712