hawkw / mycelium

🍄 an alleged 'operating system'
https://mycelium.elizas.website
MIT License
540 stars 19 forks source link

maitake: replace `task::Cell` with a union #230

Open hawkw opened 2 years ago

hawkw commented 2 years ago

currently, maitake tasks use an enum to represent the state of the user-provided task data (either the future, if the task has yet to complete, or the task's output, when it has completed): https://github.com/hawkw/mycelium/blob/619565ce10255af346a9a153407784a4d872f91f/maitake/src/task.rs#L158-L161

the task's state is currently tracked by both the enum descriminant, and the COMPLETED bit in the task's atomic state word: https://github.com/hawkw/mycelium/blob/619565ce10255af346a9a153407784a4d872f91f/maitake/src/task/state.rs#L19-L24

this means that we have what's essentially an unnecessary word[^1] of data for the enum descriminant, because whether the task cell contains the future or its output is also tracked by the task's StateCell. @jamesmunns mentioned wanting to replace this enum with a union --- at the cost of some additional unsafe code[^2], we could shave a word off of the Task's memory usage. this might be particularly useful on memory-constrained devices.

[^1]: i'm assuming it's a whole word for alignment reasons? [^2]: or...maybe not; i think everywhere the task cell is touched is already inside of an unsafe block, because it lives in an UnsafeCell...

hawkw commented 2 years ago

this may make the task Drop impl a little bit annoyinger because the future and/or the output type may have their own destructors that need to be run...

jamesmunns commented 2 years ago

If you're feeling really spicy, we can likely use my absurd allocator ideas to do really invasive optimizations.

Like if the future has completed, it never needs to go back in the run queue, right?

hawkw commented 2 years ago

If you're feeling really spicy, we can likely use my absurd allocator ideas to do really invasive optimizations.

Like if the future has completed, it never needs to go back in the run queue, right?

yeah...although, i like having the task's state word immediately after the MPSC pointers that have to be the first field, so that it's prefetched in cache when a ptr to a task is dereferenced (on x86). and, i would imagine that futures are almost always larger than their outputs[^1], so i'm not sure if we would actually be saving space by putting the run queue links in the union?

[^1]: although i can't back this up with actual data, i would imagine that any async block-generated future will at least always be large enough to contain the output type...