Dev meeting minutes 2010-09-26

Dev meeting minutes 2010-09-26

Minutes of the meetings between Spring developers are archived here.
Post Reply
User avatar
hoijui
Former Engine Dev
Posts: 4344
Joined: 22 Sep 2007, 09:51

Dev meeting minutes 2010-09-26

Post by hoijui »

Date: Sunday, 26. September 2010 (2010-09-26)
Present: abma(joined later), hoijui, jK, Tobi, zerver

_Agenda_____________________________________________________________________
  • Welcome
  • Release plan (unitsync fix)
  • communication enhancement with mod-devs
  • improve unitsync
  • jK's spring-mt problem assumptions
  • SirMavericks patches

_Topics_____________________________________________________________________

Welcome
hoijui: :D
hoijui: i will make minutes
zerver: thanks
hoijui: i also made a new etherpad for spring git guidelines
hoijui: is also referenced in the main etherpad
hoijui: sadly.. the stuff we had on the springlobby.info gobby thing are lost
hoijui: or does anyone of you still have it?
hoijui: saved locally
zerver: not even sure what that is, so probably not
hoijui: :D

main points:
  • new git guidelines etherpad created

Release plan (unitsync fix)
hoijui: yeah... jk, how is the status there?
jk: same as pre-last week
jk: RL is still sucking :<
hoijui: mmm :/
hoijui: ok then... should we do a new release without this?
hoijui: mainly for smoths snake move fix
hoijui: and .. i saw that there are still FPS crashes with 5.1
zerver: fine by me
jk: I may have some free time next weekend again, I hope to get it fixed then
hoijui: mmm
hoijui: ok so.. wait one more week?
zerver: i have a special build and when everyone else crashes i don't, so it seems fixed
hoijui: ahh good :-)
hoijui: well.. if we can release it as 5.2, then i could do it anyway
hoijui: not too much work
hoijui: can still do an other release after next weekend
hoijui: ah .. the snake move fix is not sync save of course
zerver: tell me before, so i can suggest some commits to include
hoijui: ok :-)
hoijui: i think i'll wait till after next weekend then

main points:
  • new release planned for after next weekend


UML colab. tool
hoijui: aehm... :D
zerver: discussion over
hoijui: still did not test mroe there
Tobi: last time we said best just try out when you want to make some uml anyway
hoijui: i will try to test this week
hoijui: ahh yeah true

main points:
  • just try out UML tools you want to make some UML anyway

