Optimising issue from copying

help
1

I'm having issues because I want to borrow &self.instructions which is a Vec<u64> at the same time as needing a mutable reference to self. I've been copying instructions, but the performance hit is too large. Standard borrowing issue right? But I can't solve it, I can't put the data I need to mutate in a RefCell because of Godot, as it's a field called base that attributes need to make the method self.base_mut(). The point is that I really need that mutable reference to self. I thought of giving the instructions to the struct every time they need them via a callback from another struct but that didn't work either because it requires a reference to the struct you're calling back to, and references in fields need lifetimes and Godot classes (structs) can't have lifetimes. My whole project is here and the borrowing issues occur in lib.rs in the spell_virtual_machine, ready and physics_process methods. In short I'm doing this and want to get rid of the clone while still allowing spell_virtual_machine to use a mutable reference:

self.spell_virtual_machine(&self.ready_instructions.clone())

A few things to clarify:

  • This is a library
  • The code that calls this library isn't rust
  • All instances of the struct I've been talking about could be made by another struct
2

spell_virtual_machine is an internal-only function, so the simplest way I can see to make this work is to change its type slightly:

    fn spell_virtual_machine(&mut self, instructions: Option<&[u64]>) -> Result<(), &'static str> {
        let mut instructions_iter = instructions.map(|i| i.iter()).unwrap_or(self.ready_instructions.iter());
        while let Some(&bits) = instructions_iter.next() {

You may need to play with the unwrapping of the Option to get the types right.

1 Like
3

This is a workaround that uses std::mem::take to temporarily replace the ready_instructions Vec with an empty Vec, so it can be passed separately by reference to spell_virtual_machine. This avoids passing a mut and non-mut self ref to spell_virtual_machine. There is no extra allocation, since an empty Vec does not allocate, so this is a cheap (if somewhat messy looking) solution.

Notice that I tried to carefully restore the ready_instructions field before doing anything else, to ensure that it isn't accidentally used while it is empty.

        let spell_result = {
            let instr = std::mem::take(&mut self.ready_instructions);
            let result = self.spell_virtual_machine(&instr);
            self.ready_instructions = instr;
            result
        };
        match spell_result {

Edit: Since ready_instructions is left empty while running spell_virtual_machine, which could cause problems if it is used by that function (now or later), @farnz 's solution is probably better.

2 Likes
4

If you wanted to make your thing work, I'd make use of the "drop guard pattern":

let spell_result = {
    struct Instrs<'this> {
        ready_instrs: &'this mut Vec<u64>,
        instrs: Vec<u64>,
    };

   impl<'this> Drop for Instrs<'this> {
       fn drop(&mut self) {
           std::mem::swap(self.ready_instrs, &mut self.instrs);
        }
    }

    let instrs = std::mem::take(&mut self.ready_instructions);
    let guard = Instrs { ready_instrs: &mut self.ready_instructions, instrs };
    self.spell_virtual_machine(&guard.instrs)
}

(untested, could have bugs).

The idea is that struct Instrs has a Drop implementation that will restore self.ready_instructions for you; either you abort in self.spell_virtual_machine, or drop glue will make sure that things get restored to the right way round and execution continues (either continuing the panic unwind, or normal execution).

4 Likes
5

Thank you both @farnz and @jumpnbrownweasel. I got it working with @jumpnbrownweasel's solution, It didn't work with @farnz's one as it still had borrowing issues, but that very well could have been because I didn't know what I was doing. I couldn't see how it would avoid borrowing rules as the line just seemed to me as a different way of making the instruction_iter, but I'm no professional, so I'm assuming I just don't understand. I may also look into the "drop guard pattern" as @farnz said, but it seemed to work without it either way, as I never need to access self.ready_instructions while spell_virtual_machine is running except for at the beginning.

2 Likes