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

improvement(runtime): add benchmarks for and optimise struct field marshalling #238

Merged
merged 5 commits into from
Jul 25, 2020

Conversation

Wodann
Copy link
Collaborator

@Wodann Wodann commented Jun 30, 2020

This PR resolves issue #224, while introducing several benchmarks for tracking marshalling performance. There are two new benchmarks:

  • retrieving data from a struct field
  • writing data to a struct field

Both benchmarks are tested for:

  • Rust fundamental type (f32)
  • Mun fundamental type (f32)
  • Rust shared reference to struct
  • Rust clone of struct
  • Mun garbage collected struct
  • Mun value struct

Based on profiling two optimisations were made:

  1. cache hashed values for constants (in the future these should use const fns)
  2. Store StructRef with a shared reference to the Runtime instead of a Rc<RefCell<Runtime>>

Especially the second optimisation is more involved, but results in the biggest performance gains:
image
image

Optimisation 2 has the added side-effect that marshalled structs are never rooted. You have to manually call the root function if you want to hold on to the StructRef. This turns the StructRef into a RootedStruct. Usage would look like this:

fn main() {
    let lib_dir = env::args().nth(1).expect("Expected path to a Mun library.");
    let runtime = RuntimeBuilder::new(lib_dir)
        .insert_fn("log_f32", log_f32 as extern "C" fn(f32))
        .spawn()
        .expect("Failed to spawn Runtime");

    let ctx = {
        let runtime_ref = runtime.borrow();
        let ctx: StructRef = invoke_fn!(runtime_ref, "new_sim").unwrap();
        ctx.root(runtime.clone())
    };

    loop {
        {
            let runtime_ref = runtime.borrow();
            let _: () = invoke_fn!(runtime_ref, "sim_update", unsafe { ctx.as_ref(&runtime_ref) }, elapsed_secs).unwrap();
        }

        runtime.borrow_mut().update();
    }
}

To avoid unsafe code, there is also a safe by_ref function.

Caveats

  • Currently there is no ergonomic way of calling retry or wait on the result of the invoke_fn! macro. This will have to be re-added in an alternative format.

Future Work

  • Allocate struct(value) instances in a memory arena when they are marshalled - for further optimisation.
  • We can introduce a safe variant of as_ref by including a token in Runtime that we can check to ensure that we are dealing with the same runtime that allocated this RootedStruct. Its signature would look be pub fn as_ref<'r>(&self, &'r Runtime) -> Option<StructRef<'r>>. This might remove the need for by_ref.

@Wodann Wodann added the type: perf Changes that improve performance label Jun 30, 2020
@Wodann Wodann added this to the Mun v0.3.0 milestone Jun 30, 2020
@Wodann Wodann requested a review from baszalmstra June 30, 2020 14:34
@Wodann Wodann self-assigned this Jun 30, 2020
@Wodann Wodann changed the title Improve/struct marshalling perf improvement(runtime): add benchmarks for and optimise struct field marshalling Jun 30, 2020
@codecov
Copy link

codecov bot commented Jun 30, 2020

Codecov Report

Merging #238 into master will increase coverage by 0.18%.
The diff coverage is 92.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #238      +/-   ##
==========================================
+ Coverage   78.91%   79.09%   +0.18%     
==========================================
  Files         201      200       -1     
  Lines       12667    12749      +82     
==========================================
+ Hits         9996    10084      +88     
+ Misses       2671     2665       -6     
Impacted Files Coverage Δ
crates/mun/src/ops/start.rs 0.00% <0.00%> (ø)
crates/mun_runtime/src/lib.rs 72.63% <ø> (ø)
crates/mun_runtime/src/reflection.rs 56.36% <58.82%> (+2.01%) ⬆️
crates/mun_runtime/src/adt.rs 83.33% <80.32%> (ø)
crates/mun/tests/integration.rs 100.00% <100.00%> (ø)
crates/mun_runtime/src/macros.rs 76.08% <100.00%> (+5.17%) ⬆️
crates/mun_runtime/tests/hot_reloading.rs 100.00% <100.00%> (ø)
crates/mun_runtime/tests/marshalling.rs 99.00% <100.00%> (+0.02%) ⬆️
crates/mun_runtime/tests/memory.rs 100.00% <100.00%> (ø)
crates/mun_runtime/tests/util.rs 83.63% <100.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 415bbbf...9842ee1. Read the comment docs.

@Wodann Wodann force-pushed the improve/struct-marshalling-perf branch from e7282c0 to 0152450 Compare July 1, 2020 21:39
Copy link
Collaborator

@baszalmstra baszalmstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool! But wow this involves a lot of lifetimes!

Does this mean that the wait function doesnt exist anymore? Does that mean that if you make an error in you Mun code you can only really unwrap? Do you already have a proposal on how to handle errors in Mun code?

My previous solution (with Lua) was to stop calling into Lua altogether until a reload occured. This is a more global solution where a reload could happen every frame instead of waiting for changes by blocking the entire application.

Something like this perhaps? (Im just spitting out a random idea)

let wrapped_runtime:SafeRuntime // or whatever
// dont call this method if the last execution errored out.
let value:Some<i32> = wrapped_runtime.with_runtime(|borrowed| {
  // do stuff with the borrowed runtime here
  return 0; // Returned value
});

somewhere else (at the beginning of a frame or whatever)

wrapped_runtime.update()

crates/mun_runtime/src/reflection.rs Outdated Show resolved Hide resolved
@Wodann Wodann force-pushed the improve/struct-marshalling-perf branch 2 times, most recently from 6f33cf0 to 16d5812 Compare July 17, 2020 12:38
Mun structs are marshalled as a StructRef. To make them rooted, they
need to be converted to a RootedStruct. This avoids needing to reacquire
the shared reference when getting, setting, and replacing struct fields
@Wodann Wodann force-pushed the improve/struct-marshalling-perf branch from 16d5812 to 9842ee1 Compare July 21, 2020 13:13
@baszalmstra baszalmstra merged commit aef4492 into mun-lang:master Jul 25, 2020
@Wodann Wodann deleted the improve/struct-marshalling-perf branch August 7, 2021 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: perf Changes that improve performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants