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

[DRAFT] thread safe-ish execution engine #487

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BadBastion
Copy link

@BadBastion BadBastion commented Apr 6, 2024

Draft for conversation, this is currently demonstration of what isn't a safe change.

ExecutionEngine Changes

ExecutionEngine<'ctx> -> ExecutionEngine

ExecutionEngine can have more than one module, and thus I believe more than one context. The functions that will break if context is dropped are all related to function execution. In principle this means we should try to ensure safety via JitFunction and not the lifetime of the ExecutionEngine.

JitFunction Changes

JitFunction<'ctx, F> -> JitFunction<F>

The JitFunction<'ctx, F> type is quite a challenging interface for inkwell due to the limited ways to fulfill this lifetime with safe rust. The idiomatic rust would be to change to JitFunction<F> and an Arc to both Execution Engine & Context.

This seems like the most elegant solution from the perspective of thread safe JIT functions, but incurs cascading breaking changes to the rest of the repo.

@BadBastion BadBastion marked this pull request as draft April 6, 2024 19:05
@BadBastion
Copy link
Author

Relevant to #242 and #28

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.

1 participant