Time |
Nick |
Message |
00:04 |
|
OWNSyouAll joined #minetest-dev |
00:22 |
|
bcnjr5 left #minetest-dev |
00:27 |
VanessaE |
now there's someone I haven't seen in a while. Too bad he picked the wrong channel. |
00:43 |
sapier |
Ok I think I got VanessaE's crash. If a ResultQueue job times out those functions just catch the timeout exception and leave. This way the RequestQueue itself is deleted |
00:43 |
sapier |
result queue of course |
00:44 |
sapier |
yet the Request is still queued. within that request a now invalid pointer to the original request queue is stored |
00:44 |
sapier |
once this Request is processed memory is randomly messed up |
00:44 |
VanessaE |
ok, here goes nuthin' |
00:44 |
VanessaE |
nope.avi |
00:44 |
sapier |
https://github.com/sapier/minetest/tree/fix_RequestQueue |
00:44 |
VanessaE |
crashola. |
00:45 |
sapier |
where does it crash for you vanessa? |
00:45 |
VanessaE |
looks like it's in the same place. lemme get a backtrace |
00:46 |
VanessaE |
http://pastebin.ubuntu.com/6413551/ |
00:48 |
sapier |
hmm ok there's still one possible reason left I was concerned |
00:48 |
sapier |
if a job is processed after I did clear the queue that one will be in front of queue |
00:49 |
sapier |
if I'm right that's easy to fix |
00:50 |
VanessaE |
ok |
00:52 |
sapier |
ok try again |
00:54 |
VanessaE |
trying... |
00:54 |
VanessaE |
sapier: success! |
00:54 |
sapier |
I fixed only one occurance so this might now happen at one of the other 2 locations ;-) |
00:54 |
VanessaE |
that did the trick |
00:54 |
sapier |
yes but it's ugly |
00:55 |
sapier |
I need to think about it tomorrow it's 2 o'clock in the morning now ;-) |
00:55 |
VanessaE |
ugly it may be, but I'll bet it significantly narrowed the search for the root cause |
00:55 |
sapier |
no this IS the root cause |
00:56 |
sapier |
the whole concept with result queues beeing local objects passed to another thread is ugly |
00:56 |
|
RealBadAngel joined #minetest-dev |
00:57 |
sapier |
I practicaly made them persistent by adding the "static" at cost of possible outdated jobs queued within them |
00:58 |
VanessaE |
oh ok |
00:58 |
VanessaE |
in that case, you FOUND the root cause, and now fixing it properly will be 10x easier |
00:59 |
VanessaE |
(to my ming the hardest part of debugging is identifying the cause. coding a fix is usually trivial once you get to that point) |
00:59 |
VanessaE |
mind* |
00:59 |
sapier |
that one was tricky true ... yet I always had that queue handling in mind |
01:00 |
sapier |
something seemed to be wrong there I just didn't know what |
01:01 |
ShadowNinja |
sapier: #1003 has a bunch of extra newlines at the end of the files. And should that be in jthread? |
01:02 |
sapier |
I think so yes .. I'll fix that newlines later I need to go to bed now |
01:03 |
VanessaE |
WOW |
01:03 |
VanessaE |
sapier: |
01:03 |
sapier |
vanessae I just updated that branch to fix the other two possible crashes too ... but that one needs cleanup ... new eclipse did a lot of automatic whitespace changes to those files |
01:03 |
VanessaE |
your patch not only stops the crash but it also speeds up the texture rendering process somehow. or so it seems |
01:04 |
VanessaE |
oh sure |
01:04 |
VanessaE |
lemme pull that real quick |
01:04 |
RealBadAngel |
sapier. hold on a sec |
01:04 |
sapier |
I don't have an idea how that should speedup texture processing but as that branch is a RequestQueue and ResultQueue cleanup it might be possible |
01:04 |
sapier |
yes rba? |
01:05 |
VanessaE |
nope, already up to date, so it says. |
01:05 |
RealBadAngel |
im uploading now texture pack you liked, wanna link? |
01:05 |
VanessaE |
sapier: hang around for a few more mins please. |
01:05 |
sapier |
sorry forgot to push did only commit now it should be there |
01:06 |
VanessaE |
oh ok, sec. |
01:06 |
sapier |
rba yes but I can't test it right now |
01:06 |
sapier |
ok vanessa 5 min |
01:06 |
sapier |
I have to work today ;-) |
01:06 |
VanessaE |
sapier: ok, got the update. testing it. |
01:06 |
RealBadAngel |
upload will be ready in circa 3minutes |
01:07 |
VanessaE |
sapier: ok, no crash and no immediate change when connecting to the previously-offending server. good. |
01:08 |
VanessaE |
sapier: anything I should check in particular? |
01:08 |
sapier |
no the queue fixes fix things that have been that wrong they never could've been used |
01:08 |
sapier |
fix fixes :-) |
01:09 |
VanessaE |
ok |
01:09 |
sapier |
this issue might have been cause for various strange crashes |
01:10 |
VanessaE |
sapier: testing stuff, one or two more mins. |
01:11 |
VanessaE |
wow |
01:11 |
RealBadAngel |
sapier, https://forum.minetest.net/viewtopic.php?pid=117843#p117843 ,with some new screenshots |
01:11 |
VanessaE |
Survival is several seconds faster to log in, and opening the inventory immediately on connect no longer crashes the cliebnt. |
01:11 |
sapier |
btw vanessa you should see in infostream "Waiting for shader timed out." |
01:11 |
VanessaE |
sapier: you may have just fixed #831 |
01:12 |
RealBadAngel |
sapier, have you disabled shaders when creating inv items? |
01:12 |
sapier |
lol :-) guess this might fix some other "unconfirmed" bugs too |
01:12 |
VanessaE |
ok here comes the acid test... |
01:13 |
VanessaE |
sapier: seems good. |
01:13 |
sapier |
no I just fixed the queue handling but as timeout now no longer currupts random memory locations this might result in missing shader instead of crash |
01:13 |
RealBadAngel |
that's my issue with shaders. inv items are lit and shadowed with weird angles |
01:14 |
VanessaE |
better a missing shader than a seemingy-random crash |
01:14 |
RealBadAngel |
so shaders *should* be disabled there |
01:14 |
VanessaE |
RealBadAngel: I've noticed this. |
01:14 |
sapier |
do you use my branch rba? |
01:14 |
RealBadAngel |
no, thats issue of my branch |
01:15 |
sapier |
then no I didn't disable shaders ;-) |
01:15 |
VanessaE |
sapier: doesn't help the slowness of the extrude code in the extreme cases, this is where kahrl's disable-extrude patch is needed. |
01:15 |
VanessaE |
anyways, it's good. go to bed :) |
01:15 |
RealBadAngel |
but you could if youre already here |
01:15 |
sapier |
thx for permission ;-P |
01:15 |
sapier |
good night |
01:15 |
|
sapier left #minetest-dev |
01:15 |
RealBadAngel |
cya |
01:47 |
|
Miner_48er joined #minetest-dev |
02:12 |
|
zork_ joined #minetest-dev |
02:29 |
VanessaE |
celeron55: your entity dupe patch (#1000)... is it safe to use? |
02:39 |
|
paramat joined #minetest-dev |
02:43 |
|
paramat left #minetest-dev |
02:46 |
VanessaE |
well anyway, it's been deployed to all my servers. /clearobjects is in progress on all 5 as I speak. |
02:47 |
VanessaE |
(and it's on my client) |
06:09 |
|
darkrose joined #minetest-dev |
07:00 |
|
NakedFury joined #minetest-dev |
07:17 |
|
darkrose joined #minetest-dev |
07:32 |
|
KingsleyT joined #minetest-dev |
07:53 |
|
OldCoder joined #minetest-dev |
08:32 |
|
Weedy_lappy joined #minetest-dev |
08:40 |
|
SmugLeaf joined #minetest-dev |
08:44 |
|
RealBadAngel joined #minetest-dev |
08:44 |
|
e1z0 joined #minetest-dev |
08:44 |
|
celeron55 joined #minetest-dev |
08:45 |
|
hax404_ joined #minetest-dev |
08:46 |
|
OldCoder joined #minetest-dev |
09:35 |
|
darkrose joined #minetest-dev |
10:14 |
|
proller joined #minetest-dev |
10:15 |
|
ImQ009 joined #minetest-dev |
11:05 |
proller |
https://github.com/proller/minetest/compare/indev - is any objections ? |
11:18 |
|
sapier joined #minetest-dev |
11:25 |
sapier |
Proller no idea what you change there but I assume you know what you're doing |
11:26 |
proller |
https://github.com/minetest/minetest/issues/1005 |
11:34 |
sapier |
More interesting structures sounds great but I can't test atm |
11:52 |
sapier |
Btw my fix for vanessae's crash isn't 100 % correct. It still may assert if two timouts follow directly while first request is processed in between. It's a invalid assert only that case is perfectly valid. |
11:55 |
|
Akien joined #minetest-dev |
12:43 |
pitriss |
Hi guys, I want to ask.. can default tree growing mechanism adjust growing speed based on lenght of day/night cycle? Or this is just constant interval? |
13:01 |
proller |
https://github.com/proller/minetest/commit/8427d78c5a49ad633875fdb61ffe6f5e0d70fd2c |
13:03 |
proller |
pitriss, i have idea for future: step by step growing, with adjustable speed |
13:04 |
pitriss |
proller: ah.. I got that idea when on our server was farming plus growing too fast.. And having this also for trees would be really cool.. |
13:10 |
|
EvergreenTree joined #minetest-dev |
13:16 |
|
kahrl joined #minetest-dev |
13:24 |
|
hmmmm joined #minetest-dev |
14:09 |
sapier |
Proller growing trees mod already has step by step growing trees ;-) |
14:10 |
proller |
wow, need to try |
14:41 |
|
Zeitgeist_ joined #minetest-dev |
15:12 |
|
zat joined #minetest-dev |
15:27 |
|
iqualfragile joined #minetest-dev |
15:27 |
|
EvergreenTree joined #minetest-dev |
15:33 |
|
proller joined #minetest-dev |
15:52 |
|
BlockMen joined #minetest-dev |
15:53 |
|
damiel joined #minetest-dev |
15:53 |
BlockMen |
should i make a pull for that or would someone agree that i can push directly? https://github.com/BlockMen/minetest/commit/e67dea94a2d8a5810bdfd46d8d2baa7c2853fee9 |
16:00 |
|
OldCoder joined #minetest-dev |
16:02 |
|
sapier1 joined #minetest-dev |
16:07 |
|
NakedFury joined #minetest-dev |
16:08 |
|
Jordach joined #minetest-dev |
16:35 |
|
PilzAdam joined #minetest-dev |
16:47 |
|
sapier joined #minetest-dev |
16:55 |
sapier |
proller growing trees is quite old |
16:55 |
sapier |
pre ltrees |
16:59 |
|
Calinou joined #minetest-dev |
17:13 |
celeron55 |
sapier: i was looking at your patch this morning and it seemed like you removed the timeout functionality completely |
17:14 |
celeron55 |
and that whole got_invalid_name thing is quite dubious |
17:15 |
celeron55 |
you should at least document it properly, it doesn't make any sense |
17:17 |
sapier |
yes the got invalid_name thing is to be removed it's useless there still is a situation where the assert fails for no reason |
17:17 |
sapier |
actually the timeout was source of error |
17:18 |
sapier |
but I didn't quite remove it, the requesting thread will still continue after timeout |
17:19 |
sapier |
and the processing thread will process it too (as it did before) |
17:19 |
sapier |
but now the result will be discarded on next request ... before this patch it was just written to random memory once it was processed |
17:21 |
sapier |
the timeout never did abort the request it just resulted in noone waiting for result and therefore invalid result queue pointer |
17:33 |
|
Akien joined #minetest-dev |
17:38 |
|
djdduty joined #minetest-dev |
17:40 |
|
rubenwardy joined #minetest-dev |
17:43 |
|
bas080 joined #minetest-dev |
17:46 |
sapier |
a generic question are spaces at lineend intended in minetest or can I fix them? |
17:48 |
sapier |
celeron55 this is the actual fix https://github.com/sapier/minetest/commit/fcf807b4306767776bac9a2528abb1cb571c9331 (and some whitespace fixes done by eclipse ... I'll remove them if they aren't ok) |
17:54 |
BlockMen |
sapier, are you ok with https://github.com/BlockMen/minetest/commit/e67dea94a2d8a5810bdfd46d8d2baa7c2853fee9 ? it fixes a mistake i made with background clipping in formspecs |
17:54 |
sapier |
seems to be correct yes ;-) |
17:56 |
sapier |
VanessaE could you check the new version of my patch, it should fix a error in the version you're using atm |
17:58 |
BlockMen |
ok, i gonna push it then |
18:17 |
|
Miner_48er joined #minetest-dev |
18:20 |
VanessaE |
sapier: sufficient to just pull from your branch? |
18:24 |
sapier |
yes I force pushed the changes |
18:24 |
VanessaE |
k |
18:25 |
sapier |
celeron I wonder if JEvent should be moved to it's own file in fact it's 95% identical to my jsemaphore implementation with one difference, event is a non counting semaphore |
18:26 |
|
damiel joined #minetest-dev |
18:26 |
|
damiel joined #minetest-dev |
18:28 |
|
ImQ009 joined #minetest-dev |
18:28 |
VanessaE |
sapier: ok, got it. got a test case for the thing you fixed? |
18:29 |
sapier |
you now will see a error message if something times out |
18:29 |
VanessaE |
oh by the way: |
18:30 |
VanessaE |
can someone please fix the debug-build's unit tests to try ports other than 30000? |
18:30 |
VanessaE |
(else it fails to bind, since I have a server on that port, and then aborts) |
18:31 |
sapier |
I don't see any use for it unittests do fail anyway if run in debugger |
18:31 |
VanessaE |
I'm not using the debugger in this case :) |
18:31 |
VanessaE |
merely a debug-enabled build |
18:32 |
sapier |
you shouldn't even use debug builds on production server ;-) |
18:32 |
VanessaE |
this ain't a server :) |
18:32 |
VanessaE |
this is the client; I have a regular production server already on that port. |
18:35 |
sapier |
ok ok if I have nothing to do and a good idea how to fix it correct ... no use to use port 30001 for unit test as it'd just fail in a different case |
18:35 |
VanessaE |
right, I figure it should count upwards from 30000 until it finds a free port. |
18:35 |
VanessaE |
even on Oldcoder's system, it would only take like 17 tries :P |
18:47 |
sapier |
that's fine if there is a working port but if there's really a bug in there it'll run forever |
18:55 |
VanessaE |
good point. |
18:55 |
VanessaE |
well, easy enough to just disable the unit tests at the command line. |
19:00 |
|
OldCoder joined #minetest-dev |
19:42 |
|
artur99 joined #minetest-dev |
19:43 |
OldCoder |
Hello. artur99 here lives in Romania. He may offer to produce a Romanian translation. |
19:43 |
OldCoder |
If he makes such an offer, to whom should he be directed? |
19:43 |
OldCoder |
artur99, wait for an answer. If there is none, try again in a day or two. |
19:43 |
PilzAdam |
http://translate.minetest.ru/projects/minetest/core/ |
19:44 |
OldCoder |
artur99, review that link and PM me if you have further questions. PilzAdam thank you. |
19:48 |
|
sapier joined #minetest-dev |
19:51 |
OldCoder |
sapier, Hi. Animals are not producing errors despite the sdata nil problem. But I have not observed them yet. |
19:51 |
OldCoder |
Should I have seen cows by now? |
19:52 |
sapier |
yes there should be cows |
19:52 |
sapier |
they don't spawn in already generated areas |
19:52 |
sapier |
unless there have been cows before they should remain there |
19:52 |
OldCoder |
sapier, thank you |
19:53 |
OldCoder |
http://translate.minetest.ru/projects/minetest/core/ro/translate/?checksum=686e697538050e4664636337cc3b834f |
19:53 |
OldCoder |
PilzAdam or others, artur99 is starting on Romanian translation |
19:53 |
OldCoder |
We wish simply to ask if he is doing it right. Is that page what is needed? |
19:54 |
PilzAdam |
OldCoder, looks good so far |
19:54 |
OldCoder |
PilzAdam, thank you! |
19:56 |
OldCoder |
sapier, One more question for today (I am busy at work)... If worlds exist and are large, there may be no cows? Is there any way to add cows to existing places? |
19:56 |
|
iqualfragile joined #minetest-dev |
19:57 |
|
troller joined #minetest-dev |
19:57 |
|
proller joined #minetest-dev |
19:57 |
sapier |
yes there's a method called secondary spawning to populate generated areas it's disabled for performance reason but it may be ok to enable it for some time |
19:58 |
OldCoder |
sapier, where should I add this? |
19:58 |
* OldCoder |
wants to see cows and fish and so on |
19:58 |
OldCoder |
Moo! |
19:59 |
sfan5 |
meow! |
19:59 |
sapier |
it's a setting in gui ;-) and not very common used i'm gonna check how to enable per config file directly |
20:00 |
OldCoder |
sfan5, sapier has not yet made cats, has he? We should politely request this |
20:00 |
sapier |
and I need to check if there is a secondary spawner specified for cows ... not all mobs have it |
20:00 |
OldCoder |
sapier, thank you |
20:00 |
sfan5 |
Jordach once did a model for a cat in blender |
20:00 |
OldCoder |
Hmm... if you see him, ask him about it |
20:01 |
PilzAdam |
note that this channel should be used for engine development |
20:01 |
OldCoder |
PilzAdam, yes |
20:01 |
OldCoder |
PilzAdam, I am not always clear about what is engine |
20:01 |
OldCoder |
Ah |
20:01 |
OldCoder |
Ahnimals |
20:01 |
OldCoder |
sapier, respond in PM when you can |
20:01 |
OldCoder |
PilzAdam, but translations are engine? |
20:01 |
PilzAdam |
yes |
20:01 |
OldCoder |
Very well |
20:01 |
* OldCoder |
returns to work |
20:03 |
sapier |
PilzAdam do you have time to review the request queue fixes? |
20:03 |
PilzAdam |
no, I will have more time at weekend again |
20:10 |
kahrl |
sapier: is there a way to do it without making the result queue static? |
20:11 |
sapier |
complete redesign? |
20:11 |
kahrl |
hmm |
20:12 |
OldCoder |
kahrl, if I may ask, is httpfetch likely to merge this month? |
20:12 |
troller |
https://github.com/proller/minetest/compare/master...make |
20:12 |
kahrl |
OldCoder, I think so |
20:12 |
sapier |
pushing a pointer to a stack pointer into queue transfering it to another thread is fairly bad design this only didn't cause more problems because usually no timeouts occur |
20:12 |
sapier |
-stack pointer + stack object |
20:13 |
OldCoder |
kahrl, thank you |
20:13 |
kahrl |
OldCoder: sapier has been working on a merge of the async lua events and httpfetch, I haven't had the time to look at it in depth yet but it seems good so far |
20:14 |
OldCoder |
Very well |
20:14 |
* OldCoder |
will be happy to provide testing services for all of this; as in run multiple worlds with the new stuff |
20:15 |
kahrl |
sapier: maybe make the result queue reference counted? |
20:16 |
kahrl |
it starts off at 2, then the producer and the consumer thread each decrement it by 1 once they are done with it |
20:16 |
sapier |
we just removed smartpointers from game ;-) |
20:17 |
sapier |
but yes that'd work at cost of increased overhead |
20:19 |
kahrl |
does the result queue actually have to be a queue? if it's only used for one result some other overhead could be removed |
20:20 |
|
artur99 joined #minetest-dev |
20:20 |
kahrl |
using a semaphore or condvar to wait for the result (instead of calling sleep() in a loop) would remove some other overhead too |
20:21 |
artur99 |
hi! romania has this charachers : a-z and ăîâșț |
20:21 |
sapier |
yes seen that too it's next thing on my list |
20:21 |
artur99 |
when i make the translation i must use the ăîâțș ? |
20:21 |
sapier |
but I don't want to do everything at once |
20:23 |
Jordach |
sfan5, don't ping me for nothing |
20:23 |
PilzAdam |
artur99, yes, I guess so |
20:23 |
artur99 |
ok, thanks |
20:23 |
sapier |
imho making those queues static is change with lowest risk until a more complete design fix can be done |
20:24 |
kahrl |
yeah and I'm pretty sure it works right now |
20:24 |
kahrl |
(since there is only one thread other than main that makes requests) |
20:25 |
sapier |
hmm I don't se a way how the new version could fail at all ... but it's not very good design of course |
20:26 |
kahrl |
if there were several threads sharing the same result queue, they could compete with each other to clear each other's results |
20:26 |
kahrl |
and those results would be lost forever (unless I'm missing something) |
20:26 |
sapier |
true but is that function used by multiple threads? |
20:26 |
kahrl |
atm it is not |
20:27 |
sapier |
I assumed it to be thread specific but you're right that asumption is risky |
20:27 |
kahrl |
maybe once somebody wants to try a pool of mesh update threads we need something more sophisticated |
20:28 |
sapier |
imho a queue should be handled by some sort of control instance only responsible to start/stop consumers as well as providers |
20:28 |
sfan5 |
Jordach: I am so sorry |
20:28 |
Jordach |
sfan5, use J0rdach |
20:28 |
sapier |
this way control instance can enforce validity of all involved subcomponents |
20:29 |
|
damiel joined #minetest-dev |
20:29 |
sapier |
but no idea how much changes would be required to modify it this way |
20:34 |
|
bcnjr5_ joined #minetest-dev |
20:34 |
kahrl |
actually... I noticed something |
20:35 |
kahrl |
we wouldn't need a result queue at all, just some kind of indication that the main thread finishing producing the result |
20:35 |
kahrl |
since in all three places where ResultQueue is used, the main thread places the result into a cache anyway, so the result could be fetched from there |
20:36 |
kahrl |
s/finishing/finished |
20:36 |
sapier |
that cache is a result queue ;-) |
20:36 |
sapier |
but a global one |
20:36 |
kahrl |
right |
20:37 |
sapier |
would be fine with it too but there may be issues with scaling once more consumers/providers are involved |
20:38 |
sapier |
current design was meant to have one result queue per request thread thus having constant time to fetch a result |
20:38 |
sapier |
within a global queue result fetching time depends on number of already queued results |
20:38 |
|
werwerwer_ joined #minetest-dev |
20:39 |
sapier |
this wouldn't be an issue if it didn't matter what result is returned first |
20:39 |
sapier |
async works this way |
20:40 |
sapier |
btw the mutex queue isn't actually perfect for it ... it uses a events only (non counting semaphores) |
20:41 |
|
EvergreenTree joined #minetest-dev |
20:41 |
kahrl |
I thought it didn't even use events |
20:41 |
|
bas080 joined #minetest-dev |
20:42 |
sapier |
let me have a look I saw events when looking for this bug but I may have been in another thread by that time |
20:43 |
sapier |
but I don't intend to completely rewrite this for 0.4.8 I was hoping to get 0.4.8 done by end of this month |
20:44 |
kahrl |
as far as I'm concerned, anything that fixes the memory corruption on timeout and works otherwise is fine for 0.4.8 |
20:45 |
sapier |
you're right there isn't a event used in mutex queue at all |
21:05 |
|
RealBadAngel joined #minetest-dev |
21:26 |
|
Zeitgeist_ joined #minetest-dev |
21:31 |
* VanessaE |
is back |
21:43 |
VanessaE |
I just received a request, which I can't do but others might: an option has been requested to force objects that are drawn with allfaces_optional to be drawn, client-side only, with plantlike instead. E.g. all tree leaves. |
21:44 |
VanessaE |
the rationale is that plantlike won't lower the framerate like allfaces_optional does when fancy leaves are enabled, and looks better than a solid, opaque leaf block when that feature is disabled. |
21:44 |
VanessaE |
I can force this server-side with a very simple change to a couple of mods, but this seems like it would be more appropriate if done with a client-side config option. |
22:08 |
|
KingsleyT joined #minetest-dev |
22:15 |
|
ecube joined #minetest-dev |
22:26 |
hmmmm |
hey guys |
22:26 |
VanessaE |
hi |
22:26 |
hmmmm |
yesterday 0.4.8 was supposed to be released but I was out all day |
22:27 |
hmmmm |
kahrl, sapier, you guys have something really important getting cooked up at the moment |
22:27 |
hmmmm |
? |
22:27 |
VanessaE |
I thought it was delayed till the end of November? |
22:27 |
VanessaE |
(feature freeze) |
22:27 |
hmmmm |
who said that |
22:27 |
hmmmm |
the feature freeze is for a week |
22:27 |
VanessaE |
I don't recall now |
22:28 |
sapier |
I did request but noone did answer vanessae |
22:28 |
hmmmm |
just FYI we can release as often as we'd like |
22:28 |
hmmmm |
soooooooooog |
22:28 |
hmmmm |
I personally see no reason why not to release right now while we're past a week of feature freeze and nothing experimental has been added |
22:28 |
sapier |
hmmmmm for FYI we can't release a broken version at all ;-P |
22:28 |
hmmmm |
why is this version broken |
22:28 |
hmmmm |
last time i checked it was fine |
22:29 |
sapier |
it's broken because it's corrupting memory, invalid lua error handlers and stalls within modmgr |
22:29 |
hmmmm |
are you kidding |
22:29 |
hmmmm |
omg |
22:29 |
sapier |
everything could be patched right now if someone did agree ;-) |
22:29 |
|
bcnjr5 left #minetest-dev |
22:30 |
sapier |
no I'm not kidding we really do random memory corruptions in current master |
22:30 |
hmmmm |
I take it the corrupted memory errors are really uncommon? |
22:30 |
hmmmm |
because I haven't come across any related crashes yet |
22:30 |
sapier |
have a look in issue list there are various strange crash reports for years |
22:30 |
hmmmm |
for 'years'? |
22:30 |
sapier |
yes |
22:31 |
hmmmm |
well that makes it a bit different |
22:31 |
hmmmm |
you made it sound like it was the most urgent thing ever |
22:31 |
hmmmm |
do you know what's causing the problem? |
22:31 |
sapier |
I actually have a patch ready |
22:31 |
sapier |
yes I know why it happens |
22:31 |
hmmmm |
who is disagreeing with it then |
22:31 |
sapier |
nobody yet it hasn't been reviewed |
22:32 |
hmmmm |
then?/ |
22:32 |
hmmmm |
by all means let's see it |
22:32 |
ShadowNinja |
0.4.8 should come after pcall_errfunc and sqlite_rollback IMO... |
22:32 |
sapier |
how heavy is that sqlite rollback thing? |
22:32 |
hmmmm |
sqlite_rollback is a new feature not a bugfix... |
22:33 |
ShadowNinja |
sapier: What do you mean be heavy? |
22:33 |
hmmmm |
again, only bugfixes |
22:33 |
sapier |
https://github.com/sapier/minetest/tree/fix_RequestQueue this branch fixes two issues with request queue, the last commit is the random memory corruption fix |
22:33 |
ShadowNinja |
hmmmm: It is sorta a bugfix since it fixes Minetest eating GBs of memory and taking it's sweet time on rollback and rollback_check. |
22:33 |
hmmmm |
i'll take a closer look after i'm done eating |
22:33 |
sapier |
both are bugfixes but first one was that wrong noone could've ever used this |
22:34 |
hmmmm |
shadowninja, that's not a bugfix. that's a new feature. the current version is just a poor way of doing it |
22:34 |
hmmmm |
it can wait |
22:35 |
sapier |
Shadow as of rollback I think same way as hmmmm |
22:35 |
hmmmm |
yeah everybody's trying to get their features in after the feature freeze and a day after it's supposed to have been released |
22:35 |
hmmmm |
I'll check over those commits and then we'll have to ship that |
22:35 |
ShadowNinja |
Alright, well pcall_errfunc then. |
22:36 |
hmmmm |
yes, pcall_errfunc is fine |
22:36 |
hmmmm |
I'll check that over too after |
22:36 |
hmmmm |
by any luck it'll be out tonight |
22:37 |
ShadowNinja |
I have a tweak for it though that should make it much smaller (sugested by sapier). |
22:37 |
sapier |
you should fix this prior hmmmm reviews |
22:38 |
ShadowNinja |
With a shell script it will take only a minute, but without it will take a hour. And I don't know sed well enough. |
22:40 |
sapier |
come on :-) I'd have fixed that manually in 10 min ;-) |
22:41 |
sapier |
we're talking about the non required parameter flip right? |
22:45 |
ShadowNinja |
sapier: Yes, but that has to be changed in about 20-50 places. |
22:46 |
ShadowNinja |
~40 to be more precise. |
22:46 |
sapier |
while we've been talking you could've fixed it ... and fliping parameters without a good reason isn't quite usefulll .. no imho avoiding one "NULL" isn't a good reason (the other one is equalized by writing the default value) ;-P |
22:48 |
ShadowNinja |
There, I got a working sed command... |
22:48 |
ShadowNinja |
And I understand why they should be that way and agree. |
22:49 |
hmmmm |
alright now |
22:49 |
hmmmm |
hrmm, from your last commit in that branch it seems like the problem was that each caller would grab the same results, and you solved the issue by having each pass a key or something? |
22:49 |
sapier |
no |
22:50 |
sapier |
the problem was that a pointer to a stack object was pushed to another thread |
22:50 |
hmmmm |
whaa |
22:50 |
hmmmm |
who coded that |
22:50 |
sapier |
once a timeout occured that stack object wasn't valid any longer |
22:50 |
hmmmm |
oh, i see, request was global |
22:50 |
sapier |
I fixed it by enforcing the pointer to be valid |
22:51 |
sapier |
not best solution I know |
22:51 |
sapier |
but as long as there's only one thread doing the requests thats ok |
22:51 |
hmmmm |
why don't you just use heap memory |
22:51 |
sapier |
because someone needs to clean up |
22:51 |
hmmmm |
why can't the caller do that after he's done |
22:52 |
sapier |
the caller quits by timeout |
22:52 |
hmmmm |
this whole thing looks fubar |
22:52 |
sapier |
while the request may still sit within the queue |
22:53 |
sapier |
it is but complete redesign is way to heavy to fix the bug for 0.4.8 ... yes it has to be done and I intend to do so for 0.4.9 |
22:53 |
hmmmm |
well that's a relief |
22:53 |
sapier |
if you have a look at the previous commit in that branch you see multicaller support is completely messed up too |
22:54 |
hmmmm |
result_queue.pop_front() takes what, number of MS before timeout or something? |
22:54 |
hmmmm |
i'm looking at the other commits now |
22:54 |
sapier |
1 second |
22:55 |
hmmmm |
actually, hold on, I can't really read this on the github webpage |
22:55 |
hmmmm |
need more context |
22:55 |
sapier |
yes someone is doing terrible things if processing requires more then a second but corrupting memory isn't an option anyway :-) |
22:55 |
hmmmm |
so you're starting to add doxygen comments to things |
22:56 |
hmmmm |
zz |
22:56 |
sapier |
not even celeron55 didn't remember what this thing is intended to do ;-) |
22:56 |
hmmmm |
ahhhhhhhhhhhh THAT lump of code |
22:57 |
hmmmm |
oh what a bunch of crap |
22:57 |
hmmmm |
wasn't that the thing that EmergeThread used for queued block requests before I rewrote it? |
22:57 |
sapier |
actually it's intention was quite good but it didn't ever do what it was intended to do |
22:57 |
hmmmm |
see I figured it was just better to.... not.... attempt to fix it |
22:57 |
hmmmm |
hell I didn't even understand what the problem was exactly |
22:58 |
sapier |
hmmmm you should've fixed it now we have two queue implementations in there |
22:58 |
hmmmm |
my implementation isn't generic though |
22:58 |
sapier |
and a third one in async thread ... if it's possible I'm gonna merge them for 0.4.9 |
22:58 |
hmmmm |
it's not like a queue is something so advanced that it needs to be modularized |
22:59 |
hmmmm |
no it's fine, it's fine, don't bother |
22:59 |
hmmmm |
if it were more advanced code then I'd say you have a point |
22:59 |
sapier |
no it's not fine ;-) we need to fix 3 queues instead of one ;-) |
22:59 |
hmmmm |
but it doesn't do much, and if you try to generalize a single implementation, it won't be as well suited to all the other things |
22:59 |
sapier |
and you can do a lot of things wrong in a queue ;-) |
22:59 |
hmmmm |
also my queue implementation is not broken |
23:00 |
hmmmm |
soooooooo |
23:00 |
sapier |
:-) this one did work quite some time without everyone recognizing how broken it is ;-) |
23:00 |
hmmmm |
don't break my shit |
23:01 |
sapier |
:-) I'll ask again once I had a look if it is reasonable ... I don't know by now |
23:01 |
sapier |
but for now you should just chek if this fix can be added |
23:02 |
hmmmm |
so now there's no timing out |
23:02 |
sapier |
it is |
23:02 |
hmmmm |
you just keep looping until you get your item |
23:02 |
sapier |
no |
23:02 |
hmmmm |
I don't see any other breaks |
23:02 |
sapier |
timeout throws the exception |
23:03 |
hmmmm |
oh an exception |
23:03 |
hmmmm |
ahh, ItemNotFoundException |
23:03 |
sapier |
has been done this way befor |
23:03 |
hmmmm |
that's pretty stupid |
23:03 |
hmmmm |
people really need to stop playing with exceptions |
23:03 |
sapier |
yes it is but I didn't want to invest that much time to fix something I need to rework anyway |
23:03 |
hmmmm |
right, all this is temporary |
23:04 |
hmmmm |
sure |
23:04 |
hmmmm |
I can go for this fix for now |
23:05 |
ShadowNinja |
hmmmm: This error occurs a few times, can you fix it? http://pastebin.ubuntu.com/6418430/ |
23:05 |
sapier |
actually in this situation a exception isn't that bad ... if I'm gonna rewrite this I may remove the timeout ... for what sane reason should we miss to process a request? |
23:06 |
hmmmm |
let's say something gets TOTALLY screwed up |
23:07 |
hmmmm |
is it better to go around in an infinite loop and lock everything up |
23:07 |
hmmmm |
or just bomb out with an error |
23:07 |
sapier |
"TOTALLY screwed up" is synonym to assert for me ;) |
23:07 |
hmmmm |
let's be honest, if it times out there's something wrong anyway |
23:07 |
hmmmm |
yes, weren't there asserts there anyway? |
23:07 |
sapier |
wrong location |
23:07 |
ShadowNinja |
pcall_errfunc updated with sapier's tweak. |
23:07 |
sapier |
the asserts did check result.key == name |
23:08 |
sapier |
quite useless |
23:08 |
hmmmm |
oh |
23:09 |
sapier |
they git by pure chance sometimes :) |
23:09 |
sapier |
-git+hit |
23:09 |
hmmmm |
ShadowNinja, iirc I do a lot of the same thing all over |
23:09 |
hmmmm |
isn't there more than 1 of those warnings? |
23:10 |
ShadowNinja |
hmmmm: Yes, I got two I think. |
23:11 |
hmmmm |
that's a really stupid warning |
23:12 |
ShadowNinja |
hmmmm: Actually that's the only one I got, but I didn't "make clean" first. |
23:12 |
hmmmm |
does encountering that warning disable strict aliasing rules? |
23:13 |
hmmmm |
if so then there'd be a reason to change it |
23:13 |
hmmmm |
or does it disable strict aliasing for that group of expressions? |
23:13 |
hmmmm |
shrug |
23:14 |
kahrl |
since the called function is in a different translation unit, the warning can't change how it is compiled |
23:14 |
sapier |
the strict aliasing rules warning is serious |
23:14 |
sapier |
this may result in very strange errors |
23:15 |
hmmmm |
that's true |
23:15 |
hmmmm |
in this case I really don't know if it's worth fixing though, it's a stupid error |
23:16 |
hmmmm |
do they want me to start using templates for enums? |
23:19 |
sapier |
no idea I didn't have a closer look at that warning by now |
23:19 |
kahrl |
just change the type of rot to int |
23:19 |
hmmmm |
I know what it wants me to do but it's needlessly verbose |
23:19 |
hmmmm |
int roti; |
23:19 |
hmmmm |
pass roti |
23:19 |
hmmmm |
Rotation rot = (Rotation)roti; |
23:20 |
hmmmm |
I'm sure I would've fixed that warning if I had it on my compiler though |
23:20 |
sapier |
ok I need to sleep a little bit more early today so good night :-) |
23:20 |
kahrl |
since you only assign it to dschem.rotation anyway you can skip the last of those |
23:23 |
hmmmm |
gotta take a nap now |
23:23 |
hmmmm |
later |
23:40 |
|
sapier1 left #minetest-dev |
23:51 |
|
NakedFury joined #minetest-dev |