communication enhancement with mod-devs
hoijui: .. i did not do my questions... i dont really know any. mod stuf is too far away for me
zerver: i did questions, but am not sure about the answers
Tobi: I'm trying now to think of a good second question :)
jk: I am thinking of a good bad example (Tobi's contains nearly everything already ^^)
hoijui: well.. needs more time still then, i guess :D
zerver: because actual coding is more fun i guess :D


improve unitsync
zerver: this we discussed already? was it about that bug?
hoijui: no it was abmas project
hoijui: the downloader thing..
hoijui: guess.. neither have to discuss this, as he is not here


jK's spring-mt problem assumptions
jk: I think you read Andrej's additions?
jk: that the problem can really be like I assumed (still no ones will find out if it really is like that w/o testing it)
zerver: what additions?
jk: http://springrts.com/phpbb/viewtopic.php?f=12&t=24141
jk: I think it is worth a try before doing such drastic changes as splitting the LuaHandle
zerver: this discussion is irrelevant, since we have only two threads
jk: ppl already cried when I removed a single lua function, and now they would need to rewrite their whole lua (and it couldn't even be automated)
jk: still it is undetermistic which thread runs further and which sleep
** abma_irc joined the channel.
abma: hey! :)
hoijui: hey :-)
zerver: basically, all cpu time spent inside lua by draw thread has to be subtracted from the CPU time available to sim
jk: that's true still lua just uses 20-30% cpu time
jk: so there is enough time left for sim
zerver: so assume a draw call in takes 5ms to execute, and 100FPS --> 5*100ms = 50% of sim time available
jk: 50% is mostly enough
zerver: http://springrts.com/mantis/view.php?id=2149
jk: sim takes 30-60%
zerver: only thing that might help, is to reduce the FPS
jk: that's not the problem ...
zerver: the whole point of MT is to get high FPS...
jk: as said SIM takes just 30-60% cpu time, there is enough left for lua!
zerver: lol, with what CPU? many ppl are struggling to keep up with the game already
jk: and when you say you get >80% it is because you count the waiting for mutex locks in it too
zerver: nope
zerver: i have done extensive profiling, almost no time is spent waiting in MT sim because of all the batching
zerver: the waiting problem only arises with draw call ins, since lua is (or was) single threaded
zerver: now it is multi threaded, yay0
jk: ... you don't even want to think about other ideas ...
zerver: because there are none
jk: haha
zerver: you are the OpenGL expert, im good with threads
zerver: you cant mix multi/single threaded, it will fail
jk: ever heard of job-queues?
jk: it's what all new engines use ...
zerver: irrelevant in this scenario, sim thread lua is synced
zerver: we could put all lua in a separate thread, if you have magical solution to the sync issue :)
jk: so when you ignore all other ideas, are you rewriting all the existing lua code?
jk: no?
zerver: no need to rewrite all
zerver: some changes here and there
jk: lol
zerver: can be a long term project
hoijui: lets say, we want to use zervers method
hoijui: and would change all Lua to work wiht it
zerver: we can release two versions, one legacy (backwards compatible) and one new
hoijui: and later, we decide to undo it
hoijui: would we have to change stuff back?
hoijui: and second question: is there a way to scan lua for stuff incompatible with zervers method?
abma: ...or to be more specific, what has to be changed?
zerver: You have to use SendToUnsynced to communicate from Sim --> Draw
zerver: shared variables etc, no longer works
jk: none lua code in a DrawCallin would be allowed to acces synced state
zerver: yes it would
jk: not to forget that even the fixing the c-code of the lua interface would be a huge project alone
hoijui: to my questions.. change back would not be required?
zerver: if SendToUnsynced becomes a bottleneck, possibly need to change back, but i doubt that
hoijui: ok
hoijui: and scanning.. sounds like that woudl be hard to do
Tobi: so SendToUnsynced would replace SYNCED in your implementation?
zerver: yeah
hoijui: and.. i bet that 90% of mod devs will never understand what the have to do
jk: your SendToUnsynced doesn't even support tables ...
Tobi: can't SYNCED be buffered?
hoijui: or what you mean when trying to explain them
jk: yup, hoij
Tobi: as far as I see there should be no really fundamental problems in keeping SYNCED even when unsynced and synced Lua run in different states & different threads
zerver: wait im wrong, it does not replace SYNCED
Tobi: though the implementation would get quite hairy
zerver: i will show an example to illustrate with actual code
zerver: http://pastebin.com/wAWgcPtc
zerver: well it replaces SYNCED in a way i guess
hoijui: how will this play together with Tobis proposal he made a few weeks ago
hoijui: using multiple states coordinated with TLS .. something
hoijui: and the whole adimn stuff put into a parent class
hoijui: (remember somethign like that)
zerver: Look at line 50 + 139 in that pastebin
jk: tripple buffered synced state
zerver: _G.napalmCount = #napalmExplosions
zerver: for i=1,SYNCED.napalmCount do
zerver: synced sets napalmCount, then uses SendToUnsynced, and unsynced tries to read it --> fail
zerver: so in this case, the coordinates of each explosion would have to be sent using SendToUnsynced
zerver: a small change, 5 mins maybe
jk: and if there are 1000 explosions?
jk: pushing them through SendToUnsynced is ... extremly ugly (and maybe slow too)
zerver: 1000 simultaneous explosions is a rare event
zerver: but i get your point
hoijui: the alternative to zervers method is, having multiple states?
jk: yup
zerver: ehm? my method uses multiple lua_states...
jk: synced states
hoijui: yeah.. the same like planned in C++, basically, yes? (in the alternative method)
jk: still there are problems left with splitted LuaHandles which can hardly be solved w/o multiple synced states
zerver: im kinda lost, how would multiple synced states help?
jk: lua render code can read from it?
jk: (w/o locking the sim code)
hoijui: old sim state is read only, can be used by sim and draw
hoijui: and new state can be written, by sim only
hoijui: and not read by draw
hoijui: yeah? :D
zerver: with a single lua_state?
jk: yup
zerver: not sure how that would help, when the two threads competing for a single lua state is the root of the problem
Tobi: Synced lua would modify stuff in _G like usually. At some point during each game frame, engine copies this data from synced Lua and makes it available as SYNCED in unsynced Lua.
Tobi: unless I'm missing something that would allow synced and unsynced to run in different states & threads
hoijui: there can only be one thread per lua_state?
zerver: yup, unless there are mutexes in place
zerver: sim has to wait for draw and vice versa
Tobi: if needed SendToUnsynced could be buffered and only "committed" at the same time, to make it seem to unsynced lua as if modifications to SYNCED & calls to SendToUnsynced remain ordered
hoijui: .. why?
jk: sim doesn't always have to wait ...
hoijui: i mean.. why do they have to wait, if there are multiple states
zerver: concurrency inside a single lua state --> crash
jk: sim can buffer a lot things in a job-queue and continue its work
zerver: sim needs the synced result of the lua call-in immediately
hoijui: ahh...
jk: only of LuaRules & LuaGaia
jk: LuaUI (that's what takes the most time btw) doesn't return anything synced -> SIM thread can continue
zerver: yeah, this problem affects LuaRules & LuaGaia only
zerver: that is another key point
zerver: widgets that are unsynced only, work without any modification
zerver: or i hope so :P
zerver: does sim thread ever invoke luaUI ? im kind of unsure
jk: zerver: widgets that are unsynced only, work without any modification > ???
jk: widgets READ the synced that as anything else
jk: +state
zerver: and they can...
zerver: the problem is only with variables assigned on the synced side of the lua code, such as napalmCount in my example
hoijui: but if there are two lua-states
hoijui: one for synced nad one for unsynced, right?
hoijui: and unsynced widgets woudl only use the second
hoijui: how woudl they read stuff fro mthe first?
hoijui: or they never need anythign from the first?
zerver: the unsynced side also loads all synced stuff
hoijui: and if it tries to read a table while the sycned stuff writes to it?
jk: erm if they read the health of an unit and the sim thread writes to it -> BUMM
zerver: there are mutexes for that...
zerver: there have been for a long time
hoijui: for reading a value? or for loading all synced?
jk: so you want to put in all lua functions a mutex for it?
jk: a bit much overhead, no?
zerver: no need, the necessary mutexes are already there
jk: and no there aren't such mutexes in the lua atm
Tobi: could use a mutex as fallback and learn from that to buffer that value the next frame
zerver: we basically have two identical lua_states, they only deviate once the synced side starts adding variables and such
jk: that will be a huge overhead too
Tobi: less then copying complete state every frame
Tobi: *than
zerver: i don't see the problem, as i said the mutexes are already in place
jk: don't think so (as long as the heighmaps etc. aren't copied, those are a separate problem)
zerver: how bout i modify a couple of gadgets so that we can see how big changes are really needed?
hoijui: (btw, just so it is not forgotten, we should also shortly discuss SirMavericks patches (see meeting etherpad))
hoijui: the thing is.. you are the implementer of this .. of course you know what to do, and can to it fast
hoijui: but what about all hte mods
jk: http://github.com/spring/spring/blob/53 ... .cpp#L2613
jk: I don't see any mutex in there
hoijui: mod-devs*
zerver: im thinking we are making too big a fuzz about this, especially when we have the possibility to release a legacy build
hoijui: if we find a way that the legacy build is not needed, which has no drawbacks..
hoijui: this discussion sure is worth its time
jk: and I think, you are working in the wrong direction and I am not willing to rewrite all the lua nor do I want to fix the c-code
jk: esp. if there may be other solutions
zerver: the mutexes are at the entry point of draw call ins
hoijui: so what si current state of discussion..
hoijui: alternative method woudl need more copying?
hoijui: but otherwise has no drawbacks?
hoijui: or what are the other drawbacks?
jk: zerver: the mutexes are at the entry point of draw call ins > erm so the render thread still blocks the sim thread???
jk: so where is the reason for splitting the LuaHandle then?!
zerver: it uses batching, locking is minimal
zerver: what c-code needs to be fixed by the way?
jk: you broke the SYNCED object
jk: you broke a lot other lua functions
jk: you may broke LuaRules&LuaGaia
zerver: did i break SYNCED?
jk: yes
zerver: i thought we have working SYNCED, but two copies of it
jk: because the SYNCED objects needs 1 Lua_state shared for synced and unsynced LuaRules
jk: it doesn't copy the lua objects in it, it just shares them
zerver: this means we have two copies of SYNCED
jk: ?
zerver: you can write to synced, but unsynced side cannot read it
zerver: because the unsynced side has its own SYNCED object
zerver: useless, but not broken
hoijui: thats negative, right?
hoijui: ok
Tobi: don't think it needs 1 shared lua_state, it could read from the other state in every access isn't it? (unless you were referring to the current implementation specifically)
zerver: reading from the other state requires mutex --> fail
zerver: Xcall already implements this
Tobi: hence, buffering :)
zerver: possibly
jk: the synced side doesn't have a SYNCED metatable at all
zerver: what is _G?
jk: the unsynced SYNCED metatable access the _G lua default enviroment of the synced side and makes specific stuff available to the unsynced side
jk: _G is the default lua enviroment
zerver: lol
jk: e.g. when you create a global like this: foo = "bar"
jk: then you can access it via _G["foo"]
zerver: ok
zerver: But my assumption is that sim writing to _G means draw can access it using SYNCED is correct?
jk: yup
jk: sim/synced
jk: there is sim in unsynced LuaUI too
zerver: ok, what entry point?
jk: LuaUI has a GameFrame callin like synced LuaRules has etc.
zerver: ah
zerver: so could you write to _G there as well?
jk: no the SYNCED objects is a LuaRules & LuaGaia only object
jk: -s
zerver: ok, i think most widgets will work without changes but i may be wrong ofc
jk: they won;t if you remove that mutex around all lua code
jk: and now you have 2 LuaUI instaces running ...
zerver: yeah, but not one executes only GameFrame, and the other the draw call-ins, so no more CPU time spent
zerver: -not
jk: but the other one doesn't have a GameFrame callin anymore
jk: also I don't saw any changes like that in your commit
zerver: it has, but it never gets called :P
jk: so I assume it's borken too
jk: *broken
zerver: well, it may be broken depending on how the widget relies on GameFrame then
zerver: this could maybe be worked around by invoking luaUI->GameFrame from draw thread also
zerver: anyway, I will attempt to fix BA so it runs 100% with the dual lua states
jk: as said I won't fix all these things ...
zerver: gives some measure on how big the incompatibility is
zerver: no one requested that you fix it...
zerver: now gotta eat, bye
abma: ....SirMavericks patches?
Tobi: LuaUI events could be processed using a queue in any non-sim thread afaics
Tobi: probably yeah :)
jk: jk: sim can buffer a lot things in a job-queue and continue its work
Tobi: oh, I assumed you meant sim jobs with that, not events
Tobi: seems I timed out at 18:00 without anything here noticing the disconnect
Tobi: did I miss anything?
jk: nope
Tobi: ok

