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

Improve speed by moving function collections to stack arrays #12

Open
TheDan64 opened this issue Sep 9, 2017 · 6 comments
Open

Improve speed by moving function collections to stack arrays #12

TheDan64 opened this issue Sep 9, 2017 · 6 comments

Comments

@TheDan64
Copy link
Owner

TheDan64 commented Sep 9, 2017

A common pattern being used looks something like this:

fn inkwell_does_stuff(input: &[&InkwellValue]) {
    let mut input = Vec<LLVMValueRef> = input.iter().map(|val| val.as_value_ref()).collect(); 

    // call LLVM function with raw ptr to input
}

What we're basically doing is taking a bunch of our own types and mapping them to a sequence of raw llvm pointers. The problem is that Vec will allocate on the heap, but this sequence is only needed for the scope of the function and never gets directly returned. So, I think we can improve this pattern by using something like arrayvec which stores the contents on the stack. This should also work really well because we never modify the size of these vectors, just use them as an intermediate data location for LLVM to read from.

Alternatively, if there's a way to just collect into a a stack slice, that would work too.

This might make for a good first PR if anyone's interested since it doesn't require technical knowledge of LLVM, just Rust.

@TheDan64
Copy link
Owner Author

TheDan64 commented Sep 11, 2017

This might not actually be possible with modern rust. Apparently it might require VLA (variable length arrays), which do not exist today because they're easy to blow through the stack with. Will remove the milestone but keep the issue open for future review.

Arrayvec could work today if we specify a compile time upper bound, but I'm not sure we want to do that because it seems like an artificial limitation to our users.

@TheDan64 TheDan64 removed this from the 0.2.0 milestone Sep 11, 2017
@TheDan64
Copy link
Owner Author

TheDan64 commented Sep 11, 2017

Related RFC issue: rust-lang/rfcs#618

@TheDan64
Copy link
Owner Author

TheDan64 commented May 9, 2018

If we don't have rust support for this (and this does indeed turn out to be a significant speed issue - which it may not) we could add some sort of scoped helper method which takes a fn and calls it on a C alloca/malloca ptr (possibly transmuting into a reference since we know the lifetime is valid for the scope - though the data might be uninitialized so the fn should be unsafe). Bonus points if the helper method can make os calls to check that there's enough remaining stack space to make the stack allocation, returning a Result/Err if not. Though it's worth noting there are possible issues when inlining functions which use alloca since the effective lifetime is extended to a larger scope. Maybe this is worth exploring in a separate crate: alloca/scoped_alloca both seem like available names

@TheDan64
Copy link
Owner Author

rust-lang/rust#48055 might be a better approach to this

@TheDan64
Copy link
Owner Author

TheDan64 commented Aug 8, 2019

@TheDan64
Copy link
Owner Author

TheDan64 commented Aug 8, 2019

Added experimental feature flag for testing this: 673a3d5

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

No branches or pull requests

1 participant