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

make event.pull compatable with coroutines #2366

Closed
payonel opened this issue May 3, 2017 · 17 comments · Fixed by #2421
Closed

make event.pull compatable with coroutines #2366

payonel opened this issue May 3, 2017 · 17 comments · Fixed by #2421
Assignees

Comments

@payonel
Copy link
Member

payonel commented May 3, 2017

Apparently there are users that have had a very bad experience trying to use coroutines with openos.

My guess is that they are trying to call event.pull from a coroutine state, which (currently) will cause that coroutine to lock on a direct call to computer.pullSignal.

The solution will need to intercept event.pull calls to dispatch computer.pullSignal.

@payonel payonel self-assigned this May 3, 2017
@prozacgod
Copy link

I've come here twice trying to post a response, I keep screwing it up, and writing a long one... quick update, I'm looking into writing up my issues, and diving deeper into what that problem was.

I noticed on Reddit several quick replies that made me wonder if the responses were naive and I was blatantly wrong OR the issue was deeper than I they had expected.

It looks like the author of this post retracted their simpler solution,
https://www.reddit.com/r/feedthebeast/comments/68mrpr/computercraft_is_now_open_source/dh030hg/

I'll dive into it deeper this weekend, just wanted to at least say "I got the message and I'll be here"

@prozacgod
Copy link

I've been dissecting this issue, and I've formulated a number of things I think are issues, It took me a while to process and internalize a bunch of the code, including referring to bios.lua/machine.lua and I'm still missing chunks of knowledge. But this should be a good start.

Firing up a lua program that just does coroutine.yield() exists the program...

function main()
  a,b,c,d,e,f,g = coroutine.yield()
  print(a,b,c,d,e,f,g)
end

local status, err = pcall(main)

print("done", err)

I would expect this to yield up the chain of coroutines to the main coroutine, which would then resume the stack at some point with a relevant event/signal bubbled from the bios/machine/init coroutines.

function test()
  while true do
    event.pull...
    --check event for relevant info
  end
end

local co = coroutine.create(test)
coroutine.resume(co)
-- never gets here

event.pull does not yield from a coroutine instead it just goes on it's merry way, and that 'threadlet' is locked up.

AFAIK there's no way to create a timer event, I'm not sure I understand what event.timer is doing entirely, but it would be nice to just request a timer event to be fed to your thread (request passed to luaJ/java side of things) that you just get through the event coroutine.yield() mechansim

These impasses have made making coroutine based programs rather difficult, this was the deepest dive I took into it.

I have an example program I'll post here that I MADE work, by implementing the recommendation I said in the reddit thread (implementing my own threadlet microkernel that marshals events)

@prozacgod
Copy link

I created this gimmicky toy program with three threadlets, one that watches for key presses and exits itself, one that watches for the b key and prints bang randomly on the screen and one that is an event spy printing them on the screen on the bottom row.

https://gist.github.com/prozacgod/077aafa76f8ee7b786babe3ed710912d

The threadlet kernel respects the coroutine.yield()'s and just marshals to the event.pull command, rather naively - there is no error detection or reporting. error's will just trap silently and exit the program

@payonel
Copy link
Member Author

payonel commented May 24, 2017

Thanks for the update. I really appreciate the time you've donated to help me improve the mod.

There are points you make that I need to consider to improve openos, and there may be some points made that are by design. In the end, openos needs to provide a robust and usuable set of libraries for our users. So idealy, our "by design" is also intuitive and helpful. I do work to be open to feedback and suggestions.

the tldr:
Use coroutine.yield for coroutines
Use event.pull (or computer.pullSignal) for requesting events directly from the metal
It appears what we need is a coroutine-cooperative event.pull-like method.

Some comments on your explanation and code samples:

I would not recommend coroutine.yield() be used to acquire events, that's what computer.pullSignal is for! Code may be run inside of other user-made coroutines that may not respect "bubbled events". computer.pullSignal, conversely, is specifically designed to get events, regardless of which coroutine it is called from. It cuts right through, side-stepping any coroutines that may be in place. However, pullSignal is a blocking call and not compatible with waitForAny or waitForAll parallel designs.

However, openos' shell and the programs run there should respect bubbled events and yes, you've identified a bug here with this example.

event.pull is a computer.pullSignal wrapper to give you some filtering options. But, just like computer.pullSignal, it cuts right through and is not a coroutine.yield as far as user code is concerned. And, like computer.pullSignal, event.pull is a blocking call (depending on the filter options given, by default it blocks for 0 seconds). In your example, it didn't necessary go on its merry way, but rather, it "yielded" back to the machine(metal) each time you called it.

Your gist example of the "threadlets" is a good demonstration of using event.pull for events and coroutines you marshal to.

This is definitely enough information for me to work with. I'll give this some thought. openos already has a multithreading model that is not exposed in the api that I may leverage to provide the cooperation between coroutine yields and event pulling.

@payonel
Copy link
Member Author

payonel commented May 24, 2017

Also, can we discuss further what you mean by "AFAIK there's no way to create a timer event, I'm not sure I understand what event.timer is doing entirely"
event.timer: see http://ocdoc.cil.li/api:event for more info
You provide an interval and callback, and every interval seconds, your callback is called.

@prozacgod
Copy link

prozacgod commented May 24, 2017

I would not recommend coroutine.yield() be used to acquire events, that's what computer.pullSignal is for!

Indeed!!! I should have been clearer, I tried that to see if my assumptions were correct... BUT I have an argument for allowing it, it might go against the 'standards' imposed by OpenOS, in which case, is totally understandable. And computer.pullEvent makes plenty of sense 99.9999% of the time

For the event.timer I would expect, some low level "hardware" support - very similar to how ComputerCraft does it

