shinyblink / sled

Satanic/Sexy/Stupid/Silly/Shiny LED matrix controller
https://shinyblink.github.io/sled/
ISC License
122 stars 25 forks source link

Fixes vifino/sled#29 #30

Closed marenz2569 closed 6 years ago

marenz2569 commented 6 years ago

fixes vifino/sled#29

vifino commented 6 years ago

Thanks!

orithena commented 6 years ago

@marenz2569: Thanks for that fix, but it isn't quite equivalent (which is negligible in this particular case, since the current calling code and constants make sure that neither x nor delta is negative). What I need here is a modulo function for the ring [0..mod] that extends to the negative input range instead being mirrored for negative inputs. Which means abs(mod(x, m)) doesn't quite cut it. I'd need a function that returns 0.9 for the input f(-0.1, 1.0).

I wrote addmod in that way because it takes two comparisons in the average case of being used here; or four comparisons and one addition/subtraction in the average overflow case. I now see that it might run into deadlock in very special circumstances, thanks for spotting that.

Has anybody got a better idea of fixing addmod that has neither of these problems described above?

marenz2569 commented 6 years ago

@orithena just do something like this:

y = fmodf(x, m);
y += y<0?m:0;
orithena commented 6 years ago

@marenz2569 Okay, thanks, but I have to loop back to #29 here: How did you identify addmod() as the culprit for the deadlock in the first place? I know there's a deadlock in my two modules, but a bit of printf-debugging at start and end of addmod() revealed that addmod is done when the deadlock occures. I then sprinkled a few printfs in between the parts of draw() -- and got no deadlock since! (I left it running for 8 hours while I slept.)

Currently, the only explanation I have left is some weird optimization behaviour that doesn't come up with the added load of printfs... o.O Of course, since gfx_candyflow always initializes its internal runvariables semi-randomly, it might just be that the reason for the deadlock simply did not come up.

marenz2569 commented 6 years ago

@orithena I just started sled with gdb and stoped execution when it was putting full load on my cpu whilst initializing. Doing a backtrace revealed that it was stuck in the addmod function.

marenz2569 commented 6 years ago
GNU gdb (Debian 7.12-6+b1) 7.12.0.20161007-git
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
GEF for linux ready, type `gef' to start, `gef config' to configure
50 commands loaded for GDB 7.12.0.20161007-git using Python engine 3.6
[*] 6 commands could not be loaded, run `gef missing` to know why.
Reading symbols from ./sled...done.
gef➤  run
Starting program: /home/marenz/programming/sled/sled 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Loading modules...
    - flt_rot_90... Skipping unused filter modules.
    - flt_gamma_correct... Skipping unused filter modules.
    - gfx_sinematrix... Done.
    - gfx_candyflow... Done.
    - gfx_clock... Done.
    - gfx_gol... Done.
    - bgm_opc... Done.
    - gfx_balls... Done.
    - flt_debug... Skipping unused filter modules.
    - flt_flip_x... Skipping unused filter modules.
    - bgm_xyscope... Done.
    - gfx_affinematrix... Done.
    - gfx_golc... Done.
    - gfx_math_sinpi... Done.
    - gfx_plasma... Done.
    - gfx_text... Done.
    - flt_scale... Skipping unused filter modules.
    - gfx_sinefield... Done.
    - gfx_rainbow... Done.
    - gfx_mandelbrot... Done.
    - gfx_twinkle... Done.
    - gfx_checkerboard... Done.
    - out_sdl2... Done.
    - bgm_fish... Done.
    - gfx_error... Done.
    - gfx_cube... Done.
    - gfx_ip... Done.
    - flt_smapper... Skipping unused filter modules.
    - bgm_pixelflut... Done.
    - gfx_partirush... Done.
    - gfx_matrix... Done.
    - gfx_random_rects... Done.
    - gfx_random_static... Done.
    - flt_flip_y... Skipping unused filter modules.
Loaded 27 modules.
Initializing modules...
    - sinematrix... Done.
