ret_some_do! updating variables issue

[veniamin-ilmer]

Consider the following code:

use stakker::*;
use std::time::Instant;

struct Board;

impl Board {
  pub fn init(_: CX![]) -> Option<Self> {
    Some(Self)
  }

  pub fn get_byte(&self, _: CX![], ret: Ret<u8>) {
    ret!([ret], 123)
  }
}

struct ExampleStruct {
  value: u8,
}

fn main() {
  let mut stakker0 = Stakker::new(Instant::now());
  let stakker = &mut stakker0;

  let board = actor!(stakker, Board::init(), ret_nop!());

  let mut example_struct = ExampleStruct { value: 9 };

  let ret = ret_some_do!(move |result| {
    example_struct.value = result
  });
  call!([board], get_byte(ret));
  stakker.run(Instant::now(), false);

  println!("{}", example_struct.value);
}

It has the following compiler error:

error[E0596]: cannot borrow `cb` as mutable, as it is not declared as mutable
  --> src\main.rs:28:13
   |
28 |     let ret = ret_some_do!(move |result| {
   |  _____________^
29 | |     example_struct.value = result
30 | |   });
   | |    ^
   | |    |
   | |____cannot borrow as mutable
   |      help: consider changing this to be mutable: `mut cb`
   |

Is there a better way for me to do this? I like the potential use of ret_some_do! due to me avoiding the use of cx in my deeper modules which don't pass cx down...

[uazu]