you put in a request like...

local function sleep(time)
  local handle = computer.requestTimer(time)
  while true do
    evt, addr, h = computer.pullEvent('timer')
    if h == handle then
      break
    end
  end
  computer.cancelTimer(handle)
end

sleep(1)

Timer could/would be a derivative of the above code, just calling a callback.

This would then work within coroutines, or other tasks where you may request multiple timers at different intervals all being handled in one event loop as timer events are received. That's entirely a contrived example, I would expect a person to have written that to get laughed at, but there may be a reasonable use case.

Okay onto why coroutine.yield() maybe be allowed, and better still why computer.pullEvent() should not be low-level "hardware" access.

Composability of coroutines! One of my first novel programs in CC was making a multi-terminal system that could never have worked if coroutines were not composable in some form. I was able to intercept events, modify them to more accurately fit the environment I was providing and re-transmit them to the children.

Like touch events, if I can't intercept events I can't fix the offset of the touch event, and therefore cannot make child programs load in area on the screen, while still believing that they are in 'full control'

@prozacgod
Copy link

There's other issues with the last paragraph too. In CC I was able to overload the environment with setfenv, so the globals like term and such were easily overwritten and differences reconciled.

Obviously that's a big difference with Lua 5.1 and Lua 5.2 - in 5.2 you can "only" load an environment for a function that's compiled from a string. Which is a bit frustrating! But makes sense.

OpenComputers also has require which works wonderfully, please don't change that :P but if I wanted environmental changes...

Like If I wanted to write a multiple terminal window program, it seems... it might be impossible to do because no program would run within your window, unless you wrote it to support some new windowing api.

@prozacgod
Copy link

Oh BTW, event.timer with a callback doesn't release the callback when the program terminates! Old school DOS TSR style!

But I bet a solution that was coded that respected events from coroutines would just quit upon the program terminating. ;) (haha, j/k... sorta!)

@skyem123
Copy link
Contributor

@prozacgod, eh? All programs are loaded from a string, which is the contents of a .lua file. 😛

@prozacgod
Copy link

prozacgod commented May 24, 2017

ha, oh right - I mean... setfenv operated on an already created function instance. It allowed you to modify the code that was already inside your existing lua file. So you could modify the environment a function expected to see that was already in your code.

IIRC - One of the 'best practices' new (5.2) way to do something similar is to create a new function instance from a string directly, and pass in the environment/context.

f = load("print('')", {print=customPrint})

so this would create a new function instance, that has a custom environment containing only customPrint as the global print function.

whereas before ...

function printBlank()
  print('')
end

setfenv(printBlank, {print=customPrint}

It was maybe 'easier' to build composable systems with the setfenv mechanism. But loading a lua program from a file is fairly easy. It was just easier to maybe prototype things before.

@skyem123
Copy link
Contributor

IIRC, Lua 5.2 and up also have the _ENV variable, but I'm not sure exactly how that works... I think it clobbers every function's environment? http://lua-users.org/wiki/EnvironmentsTutorial I don't think you can do the same as setfenv() but it should be interesting.

@payonel
Copy link
Member Author

payonel commented May 26, 2017

@prozacgod e3ab7fc fixes your coroutine.yield example, it'll get the next event now
there is more work to be done for a parallel-like event.pull, but i have a good idea how i want to support it

@prozacgod
Copy link

prozacgod commented May 29, 2017

I decided to jump feet first into understanding more on how OC handles various events and the event module itself, now I get how timers are implemented, they're handled as a side-effect of general event timeouts. within the event scheduling mechanism.

It hurts my head! ha! ;) but "I get it" and perhaps with that mechanism asking for a 'hardware' timer interrupt might not be necessary. Although it would be closer to how hardware works.

I started moving my thread co-routine kernel into a module and implementing features on it (error handling), adding timers is a big one - the goal for me with this is not to build a 'competing' result, but to write more real-world tools and see where the cross-sections lie for improving what OC's OpenOS has.

Hopefully this will give you even more feedback. This has also let me understand how the event.timer function never lets go of a callback once registered.

Once a program loads up, it's subject to the Lua package cache - libraries are loaded once and only once(^1) since it's effectively a singleton, when you register your function to the handler the function you created is still an in-memory reference, and cannot be 'removed' as there's no "true process" concept, and you can't release resources. BUT I started pondering how that would look, and realized you'd probably quickly run out of memory for the Lua scripts, as everything would load their own private libraries... (time to implement shared libraries and copy-on-write kernel threading semantics... right!? ahhahah j/k)

@skyem123
Copy link
Contributor

It's possible to unload packages... package.loaded[name] = nil.
It's probably a bit of a hack, but from what I can tell it's sort of an official / used a lot hack.

@prozacgod
Copy link

I got a bit bored, so ... I'm going to live stream some of my random musings and experiments with this topic.... feel free to give me positive vibes :P haha

https://www.twitch.tv/prozacgod

@prozacgod
Copy link

The results of my musings

signals.lua

Which is a low level middle ground between "computer.pullSignal" and "event.pull", I wrote it using computer.pullSignal, but it works fine using event.pull, so it's a model that works within the current event model.

Has the ability to schedule future pseudo signals, which is what gives us schedule-able timers events

https://gist.github.com/prozacgod/ee9f67a1b8f9be4369c6d1cda7fe081e

Sorry if I'm dumping to much junk into here, but I figured this might be useful to what you'd want to do with event.lua

The video is pretty nice so you can follow my thought process.

@payonel
Copy link
Member Author

payonel commented Jun 11, 2017

new wiki page documented the new thread api: http://ocdoc.cil.li/api:thread

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

Successfully merging a pull request may close this issue.

3 participants