^C
Program received signal SIGINT, Interrupt.
────────────────────────────────────────────────────────────────────────────────────────────────────────────[ registers ]────
$rax   : 0x00056dcdc799ffd5
$rbx   : 0x0000000000000001
$rcx   : 0x00007ffff7065380  →  0x0000000000000000
$rdx   : 0x0000000000000000
$rsp   : 0x00007fffffffe340  →  0x0000000000000001
$rbp   : 0x00007fffffffe3b8  →  0x0000000000000011
$rsi   : 0x0000000000000000
$rdi   : 0x00007fffffffe310  →  0x000000005b1509fc
$rip   : 0x00007ffff6e63d7e  →  <init+131> jae 0x7ffff6e63d77 <init+124>
$r8    : 0x0000000000000009
$r9    : 0x000055555575d428  →  "candyflow"
$r10   : 0x00007fffffffe310  →  0x000000005b1509fc
$r11   : 0x00007fffffffe310  →  0x000000005b1509fc
$r12   : 0x000055555575d428  →  "candyflow"
$r13   : 0x0000000000000000
$r14   : 0x0000000000000000
$r15   : 0x0000000000000000
$cs    : 0x0000000000000033
$ss    : 0x000000000000002b
$ds    : 0x0000000000000000
$es    : 0x0000000000000000
$fs    : 0x0000000000000000
$gs    : 0x0000000000000000
$eflags: [zero carry parity adjust sign trap INTERRUPT direction overflow resume virtualx86 identification]
────────────────────────────────────────────────────────────────────────────────────────────────────────────────[ stack ]────
0x00007fffffffe340│+0x00: 0x0000000000000001     ← $rsp
0x00007fffffffe348│+0x08: 0x0000555555558c41  →  <modules_init+345> mov r13d, eax
0x00007fffffffe350│+0x10: 0x00000000ffffffff
0x00007fffffffe358│+0x18: 0x00000000ffffffff
0x00007fffffffe360│+0x20: 0x0000000000000000
0x00007fffffffe368│+0x28: 0x0000000000000000
0x00007fffffffe370│+0x30: 0x00007fffffffe7e0  →  0x0000000000000001
0x00007fffffffe378│+0x38: 0x0000555555556da0  →  <sled_main+763> mov ebp, eax
─────────────────────────────────────────────────────────────────────────────────────────────────────[ code:i386:x86-64 ]────
   0x7ffff6e63d6c <init+113>       cvtsi2sd xmm0, rcx
   0x7ffff6e63d71 <init+118>       addsd  xmm0, xmm0
   0x7ffff6e63d75 <init+122>       jmp    0x7ffff6e63dc3 <init+200>
   0x7ffff6e63d77 <init+124>       subss  xmm0, xmm1
   0x7ffff6e63d7b <init+128>       ucomiss xmm0, xmm1
 → 0x7ffff6e63d7e <init+131>       jae    0x7ffff6e63d77 <init+124> TAKEN [Reason: !C]
   ↳  0x7ffff6e63d77 <init+124>       subss  xmm0, xmm1
      0x7ffff6e63d7b <init+128>       ucomiss xmm0, xmm1
      0x7ffff6e63d7e <init+131>       jae    0x7ffff6e63d77 <init+124>
      0x7ffff6e63d80 <init+133>       jmp    0x7ffff6e63d86 <init+139>
      0x7ffff6e63d82 <init+135>       addss  xmm0, xmm1
      0x7ffff6e63d86 <init+139>       pxor   xmm2, xmm2
────────────────────────────────────────────────────────────────────────────────[ source:src/modules/gfx_candyflow.c+65 ]────
     61  /* helper function: add on a ring
     62   */
     63  static inline float addmod(float x, float mod, float delta) {
     64    x = x + delta;
 →   65    while( x >= mod ) {
     66      x -= mod;
     67    }
     68    while( x <  0.0 ) {
     69      x += mod;
──────────────────────────────────────────────────────────────────────────────────────────────────────────────[ threads ]────
[#0] Id 1, Name: "sled", stopped, reason: SIGINT
────────────────────────────────────────────────────────────────────────────────────────────────────────────────[ trace ]────
[#0] 0x7ffff6e63d7e → Name: addmod(delta=<optimized out>, mod=1, x=7.75688081e+12)
[#1] 0x7ffff6e63d7e → Name: init(moduleno=0x1, argstr=<optimized out>)
[#2] 0x555555558c41 → Name: modules_init(outmodno=0x7fffffffe3b8)
[#3] 0x555555556da0 → Name: sled_main(argc=<optimized out>, argv=<optimized out>)
[#4] 0x555555558de9 → Name: main(argc=<optimized out>, argv=<optimized out>)
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
0x00007ffff6e63d7e in addmod (delta=<optimized out>, mod=1, x=7.75688081e+12) at src/modules/gfx_candyflow.c:65
65    while( x >= mod ) {
marenz2569 commented 6 years ago

I compiled with: DEBUG = 1 CFLAGS += -ggdb

marenz2569 commented 6 years ago

This could take a lot of time :sweat_smile:: addmod(delta=<optimized out>, mod=1, x=7.75688081e+12)

marenz2569 commented 6 years ago

And to be honest reimplementing what cloud be done with division and subtraction like fmodf using a while loop can only go wrong.

orithena commented 6 years ago

@marenz2569 This is getting weirder with each new info... I do believe you, but I'm rather confused now as to why I found an instance where, according to my printf debugging, it got stuck outside addmod... except maybe it did got stuck in addmod, but the optimizer moved the printf call to the end of the function. Or output buffering took its toll somehow...

Anyway: If someone can get the contents of the runvars array when it gets stuck again, I appreciate it being posted here.

orithena commented 6 years ago

@marenz2569 The interesting part to me is: How did it get out of whack that badly? (yeah, I know, I'm gonna need to check float arithmetic edge cases to fully understand this.) Anyway, you are right in that we shouldn't reinvent the wheel.

Your trace log also helps a lot, thanks for that! I see why it might go wrong during init(), but still can't make out why the module sometimes gets stuck after some time running (unless, of course, init() gets called again, which should not happen according to @vifino).

I'm going to fix some stuff with what I learned here (but probably not today).

Thanks again for your patience, @marenz2569!

vifino commented 6 years ago

Hi! First off, sorry @orithena for preemptively merging without consulting you first, I assumed it to do much the same. I also had the deadlock, which is why I wanted this merged badly.

init() only gets called again after a deinit(). Right now, it only gets called once, but it might get reinited incase the resolution changes. Future out_sdl2 work? Yay!

orithena commented 6 years ago

I now was able to look into the stacktrace when it gets stuck after a while... looks like a timer overflow issue on 32bit. Moving to #35.