Question: Do we need to pass Fiber.current to C 50 different times?

[joshgoebel]

trafficstars

I'm looking into cleaning up the Scheduler stuff further and it seems that the C code already knows the current fiber... we shouldn't need every single function preparing to suspend itself to pass Fiber.current down into C. We could either retrieve it by doing the traditional dance:

wrenGetVariable(vm, "core", "Fiber", 0);
wrenMakeCallHandle(vm, "current");
// or saving these references upfront

Or the much simpler:

WrenHandle* currentFiber(WrenVM* vm) {
  return wrenMakeHandle(vm, OBJ_VAL(vm->fiber));
}

Is there a good reason to prefer one over the other? Would simply calling vm->fiber() directly be frowned upon (private API or some such concerns?) I can't imagine this is something that would be changing super often in the implementation.

---

The below commit makes this change for just Timer:

https://github.com/joshgoebel/wren-cli/commit/6639dcfcdaca6427d4733f7b696d8e431eb5f65b

[ChayimFriedman2]

The second option is not an option because it involves implementation details. The first option is impossible because it requires reentrancy.

[joshgoebel]

The first option is impossible because it requires reentrancy.

Ah, duh. Right.

The second option is not an option because it involves implementation details.

I assume that you're speaking WRT to wren_cli only (correct me if I'm mistaken)... yet if someone wanted to fork the CLI - or do this in their own project or library it seems pretty safe overall, does it not? With the caveat that they were willing to flag it as a potential breakage point that needed to be tested whenever they upgraded to a newer version of Wren?

Is there anything truly unsafe about doing this that I'm missing - or is the problem just that "it could break" because you're technically using a private API? Where-as (eventually) we'd make guarantees about the public API.

[ChayimFriedman2]

"Unsafe" as in "memory safety"? No. But I wouldn't do that.

Another thing we can do, however, is to expose a wrenGetCurrentFiber() method in the API (because it's useful for schedulers in general).

[joshgoebel]

expose a wrenGetCurrentFiber() method in the API (because it's useful for schedulers in general).

I would agree. That was going to be my next question/suggestion. :)

Until then we could just do (branch updated):

  static await_(fn) {
    preserveFiberCurrent_(Fiber.current)
    fn.call()
    return Scheduler.runNextScheduled_()
  }

So no re-entrance issues, await just pushes the current fiber down into C directly so that it's always available on the C side... if you're using await_ it's all just magical.