Time  Nick      Message
07:08 sfan5     are you sure simply looking up a value in the map is a bottleneck?
07:09 sfan5     obviously something has to come out on top when you profile but if everything is fast enough then there's nothing more to do
10:57 MTDiscord <exe_virus> Actually yes, looking up object pointers in modifySafeMap becomes a bottleneck at 5000 entities after fixing the spatial index piece.   And no, std::map is notorious for being slow, there are drop in replacements that can be 3-5 times faster, which is enough to matter once we fix lookups by position
11:06 sfan5     a bottleneck in what exactly?
11:10 sfan5     maybe my intent is clearer when I ask: after replacing the map implementation something else will be the bottleneck? will you optimize that too? or when is it enough?
11:13 sfan5     the reason I am questioning this is 1) importing some new dependency is best avoided 2) my gut feeling says the source of lag is the O(n) spatial lookup, not the container implementation
11:18 sfan5     (to be clear you could use the same argument to say that spending time on ModifySafeMap was misguided and it would be correct)
11:32 MTDiscord <exe_virus> understood, this was what we were seeing with Lars' imlpementation, we didn't get to profile mine
11:32 MTDiscord <exe_virus> second: yes it was a significant enough percentage of execution to warrant optimization
11:33 MTDiscord <exe_virus> thrid: us implementing something from scratch is identical to brining in one of the header-only specialized maps
11:33 MTDiscord <exe_virus> It was about 35% of execution time for the get() function on modifySafeMap alone after fixing getObjectsInArea and getObjectsInRadius.
11:34 MTDiscord <exe_virus> After things start to drop to 10-15% equal across 6 different hot paths, then I stop optimizing. But as long as things are 30+% of execution time, that warrants optimization in my eyes
11:41 sfan5     what's the test case? "the 100%"
11:41 MTDiscord <exe_virus> Oh, it was just a yourlands test server with about 5000 entities all trying to move, collide, etc.
11:42 MTDiscord <exe_virus> After whacking the SpatialIndex problem, the next whack a mole was modifySafeMap get, which makes sense, because I can imagine hitting that like 2-3 times per entity, so we could get 10-15 thousand std::map lookups
11:44 sfan5     that makes sense
11:44 sfan5     how was the max_lag during that?
12:02 MTDiscord <exe_virus> 5 seconds
12:03 MTDiscord <exe_virus> If we can support 5000 entities with 20 server ticks per second (STPS), then I would be done with optimizing for a while. That's enough for 50 players all to have 100 entities haha
12:04 sfan5     sounds worth optimizing then
12:04 sfan5     btw I wasn't suggesting writing our own optimized map, that would be stupid
12:05 MTDiscord <mistere_123> fyi there are now commercial users of minetest who want to have 100 concurrent users. Not that the Minetest team needs to cater to them especially, but a use for optimization exists
12:05 MTDiscord <mistere_123> well, 100+
12:07 MTDiscord <exe_virus> That's, not that many entities?
12:10 MTDiscord <mistere_123> users, not entities
12:20 MTDiscord <exe_virus> right, but that many users = only 100 entities, what are they doing with minetest? node stuff? entity stuff? particles?
12:21 MTDiscord <exe_virus> Also, don't we already support 100+ connected users?
12:30 MTDiscord <exe_virus> On a different subject: why do we still have the BS macro/variable everywhere, is it still performing a purpose?
13:56 sfan5     the purpose of it has not changed, it depends on whether you consider it useful
14:04 srifqi    hello. is merging this directly to master fine? (pinging sfan5 as the author of the commit causing the issue)
14:04 srifqi    https://gist.github.com/srifqi/f00717f4bb61e28e4247aadca2edfbc0
14:05 sfan5     of course
14:07 srifqi    okay, will do soon-ish
14:14 srifqi    done
15:36 MTDiscord <exe_virus> I guess rather than use it to keep people from blindly converting floats to ints, and thereby introducing a bunch of multiplication everywhere, we just wrap position into a class, so static casting to an int isn't allowed, without an explicit member function. And that can be fully performant, without underlying additional operations introduced
17:13 grorp     sfan5: could you update the sdl2 version used by the mingw CI? 2.30 contains a Windows fix that's relevant for my touchscreen stuff (https://github.com/libsdl-org/SDL/pull/8750 / https://github.com/libsdl-org/SDL/commit/bbe4d693eb0642c8011d1936d40185e4b5669eb7)
20:08 sfan5     sure