There are several problems here. You have to consider that example_struct in the closure is a reference to a variable on the stack, i.e. there is a lifetime (call it 'a) associated with that pointer. However the closure held by Ret must be 'static, because it may be called back from anywhere. (Strictly speaking, we could specify that the Ret has a lifetime 'b that must not outlive the variable that it borrows from 'a, but adding lifetimes and carrying them around in the code would make Ret unusable.)

So the short version is that your approach is wrong and won't work.

In any case, to call ret! on a Ret doesn't need cx, so even in deeper modules, you can do a ret!, and the handler code will get a context anyway (because the handler normally defers to the queue and so is called from the main loop).

The whole point of actors in Stakker is to provide a context and state that can receive events. So the idea is that you would receive the "return" event in an actor (normally), unless you were trying to interface with some external code.

I suppose I should try and make it clearer. A Ret is a FnOnce that gets called synchronously when someone does a ret!. ret_some_do! is a direct interface to this. So the closure is called synchronously, but with no context at all, because it may be called deep in the code. So if you were to use ret_some_do! you'd normally need to have captured a Rc<Cell<...>> or similar in the closure to be able to access something else safely. So instead what the other ret_*! macros do is to capture a Deferrer and when called in that very limited context, just use the Deferrer to defer the work to the queue. Then when they are called from the queue, they have a full context and normal access to everything, so that an actor method can be called. That is the way around the problem of keeping Rust's borrow checker happy, but still allowing callbacks.

Anyway, you could get your example to work by putting ExampleStruct inside a Rc<Cell<...>>, but that won't help you much because when you are running in an actor, it would be much better just to put the ExampleStruct within the actor's struct, and then receive the Ret in an actor method.

If the problem is how to interface to the actor system from outside, then there is query! which does a synchronous call if the actor is Ready. But this is only for interfacing from outside.

[uazu]

Perhaps the confusion is due to you writing code outside of the main loop. Actually you don't have a main loop at all in your example. Really you need to create a System actor (or Universe or whatever), and start that up, and then run the main loop. Then within that System actor you start up all the boards and whatever. Then coding within that System actor, you could put your ExampleStruct within the System struct, and call get_byte from the System init method, and have another actor method that receives that return.

Maybe I have confused things by showing examples outside of the main loop, which is a context that isn't used much, except for interfacing with external code.

[uazu]

Here's a working example:

use stakker::*;
use std::time::{Duration, Instant};

struct Board;

impl Board {
    pub fn init(_: CX![]) -> Option<Self> {
        Some(Self)
    }

    pub fn get_byte(&self, _: CX![], ret: Ret<u8>) {
        ret!([ret], 123)
    }
}

struct ExampleStruct {
    value: u8,
}

struct System {
    board: ActorOwn<Board>,
    example_struct: ExampleStruct,
}

impl System {
    fn init(cx: CX![]) -> Option<Self> {
        let board = actor!(cx, Board::init(), ret_fail!(cx, "Board failed"));
        let example_struct = ExampleStruct { value: 9 };
        call!([board], get_byte(ret_some_to!([cx], got_value() as (u8))));
        Some(Self {
            board,
            example_struct,
        })
    }

    fn got_value(&mut self, cx: CX![], result: u8) {
        self.example_struct.value = result;
        println!("{}", result);
        stop!(cx);
    }
}

fn main() {
    let mut stakker0 = Stakker::new(Instant::now());
    let stakker = &mut stakker0;

    let system = actor!(stakker, System::init(), ret_shutdown!(stakker));

    stakker.run(Instant::now(), false);
    while stakker.not_shutdown() {
        let maxdur = stakker.next_wait_max(Instant::now(), Duration::from_secs(60), false);
        std::thread::sleep(maxdur);
        stakker.run(Instant::now(), false);
    }
}

I think the trouble is that all this is so obvious to me, having worked on it so long, I don't know how best to explain it to others.

[veniamin-ilmer]

Indeed, I was able to get the code to work by using get_some_to! instead of get_some_do!..

I was just looking at the use-case of get_some_do! to see if I can use it to update data.

I finished migrating my code from using multithreaded channel passing to using Stakker. It is working great.. All of the chips are running in sync, much better than I could ever hope from my multi threaded solution.. So thank you for your crate.

If you are interested by what I mean by code being run "deeper modules where I don't want to pass CX down", here is what I mean:

https://github.com/veniamin-ilmer/chip_actor/blob/main/src/chips/cpu8088/instructions/set.rs#L92

In order to use ret_some_to!, I had to pass down the Actor down to the sub module. Also, I wasn't able to declare the callback function with inside of the submodule, because all call! based functions require to have self as an attribute. So I had to declare the callback function in the higher level module, where the Actor was defined: https://github.com/veniamin-ilmer/chip_actor/blob/main/src/chips/cpu8088.rs#L69

[uazu]

I just got a chance to look through all this. Some comments:

• Your main loop will spin between timers. But maybe that's okay considering how responsive you need it to be. Maybe sleeping to wait for the next timer would be too much overhead (i.e. a syscall, relatively slow). Also it would be quite easy to make this run in virtual time instead, which would avoid the spinning, but that is only useful if you're planning to run a hosted program to completion and then stop.
• In your submodules, I see what you're saying. Those are really impl CPU {...}, but written out by hand. Doesn't Rust allow you do split an impl CPU {...} across files? I thought it did (although I haven't tried recently). But anyway, you could use the closure format of ret_some_to!([cpu_actor], |this, cx, ...| ...) to direct the call wherever you need.

I'm interested in what you're doing here because this is the first app I've seen written in Stakker that is CPU-bound instead of I/O-bound. So it could make a good benchmark for optimisation. For example if I improve timers, or add a small-Ret optimisation, that should show up well.

How is performance right now? Or do you not have enough running yet?

I think a reasonable benchmark would run a fixed 8088 program to completion in virtual time. Then I could profile it and see what could be improved.

[uazu]

There is a "discussion" section on github for Stakker that you can use for general discussion in the future, for anything that's not an issue. I'm interested in seeing how your project develops, and what opportunities it provides for optimisation of Stakker. Also, when you have an idea of what kind of performance you're getting out of it, it might be interesting for promoting Stakker. I remember some guy who was absolutely certain that tokio channels would beat my inter-actor calls. What can you do? I'm counting branch instructions, and they're doing cross-core synchronization ...

[uazu]

One more thought on your main loop. If your application gets scheduled out, then when it comes back, Instant::now() will have leapt forwards. This means that in your code where you do cx.now() + duration, that time will also have leapt forwards. So this breaks the simulation if you want it cycle-perfect. In a normal application, if you get behind (too much load), you want to be tolerant to that and degrade performance gracefully (for example, if a timer is called late, you set the new timer relative to cx.now() to not add further to the load). But you don't have to do that here.

What you can do is mix virtual time and real time. So if it gets behind it can run in virtual time until it catches up:

    stakker.run(now, false);
    while stakker.not_shutdown() {
        now += stakker.next_wait_max(now, Duration::from_secs(60), false);
        while now > Instant::now() {}  // Spin                                  
        stakker.run(now, false);
    }

If it never catches up (never spins), then I guess performance isn't good enough to run in real time, and it will just proceed in virtual time, doing the best it can. I guess you know your simulator better than I do, so it's up to you whether you think this is a good idea or not.

Edit: Sorry, got the now > Instant::now() condition the wrong way around.