main points:
  • there is still no consensus at all

SirMavericks patches
Tobi: patches seem fine to me
hoijui: ok :-)
hoijui: i can merge them
Tobi: if we get PlayerRemoved then I think PlayerAdded should be added too, having only PlayerRemoved and handling added+changed in PlayerChanged would be inconsistent
hoijui: jk.. you commented on one of them
hoijui: yeah
Tobi: (re the comment on PlayerAdded)
jk: I don't see a reason for PlayerAdded when there is PlayerChanged, but in the end the PlayerChanged is the more redundant/ugly callin
Tobi: when does playerchanged get called?
jk: when teams dies, player joins, ...
Tobi: and not when a player is removed?
jk: player types /spectate
jk: there is already PlayerRemoved afaik
Tobi: confirmed
jk: ah yeah, SirMav's just adds it to the default widgetHandler
Tobi: maybe patch got merged earlier already?
jk: CA's already got it
jk: trepan added those callins
jk: PlayerChanged, PlayerRemoved, TeamDied, ...
jk: they weren't the cleanest
jk: e.g. PlayerChanged has a lot of meanings
hoijui: what if player changes from team X to team Y?
jk: calls it too I think
hoijui: ok..
jk: so a PlayerAdded callin isn't that dumb
hoijui: so form C to Lua we have PlayerChanged, and the (default) gadget handler creates all the other calls?
jk: there are no other callins
jk: and SirMav's PlayerAdded callin is done on engine level
hoijui: ah ok
hoijui: mmm
abma: then.. do it like ca and improve the lua widget or do it at engine-level?
abma: imho at engine level it's much cleaner...
jk: ?
jk: CA doesn't has a PlayerAdded callin
jk: it just has the PlayerRemoved callin which is already engine side
jk: but not part of defautl widgethandler
abma: aah, ok
Tobi: I'm off to dinner too
hoijui: as i get it, cleanest way woudl be, to only have PlayerChanged engine side
hoijui: which allows you to have all the others in Lua land
hoijui: assuming teamId=-1 is used
hoijui: if player got removed. died, or was added
abma: that requires the lua side to scan for what changed...hmm
abma: but it would be easier to maintain...
abma: hm :)
hoijui: yeah, you woudl have some if (newTeamId == -1) and similar
hoijui: but... its not like this was called 100 times a second, so no problem
jk: for that the widgetHandler would need to be cleaner coded
jk: doing such things atm, just blow it up and make it even less maintainable
abma: ok, then don't merge and do it in lua :)
hoijui: what jk sais, suggests doing the opposite, if i got it right
jk: yup
hoijui: ok will do that then
abma: hm, ok

