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

[no sq] Generic IPC mechanism between Lua envs #14659

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sfan5
Copy link
Member

@sfan5 sfan5 commented May 14, 2024

the first two commits are totally unrelated but this is a package deal, sorry.

To do

This PR is ready.

TODO:

  • CAS primitive
  • Polling function that blocks

How to test

  1. read and verify the unittests
  2. the forbidden ping-pong:
-- Note: this will just not work if you have less than 2 async workers.
-- It also hangs the server at shutdown but that's expected.

local function fa()
	while true do
		core.ipc_poll("btoa", 60 * 1000)
		local n = core.ipc_get("btoa")
		core.ipc_set("btoa", nil)
		print("a:", n)
		core.ipc_poll("", 100) -- sleep
		core.ipc_set("atob", n + 1)
	end
end
local function fb()
	while true do
		core.ipc_poll("atob", 60 * 1000)
		local n = core.ipc_get("atob")
		core.ipc_set("atob", nil)
		print("b:", n)
		core.ipc_poll("", 200) -- sleep
		core.ipc_set("btoa", n + 1)
	end
end
core.handle_async(fa, function() end)
core.handle_async(fb, function() end)
core.ipc_set("atob", 0)
  1. (optional) invent your own code

@sfan5 sfan5 added @ Script API WIP The PR is still being worked on by its author and not ready yet. Feature ✨ PRs that add or enhance a feature labels May 14, 2024
@sfan5 sfan5 linked an issue May 14, 2024 that may be closed by this pull request
Copy link
Member

@grorp grorp left a comment

Choose a reason for hiding this comment

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

only looked at the documentation

doc/lua_api.md Outdated Show resolved Hide resolved
doc/lua_api.md Outdated Show resolved Hide resolved
@Kimapr
Copy link

Kimapr commented May 19, 2024

I think channels, as done in love2d would be preferable to this.

@sfan5
Copy link
Member Author

sfan5 commented May 22, 2024

Channels are definitely more flexible, but there are two reasons that made me choose this approach:

  • The main usecase (also mentioned in the OP issue) is sharing some more or less static data with other environments. This is possible with channels but requires more setup/work.
  • Minetest does not allow arbitrary thread spawning, and you have to make do with the ones that exist. Except for the emerge thread none of these should be blocked for extended periods of time. This reduces the usefulness of channels that can be blocked on.

@BuckarooBanzay
Copy link
Contributor

nice work 👍

do i get this right: this is basically a persistent and global key/value store and i have to make sure the items don't pile up and fill my memory (as a modder)?

@sfan5
Copy link
Member Author

sfan5 commented May 23, 2024

do i get this right: this is basically a persistent and global key/value store and i have to make sure the items don't pile up and fill my memory (as a modder)?

Correct.

@Kimapr
Copy link

Kimapr commented May 24, 2024

Minetest does not allow arbitrary thread spawning, and you have to make do with the ones that exist. Except for the emerge thread none of these should be blocked for extended periods of time. This reduces the usefulness of channels that can be blocked on.

What about the async environment?

For my usecase I want a long-running task in its own thread. This is so I can run user-provided code in my (yet to be made, though i have implemented most of the VM for my fictional CPU architecture in Lua already) computer mod without blocking the main thread. The main thread and the code execution thread would communicate with each other by sending messages, where the main thread would send events to respond to and the VM in the execution thread would send actions it wants to perform in the world. Without an IPC mechanism i would have to serialize, send the entire VM state back and forth and reinitalize it on every event or action, which might be far too much overhead for this to be practical.

The main usecase (also mentioned in the OP issue) is sharing some more or less static data with other environments. This is possible with channels but requires more setup/work.

If the data is static, no IPC mechanism is needed. Something similar to the way its done in async env could be done for mapgen env, allowing modders to pass initial values. If more communication is needed, pass a channel to the env. Peekable channels could be used to store values.

@sfan5
Copy link
Member Author

sfan5 commented May 24, 2024

What about the async environment?

There is a fixed maximum amount of async workers, so if you block one of them you risk stalling all other jobs.

If the data is static, no IPC mechanism is needed. Something similar to the way its done in async env could be done for mapgen env, allowing modders to pass initial values.

There is in fact no good way to pass static data into the async env. You can pass data together with every job, but then you pay the transfer cost every time.
I considered implementing a mechanism for one-way static data copies from the main -> mapgen env, but a shared data table like it is now is bother simpler and more powerful.

Regardless in #14451 OP requested the ability to change this data at runtime. So it's not really static.

@Kimapr
Copy link

Kimapr commented May 24, 2024

There is a fixed maximum amount of async workers, so if you block one of them you risk stalling all other jobs.

I think this limit should be removed. It's not even mentioned in the docs. Blocking only one of them also shouldn't stall anything as the work stealing algorithm will distribute the jobs among remaining workers.

@sfan5
Copy link
Member Author

sfan5 commented May 28, 2024

If the maximum of worker threads is 1, then blocking a single one will block everything.
A generalized thread API could definitely be added one day but I think "repurposing" the async jobs for this is wrong.


Due to lack of usecase I have decided to not implement a polling function. This PR is now ready.

@sfan5 sfan5 removed the WIP The PR is still being worked on by its author and not ready yet. label May 28, 2024
@Kimapr
Copy link

Kimapr commented May 28, 2024

Due to lack of usecase I have decided to not implement a polling function. This PR is now ready.

:/

@sfan5
Copy link
Member Author

sfan5 commented May 28, 2024

I changed my mind. People will find uses for it when it exists.

@sfan5 sfan5 force-pushed the ipc-our-saviour branch 2 times, most recently from 891ecbe to 7be58d9 Compare May 28, 2024 21:38
@Desour
Copy link
Member

Desour commented May 29, 2024

It might also be useful to have an async polling function with a callback. With this one could do the same as checking a value in each server-step, but with less latency:

core.ipc_set("mymod:please_write_it")
-- wait for value
local function wait_for_it()
  local val = core.ipc_get("mymod:val")
  if val == nil then
-   core.after(0, wait_for_it) -- not called until next step
+   core.poll_async("mymod:val", nil, wait_for_it) -- called as soon as possible. (nil given for comparison, similar to futex API)
  else
    print(val)
  end
end
wait_for_it()

(Mods can also use it for synchronous polling by yielding a coroutine.)

@sfan5
Copy link
Member Author

sfan5 commented May 29, 2024

It might also be useful to have an async polling function with a callback

This would not share any code with the blocking implementation and have no advantages over manually polling with core.after(0, ...).

@sfan5 sfan5 force-pushed the ipc-our-saviour branch 2 times, most recently from 8817b1b to b2b25e7 Compare May 29, 2024 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature ✨ PRs that add or enhance a feature @ Script API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow passing arbitrary data to mapgen Lua env (also at runtime)
5 participants