Time Nick Message 09:17 MTDiscord is it possible for the engine to run more than one lua vm instance at the same time? 09:26 pmp-p everything is possible, why would you need that specifically ? 09:26 rubenwardy Yes - it already runs an instance for the client and the server 09:27 rubenwardy Although, these are Lua states - not sure if they share stuff somewhere 11:33 pgimeno I think Lua states are totally isolated. That's how Löve implements threads. 11:35 pgimeno Instead of a GIL, it has a separate state for each thread, and no locking. 11:36 pgimeno And communication channels for sharing data, of course. 11:37 rubenwardy Yeah that makes sense 11:40 pgimeno (it would have been nice if Minetest's Lua threads were that way, but that ship sailed long ago) 11:45 sfan5 what are "Minetest's lua threads"? 11:46 sfan5 you mean mods could/should have been isolated? 12:35 pgimeno I mean the Lua mapgen code should run in an isolated Lua instance 12:38 sfan5 true 12:51 rubenwardy yeah, I think there's an issue for that 12:52 pgimeno that's a very breaking change though; every Lua mapgen would need to be rewritten 12:53 rubenwardy not necessarily breaking 12:53 rubenwardy you can have a different mapgen callback 12:53 rubenwardy old mods would do it the slow way after newer mods 12:54 rubenwardy also, the mapgen thread could communicate more with the mapgen itself 12:54 pgimeno hmm, sounds like hope :) 13:09 MTDiscord Doesn't https://github.com/minetest/minetest/pull/10723 partly do the same as on_death()? 13:10 MTDiscord I think on_deactivate() should really only be called when the entity unloads not when it 'dies' unless I misunderstand something there 13:12 rubenwardy it should on_death() and then on_deactivate() 13:12 rubenwardy when dying 13:17 MTDiscord But then 'deactivate' sounds wrong...should be 'on_remove' or so 13:17 MTDiscord Cause it's removed from the map...fits both cases 13:18 sfan5 but it's not permanently removed 13:18 rubenwardy they're the same thing 13:19 rubenwardy well, you need to deactivate to remove 13:22 MTDiscord sfan5, it can be permanently removed that is what this convo is all about...it gets called in both cases...no matter if it's just unloaded or entirely deleted 13:23 sfan5 right, I misunderstood your suggestion 13:23 MTDiscord So if you insist on keeping that name it should be documented well enough that this also gets called when the entity is destroyed/deleted 13:24 sfan5 not sure if that's a good idea, if mods have to do some cleanup when an entity disappears it'd be easier if they could reply on on_deactivate() always being acalled 13:24 sfan5 called* 13:24 sfan5 rely* 13:26 MTDiscord If I understood it correctly it gets called in every case but maybe there are mods will would want a callback that gets only called when the object gets unloaded and not entirely deleted and for such a case the name could be misleading 13:29 MTDiscord And the documentation for on_death() is also very sparse...like does it only get called if you punch the entity and its HP reach 0? Then that would be kind of useless cause many mods implement their own HP systems with entities...or does it also get called after an 'object:remove()'? 13:36 rubenwardy those mods can call on_death themselves 14:10 pgimeno rubenwardy: hm, on second thought, you can't disable the GIL if you want compatibility with the old code. I'm not sure if there can be a separate GIL per Lua state. 14:10 rubenwardy I don't see how that matters 14:10 rubenwardy they're be different callbacks 14:10 rubenwardy the new thread would run a callback first 14:11 rubenwardy then the MapBlock would be sent to the server thread, and any old mods would run callbacks then 14:11 pgimeno if one Lua thread locks another Lua thread, there's nothing to gain from separate Lua instances 14:11 rubenwardy why would they lock another thread? 14:11 pgimeno because the GIL is Global 14:12 rubenwardy so there should be the same issue with client scripting 14:12 pgimeno probably 14:13 rubenwardy also, compatibility doesn't matter here - either way, it'll be the same 14:13 pgimeno dropping that compatibility would be the only way to get rid of the GIL 14:13 rubenwardy I don't see why 14:14 rubenwardy either way, you have 1+ Lua states in emerge threads, and a Lua state in the server thread 14:14 pgimeno how do you get rid of the GIL without having two Lua threads in the same Lua state fight with each other? 14:14 rubenwardy the new code would run its callback in the emerge thread, the mapblock then gets added to the environment, and the register_on_generated would run in the server thread 14:15 rubenwardy You would have multiple lua states 14:15 pgimeno the way I see it, the new way would be 1 new Lua state per thread 14:15 pgimeno per emerge thread 14:15 rubenwardy sure 14:16 pgimeno if compatibility with old style Lua mapgen is necessary, it means that the emerge threads must be able to call stuff in the main Lua state 14:16 rubenwardy not at all 14:16 rubenwardy the new code would run its callback in the emerge thread, the mapblock then gets added to the environment, and the register_on_generated would run in the server thread 14:17 rubenwardy different callback, different stage 14:18 pgimeno current mapgens may rely on globals or local upvalues from the main Lua state 14:18 rubenwardy that doesn't change with a new system without compatibility 14:19 rubenwardy also, current mapgens can still use globals and local upvalues from the main Lua state - because they are in the main Lua state 14:19 rubenwardy A new threaded Lua mapgen API would be a different callback in a different state, without access to the main state Lua stuff 14:19 pgimeno that's exactly the issue, that's why the GIL is necessary 14:20 pgimeno when the state is shared, you need a GIL 14:20 rubenwardy it doesn't change either way, with or without compatability though 14:20 pgimeno r 14:20 pgimeno -r 14:20 pgimeno of course it does 14:20 rubenwardy because the old mapgen would still run in the main server thread with the main Lua state 14:20 pgimeno you don't need a GIL if the states are not shared 14:20 rubenwardy as register_on_generated 14:21 pgimeno yes, and that's why I say that you need to drop the old mapgen compatibility if you want to get rid of the GIL 14:21 rubenwardy Either you want the new mapgen to share the state or you don't. The old mapgen compatability is irrelevant, because it runs in the server thread with the main state as it always has 14:22 pgimeno If you want compatibility with the old mapgen, you need the GIL, and that will affect all other instances unless the GIL can be made state-local. 14:22 rubenwardy what needs the GIL? 14:23 pgimeno accessing the same Lua state from different threads needs a GIL 14:23 rubenwardy why do you need to do that? 14:23 pgimeno that's what happens with the current mapgen code 14:24 sfan5 yes and that will be kept 14:24 rubenwardy that seems fraught with errors to me, I thought that callback ran on the server thread after the block comes back from the emerge thread 14:25 sfan5 but you can move the intensive part (the lua mapgen) to a different non-shared lua state 14:26 pgimeno sfan5: the problem is that either you have two Lua interpreters running, one with GIL and one without, or the GIL will affect all states 14:26 sfan5 why is it a problem to have one with and one without? 14:26 pgimeno unless the GIL can be made state-specific 14:26 pgimeno I don't say it is 14:27 pgimeno I'm not 100% sure it's possible or desirable 14:28 sfan5 rubenwardy: it indeed runs on the emerge thread 14:38 pgimeno it sounds difficult to add two interpreters to the code 14:38 pgimeno er, add one* 14:47 pgimeno yeah, dropping compatibility is definitely out of the table. There are worlds that depend on the Lua mapgen behaving as it currently does. If a mapgen mod isn't maintained, dropping compatibility would kill these worlds. 15:12 Andrey01 hi, can #10363 BE merged now at long last or can NOT? 15:12 ShadowBot https://github.com/minetest/minetest/issues/10363 -- [MainMenu] Add clear button and icon for search one. by Andrey2470T 15:12 sfan5 it requires approval by second core dev 15:13 Andrey01 :O 15:17 Andrey01 looking at all, I feel it will continue to hang out so onwards... 15:20 Andrey01 if for ONLY still approving it needs to wait for weeks 15:23 rubenwardy I'm not a fan of that PR, but the better version requires work so I won't object 15:23 rubenwardy maybe I'm being too perfectionist 15:24 rubenwardy clear buttons should be inside the textbox and much less obvious - https://i.stack.imgur.com/Q2YxX.png 15:25 sfan5 and irrlicht should just provide them 15:25 rubenwardy yeah 15:26 rubenwardy that's the work I was referring to 15:31 Andrey01 well, I think it is not difficult, just it needs to put transparent buttons with the same icon inside the textbox 15:32 Andrey01 is it possible to make transparent buttons? 15:32 rubenwardy yeah, with border=false 15:32 rubenwardy that's a good point actually 15:33 rubenwardy although I guess it might be impossible to click text behind the button 15:33 rubenwardy you could also make the textbox without border, and draw the textbox manually. That might be a bit too much effort 15:33 rubenwardy btw, border is a style[] 15:36 Andrey01 and then set bgcolor with the same color value that the textbox has, yes? 15:37 Andrey01 why make textbox without border ? 15:37 rubenwardy because that allows the touch zones be different 15:39 Andrey01 hmm, I don`t quite understand what you mean 15:40 rubenwardy if you put a button on top of something, that something won't be clickable. So what you can do instead is disable the borders for the button and text area, and then draw it manually around the button and text area 15:40 rubenwardy This probably isn't needed for that PR though, and it feels a bit like cleaning one corner with manure everywhere else 15:41 Andrey01 also I noticed the current clear button clears the text only after the search 15:41 Andrey01 I dont know whether it is deliberate 15:41 rubenwardy that's because the formspec needs to change for it to clear 15:42 rubenwardy so what you can do is add a random variation to the formspec 15:42 rubenwardy say, a random number of spaces 15:42 rubenwardy this sucks but \o/ 15:45 Andrey01 ok 15:49 Andrey01 I suppose the borders could be then box[] elements, only very narrow, if you want them to be disabled for that reason 15:50 Andrey01 but at all it would be better to make fixes to the engine with the borders and not make such workarounds 15:52 Andrey01 because such problem happens with all gui elements which has borders, I suppose, not only with those are in the main menu 15:54 Andrey01 or is it already problem in Irrlicht itself? 16:09 rubenwardy I think I'm being unreasonable, the current PR is good enough 19:02 lhofhansl Any opinions on #10724 vs #10709? They are not entirely equivalent, but have the same goal of reducing server unresponsiveness. 10709 allows the compression to be without locking, but all the rest is still under lock. 10724 breaks up map-saving is multiple step, giving the server a chance to do other work - such as sending packets to the client - in betwee, 19:02 ShadowBot https://github.com/minetest/minetest/issues/10724 -- Enforce a map save time budget per server step. by lhofhansl 19:02 ShadowBot https://github.com/minetest/minetest/issues/10709 -- Allow map save to compress blocks asynchronously. by lhofhansl 19:04 lhofhansl Could also do both. 10709 also has "map maintenance" (save and timerUpdate) in its own thread, so it's not subject to other reason AsyncRunStep might be slow. 19:06 pgimeno lhofhansl: is there a queue of blocks to save? if so, is there a mechanism to remove from the queue the blocks that are overwritten? 19:06 sfan5 10709 definitely looks like the better change to me 19:08 lhofhansl I tend to agree... But... The concern I have about 10709 is that writing the uncompressed parts still take a lot of time (about 1/3 serialization and 2/3 compression and level 3). And if we use faster compression (zstd, etc) it may become pointless to compress outside of the lock. 19:09 lhofhansl Perhaps there idea to make the serialization (without compression) faster...? 19:09 lhofhansl ... are ... 19:09 sfan5 isn't serialization pretty much just memory copies? 19:10 sfan5 also what about the idea to clone mapblocks and do the entire serialization in the other thread 19:10 lhofhansl Yes. Looks like ostringstream is pretty slow. 19:11 lhofhansl I gave up on the cloning. It ballooned out to many other things, some of which are only maintained as pointers (gamedef, etc). Would have to clone those as well. 19:12 lhofhansl The nice part is map maintenance out of AsyncRunStep. 19:13 lhofhansl pgimeno: 10709 does that work on the same thread. There is no queue as such but a list of serialization buffers to be compressed and saved. The real blockes are marked and not-modified so that any concurrent changes are still "recorded" and then serialized and compressed later. 19:15 sfan5 uhh 19:15 sfan5 for serialialization purposes the gamedef is immutable 19:16 lhofhansl There is one theoretical race which is this: The map save threads generates a list of buffers. After that, but before the blocks are compressed and save a block is modified *and* the server is shutdown. In theory the shutdown hooks might get the lock after the block was modified first then save, and now that information is clobbered by the saving thread when finally finishing the save. 19:17 lhofhansl That is unlike, close to impossible to happen, but it *is* possible I think. 19:18 lhofhansl sfan5: There's NodeMetadataList, StaticObjectList, and on and on. I just could not get it to work. 19:21 sfan5 I might take a look at it then, since that approach is an even better idea IMO 19:21 lhofhansl That'd be nice. 19:22 lhofhansl Would definitely prefer that over the serialization cloning. 19:22 lhofhansl (or locking) 19:23 lhofhansl If compression would be really fast the entire thing might moot, though. 19:24 lhofhansl Compression and serialization that is. 19:31 lhofhansl I can clone more partial things - like the id-adjusted nodes... Actually lemme try that router. NodeMetadataList is the tricky part as far as I can tell. 19:58 lhofhansl Hah. So that bring the serialization to compression time ratio to about 1:5. I.e. 1/5 spent in serialization/copy, 4/5 spent in compression. So that's much better! 20:06 lhofhansl Pushed an update with that. Looking better now. Still not happy with API. It leaks too much out of the Map class, I feel.