Skip to content
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

Let the compiler do memory layout computation for tasks at compile time #27

Conversation

michaelwoerister
Copy link
Contributor

Let the compiler do memory layout computation for tasks at compile time instead of doing them manually at runtime.

Also note that it is advantageous for (crash dump) debugging to have an actual type that defines the layout, because that type will be described in debuginfo, which in turn allows debuggers to decode a task's raw memory blob. Without that type definiton, we cannot compute task layouts at runtime because debuginfo does not contain information about type alignments on all platforms.

Being able to decode raw task memory in debuggers is a prerequisite for eventually implementing logical stack traces.

@michaelwoerister
Copy link
Contributor Author

I'm not sure this PR changes something that would make those MIRI tests fail.

I'll look into Rust 1.39 support...

@taiki-e
Copy link
Collaborator

taiki-e commented Jul 1, 2022

Miri test fail is unrelated and will be fixed by matklad/once_cell#185.
Until that is fixed, -Zmiri-strict-provenance should be removed from MIRIFLAGS.

MIRIFLAGS: -Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-ignore-leaks -Zmiri-preemption-rate=0

As for MSRV, dropping Rust 1.39 support is fine, but there are currently no plans to drop support for Rust 1.48 used by Debian stable. See also #25.

@michaelwoerister
Copy link
Contributor Author

As for MSRV, dropping Rust 1.39 support is fine, but there are currently no plans to drop support for Rust 1.48 used by Debian stable.

It looks like unions with non-copy fields were stabilized in 1.49. So close!

I'll see if I can find another way to do this.

@notgull
Copy link
Member

notgull commented Jul 1, 2022

Are there any downsides to using an enum in this case instead of a union? enum adds another byte to memory, but the branch prediction can be effectively eliminated through unreachable_unchecked() and it doesn't lead to an MSRV bump.

@michaelwoerister
Copy link
Contributor Author

I couldn't really get this to work in any satisfactory way. Miri rightfully kept complaining that taking the address of a field in potentially uninitialized memory is UB.

I did get things to pass Miri at https://github.com/michaelwoerister/async-task/tree/raw-task-layout-experiment, and that seemed to also be a bit faster (15 instead of 19 nano seconds for task_create, 21 instead 23 nanoseconds for task_run) -- but it's still toying around way too much with raw pointers and unsafe code for my taste.

I'll open an alternative PR that stores the minimal needed information for logical stack traces in the TaskVTable. You can let me know if that seems acceptable there :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants