| Time |
Nick |
Message |
| 00:01 |
|
lhofhansl joined #minetest-dev |
| 00:27 |
|
olliy joined #minetest-dev |
| 01:11 |
|
YuGiOhJCJ joined #minetest-dev |
| 03:00 |
|
Taoki joined #minetest-dev |
| 03:35 |
|
vesper11 joined #minetest-dev |
| 04:01 |
|
Taoki joined #minetest-dev |
| 04:36 |
|
vesper joined #minetest-dev |
| 05:00 |
|
MTDiscord joined #minetest-dev |
| 08:00 |
|
ShadowNinja joined #minetest-dev |
| 09:03 |
|
calcul0n joined #minetest-dev |
| 10:30 |
|
absurb joined #minetest-dev |
| 10:38 |
|
proller joined #minetest-dev |
| 11:48 |
|
Fixer joined #minetest-dev |
| 13:00 |
|
turtleman joined #minetest-dev |
| 14:22 |
|
lisac joined #minetest-dev |
| 14:43 |
|
p_gimeno joined #minetest-dev |
| 15:00 |
|
basxto joined #minetest-dev |
| 16:54 |
|
Fixer_ joined #minetest-dev |
| 17:27 |
|
lhofhansl joined #minetest-dev |
| 18:09 |
|
Taoki_1 joined #minetest-dev |
| 18:12 |
|
Taoki joined #minetest-dev |
| 18:29 |
lhofhansl |
Hey... Looking into shared locking for mapblock. std::shared_lock is only available in c++14 onwards, and we limit to c++11 (at least on Linux) |
| 18:30 |
lhofhansl |
Any advice? |
| 18:31 |
sfan5 |
don't you need a read/write lock anyway? |
| 18:33 |
|
proller joined #minetest-dev |
| 18:34 |
celeron55 |
what are you trying to do? |
| 18:35 |
celeron55 |
originally i wrote mapblocks to have mutexes and it was awfully slow, there's so much random access to them all over the place |
| 18:36 |
celeron55 |
probably don't want to repeat the same mistake |
| 18:38 |
lhofhansl |
Hmm... Trying to break the lock between mapsaving, map generation, and client handling. |
| 18:38 |
lhofhansl |
All of these are under one giant lock - the env lock. |
| 18:40 |
lhofhansl |
celeron55: Was is slow because of contested locks, or was it slow due to the memory barrier locks need to take even when not contested? |
| 18:41 |
lhofhansl |
Perhaps it's better to lock at the sector level...? |
| 18:41 |
celeron55 |
well the same code ran fast when i removed the locks |
| 18:41 |
celeron55 |
you can't do any locking that causes a lock/unlock at each node access |
| 18:43 |
lhofhansl |
Probably memory barriers them, which especially slow down multiple threads on multiple cores. |
| 18:44 |
celeron55 |
accessing a node currently usually causes just two pointer references and then indexing the node array in the mapblock |
| 18:44 |
celeron55 |
it's a way faster code path than you might think |
| 18:45 |
celeron55 |
well ok, also comparing the position of the cached sector and mapblock, but that's nothing also |
| 18:45 |
celeron55 |
the last used sector and last used mapblock is cached |
| 18:45 |
lhofhansl |
I totally believe that. I've seen bad locking slowing down services to a crawl even without any lock contention. (in real life I work on large distributed databases) |
| 18:46 |
lhofhansl |
I saw that part. When I tried to remove MapSector completely I realized how important that cache was :) |
| 18:46 |
celeron55 |
then when the sector or mapblock change, they have to be looked up from the std::maps |
| 18:48 |
celeron55 |
if you add a mutex, you have to implement some kind of node area locking that will then have to be used from every piece of code that's iterating through map content |
| 18:48 |
celeron55 |
(or some other lock) |
| 18:48 |
lhofhansl |
So what can be done? What's I'm seeing is that the server is not utilizing it's H/W well. When a block is compressed and saved, no other thread can do any work on the map. |
| 18:48 |
celeron55 |
so that each iteration doesn't cause extra operations |
| 18:48 |
lhofhansl |
That's what I was thinking. Keep the outer lock, but it cover a subset of the map. |
| 18:48 |
lhofhansl |
Do you think a MapSector is too small for that? |
| 18:49 |
celeron55 |
what's done currently is that when something is handed to another thread, the data is copied |
| 18:49 |
celeron55 |
if copying the mapblock in memory is much faster than compressing and saving it, you could make a copy first |
| 18:49 |
celeron55 |
then compress and save it in another thread |
| 18:49 |
lhofhansl |
I tried that for MapBlocks, but since they reference other stuff (iGameDef, etc) I still got corrupted data. |
| 18:50 |
celeron55 |
everything they reference is basically unmutable, no? |
| 18:50 |
celeron55 |
you'll probably have to change some code to not do stupid things if you do that |
| 18:50 |
lhofhansl |
Not the id-mapping it seems. I tried the emerge threads to pre-compress the block, and found the node-id mapping is wrong. |
| 18:51 |
lhofhansl |
I'll think about this more. Nothing concrete. Operating on immutable data or copies is nice. |
| 18:51 |
celeron55 |
well the mapping is created when compressing? |
| 18:52 |
celeron55 |
there's no such thing in mapblocks in memory |
| 18:52 |
lhofhansl |
Yes, it |
| 18:52 |
lhofhansl |
's derived from somewhere else. |
| 18:53 |
celeron55 |
and node ids never change |
| 18:53 |
celeron55 |
i think you just created some kind of bug and thought it meant what you were doing was impossible |
| 18:54 |
lhofhansl |
I just called serialize on the mapblock and cached that until the block's state was changed. That does not work. |
| 18:54 |
lhofhansl |
https://github.com/minetest/minetest/blob/master/src/mapblock.cpp#L390 |
| 18:54 |
lhofhansl |
Has to do with this. |
| 18:56 |
lhofhansl |
But not even trying to lock at the block level is good advice, saved me some time :) |
| 18:58 |
celeron55 |
i think collecting a log of node accesses could be useful |
| 18:59 |
celeron55 |
just to see the pattern and where the accesses originate from |
| 18:59 |
lhofhansl |
I also tried adding a copy constructor to mapblock and then saving a copy of the block. Interestingly I got the same id mismatch. I don't have the exact error message anymore, but it happened when the block was loaded again. |
| 18:59 |
celeron55 |
then you can think about if modifying the sources of most accesses is possible to a block or some kind of volume level locking |
| 18:59 |
celeron55 |
if it is possible, then you can try doing it |
| 19:00 |
celeron55 |
however you never know what mods want to do |
| 19:00 |
celeron55 |
so having the current fast access but wide locking works best i think |
| 19:00 |
lhofhansl |
Better avoid fine grained locking if possible. |
| 19:01 |
celeron55 |
well i don't understand why you would get an id mismatch |
| 19:01 |
celeron55 |
or what code would even detect it |
| 19:16 |
lhofhansl |
I can post a PR with the code, maybe folks can have a look what I am missing. |
| 19:16 |
lhofhansl |
(anyway work calls for now) |
| 19:31 |
|
lhofhansl joined #minetest-dev |
| 20:47 |
|
proller joined #minetest-dev |
| 20:53 |
|
mizux joined #minetest-dev |
| 21:54 |
|
Foz joined #minetest-dev |
| 21:58 |
|
Andrey01 joined #minetest-dev |
| 22:03 |
Andrey01 |
is my long-awaited PR going to be merged eventually? #10363 |
| 22:03 |
ShadowBot |
https://github.com/minetest/minetest/issues/10363 -- [MainMenu] Add clear button and icon for search one. by Andrey2470T |
| 22:05 |
Andrey01 |
this contains actually extremely simple changes and two months (even longer) are required for review... |
| 22:06 |
specing |
Andrey01: two months? Better get a coffee.. :P |
| 22:06 |
specing |
*a barrel of* |
| 22:07 |
Andrey01 |
yes, two months. The recent review was fulfilled on 19th September by sfan5 |
| 22:08 |
Andrey01 |
XDDD |
| 22:30 |
|
ShadowNinja joined #minetest-dev |
| 22:31 |
|
kb1000 joined #minetest-dev |
| 22:56 |
|
calcul0n_ joined #minetest-dev |
| 23:25 |
|
hecks joined #minetest-dev |
