-
Notifications
You must be signed in to change notification settings - Fork 51
Attempt Polymorphization #87
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
Conversation
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.
Looks good to me overall. Sorry for the delay.
Do we have evidence that this actually reduces polymorphization in practice? Like, if we compile Bevy before and after this patch, how much does the code size decrease.
src/raw.rs
Outdated
|
||
// Finally, deallocate the memory reserved by the task. | ||
alloc::alloc::dealloc(ptr as *mut u8, task_layout.layout); | ||
fn header<'a>(ptr: *const ()) -> &'a Header { |
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.
Mark this as unsafe
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.
Just went ahead and deleted the function. It's was only used in one location.
@notgull there is the (admittedly artificial) benchmark results seen here: #66 (comment). Bevy actually uses surprisingly few new instances of tasks (generally not used in generic code), and has other compilation bottlenecks. |
b05e31d
to
c2ee67a
Compare
Went ahead and reverted the |
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.
Looks good to me!
Partially fixes #78.
This attempts to minimize the amount of monomorphized codgen created while using this crate.
Header<M>
has been split intoHeader
andHeaderWithMetadata<M>
, usingrepr(C)
to keep pointers trivially castable between each.Header
type.TaskVtable
to operate without every generic argument onRawTask
, ,just the ones they need.get_output
a function on the vtable itself instead of put in the vtable, both avoids an extra dynamic dispatch call, and shrinks the vtable.#[inline(never)]
on these polymorphized functions.This PR also shifts to use
byte_add
, which does bump the MSRV to 1.75. If this is too much of a bump for such a gain, I can revert that.Benchmarks: