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

Obviate 'static lifetime Clone requirements for operations #25

Merged
merged 10 commits into from
Dec 1, 2023

Conversation

cpubot
Copy link
Contributor

@cpubot cpubot commented Nov 30, 2023

Removing 'static and Clone requirements

The signature of Task has been updated to accept references to operations, rather than an owned value. This requires threading lifetimes through directive chains such that the compiler can guarantee the passed in operations live as long as the execution chain. The implication with respect to the public API is that operations must be passed as references to directive chainers like .map() and .fold(). Ultimately this means that directive implementations of Functor and Foldable no longer need to clone operations, and can simply hold a single immutable reference to operations through the duration of execution. This is a significant efficiency improvement, especially for large operation payloads and large input streams.

Reworking the derive procedural macro

A significant ergonomic improvement to the derive macro is included here. Previously, Operation implementors were required to maintain an enum of all Operation definitions to facilitate dynamic remote execution. This scheme has been superseded by a #[derive(RemoteExecute)] that can be derived directly on Operations. Operations annotated with #[derive(RemoteExecute)] will now be automatically collected at compile time into a contiguous section of the binary by the linker — no more manual maintenance of a comprehensive enum.

This change was precipitated by the aforementioned 'static and Clone requirements for Operation being dropped. With those requirements dropped, and Task requiring a lifetime annotation, the OpKind enum would have had to additionally define a lifetime annotated version with references to the internal operations.

For example:

struct MyOperation;

impl Operation for MyOperation {
    // ...
    type Kind = MyOps;
}

#[derive(OpKind)]
enum MyOps {
    MyOperation(MyOperation)
}

With Task requiring a reference to the OpKind, rather than an owned copy, an additional lifetime would need to be introduced onto Operation to support a variant of the MyOps containing references.

struct MyOperation;

impl<'a> Operation<'a> for MyOperation {
    // ...
    type KindRef = MyOpsRef<'a>;
    type Kind = MyOps;
}

#[derive(OpKind)]
enum MyOps {
    MyOperation(MyOperation)
}

#[derive(OpKind)]
enum MyOpsRef<'a> {
    MyOperation(&'a MyOperation)
}

Which was rather unfortunate for the public API. I took this as an opportunity to simplify the public API consumers. As such, the RemoteExecute trait was introduced.

pub trait RemoteExecute {
    const ID: u8;
}

This is accompanied by a derive macro, which generates a unique ID for each Operation deriving RemoteExecute. This macro additionally generates a unique execution function with types specialized to the given Operation. A pointer to this function is then inserted into a distributed_slice, indexed by the Operation's unique ID. With this scheme, Tasks can simply include the given Operation's ID in the payload, and the remote worker can use that to retrieve its associated execution function from the slice. This exhibits the same behavior of the previous OpKind enum construction without requiring such an enum to be maintained by the paladin user.

@cpubot cpubot force-pushed the zbrown/remove-static-lifetime-requirement branch from d3f4aed to 825a047 Compare November 30, 2023 15:52
@cpubot cpubot force-pushed the zbrown/remove-static-lifetime-requirement branch from 825a047 to eeac3c7 Compare November 30, 2023 16:03
@muursh muursh self-requested a review November 30, 2023 16:44
@cpubot cpubot merged commit 9b5ce7e into main Dec 1, 2023
2 checks passed
@cpubot cpubot deleted the zbrown/remove-static-lifetime-requirement branch December 1, 2023 05:09
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