main points:
  • merge the patches in (hoijui's job)
User avatar
smoth
Posts: 22309
Joined: 13 Jan 2005, 00:46

Re: Dev meeting minutes 2010-09-26

Post by smoth »

I use a nonstandard widget handler, so can I get some details on sir mav's patch?

also I don't follow this:
hoijui: ah .. the snake move fix is not sync save of course
User avatar
FLOZi
MC: Legacy & Spring 1944 Developer
Posts: 6240
Joined: 29 Apr 2005, 01:14

Re: Dev meeting minutes 2010-09-26

Post by FLOZi »

Probably means that the fix will not sync with other 0.82.5 releases, i.e. it can't simply be in a non-mandatory upgrade like 0.82.5.1 -> 0.82.5.2 but will have to be in 0.82.6 (If I remember rightly how the versioning system works wrt sync)
SirMaverick
Posts: 834
Joined: 19 May 2009, 21:10

Re: Dev meeting minutes 2010-09-26

Post by SirMaverick »

hoijui wrote:jk: ah yeah, SirMav's just adds it to the default widgetHandler
Tobi: maybe patch got merged earlier already?
jk: CA's already got it
jk: trepan added those callins
jk: PlayerChanged, PlayerRemoved, TeamDied, ...
No. As it wasn't added to default widgetHandler yet, it seems no one outside CA is using it/interested in.
jk: they weren't the cleanest
jk: e.g. PlayerChanged has a lot of meanings
jk: so a PlayerAdded callin isn't that dumb
[...]
hoijui: as i get it, cleanest way woudl be, to only have PlayerChanged engine side
hoijui: which allows you to have all the others in Lua land
hoijui: assuming teamId=-1 is used
hoijui: if player got removed. died, or was added
abma: that requires the lua side to scan for what changed...hmm
abma: but it would be easier to maintain...
PlayerAdded/PlayerRemoved state what happend. PlayerChanged doesn't tell you what happened. So you either rebuild your data in Lua for all players or you copy the whole player state (or parts you are interested in) and check what changed. It's both ugly.
SirMaverick
Posts: 834
Joined: 19 May 2009, 21:10

Re: Dev meeting minutes 2010-09-26

Post by SirMaverick »

smoth wrote:I use a nonstandard widget handler, so can I get some details on sir mav's patch?
I suggested to add the following call-ins:
PlayerAdded(playerID) - called when someone joins mid-game
GamePaused(playerID, paused)

and update default widget handler for already existing call-in:
PlayerRemoved(playerID, reason) - reason: 0 = timed out, 1 = quit, 2 = got kicked
abma
Spring Developer
Posts: 3798
Joined: 01 Jun 2009, 00:08

Re: Dev meeting minutes 2010-09-26

Post by abma »

more details are here:
* PlayerAdded call-in: http://springrts.com/mantis/view.php?id=2134
* PlayerRemoved call-in: http://springrts.com/mantis/view.php?id=2133
* GamePaused call-in: http://springrts.com/mantis/view.php?id=2135
User avatar
zwzsg
Kernel Panic Co-Developer
Posts: 7049
Joined: 16 Nov 2004, 13:08

Re: Dev meeting minutes 2010-09-26

Post by zwzsg »

SirMaverick wrote:No. As it wasn't added to default widgetHandler yet, it seems no one outside CA is using it/interested in.
At one point I was needing it, and was told there was no such callin and that I had to scan periodicaly the playerlist.
SirMaverick
Posts: 834
Joined: 19 May 2009, 21:10

Re: Dev meeting minutes 2010-09-26

Post by SirMaverick »

zwzsg wrote:
SirMaverick wrote:No. As it wasn't added to default widgetHandler yet, it seems no one outside CA is using it/interested in.
At one point I was needing it, and was told there was no such callin and that I had to scan periodicaly the playerlist.
Depends when you asked. The call-in was added in 0.81.0.
Post Reply

Return to “Meeting Minutes”