Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Many FP exceptions thrown all the time compared to original version #78

Open
jcelerier opened this issue Jan 4, 2025 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@jcelerier
Copy link

Check latest release
I'm on git head

Describe the bug

I'm developing https://ossia.io which uses ysfx. So far I've been using jpcima's version, but I am looking into upgrading to this fork. But here SIGFPE is raised very often. Also from quick tests, the performance seems worse in AArch64 - I can chain only a few plug-ins while I can chain dozens with the jpcima version.

To Reproduce

This is how I'm loading YSFX plug-ins:

  ysfx_guess_file_roots(config.get(), arr.c_str());

  fx.reset(ysfx_new(config.get()), ysfx_u_deleter{});
  if(!ysfx_load_file(fx.get(), arr.c_str(), 0))
    return;

  uint32_t compile_opts = 0;
  if(!ysfx_compile(fx.get(), compile_opts))
    return;

  ysfx_init(fx.get()); // Here I see SIGFPE thrown when instantiating saike_protosynth

  ysfx_process_double(fx.get(), 0, 0, 0, 0, 0);

I also get some very regularly with Saike_Pitch_Shift, when playing with the semitones slider.

Expected behavior
No SIGFPE when loading plug-ins or playing with them.

Desktop (please complete the following information):

  • OS: Asahi Linux (Fedora AArch64)
  • DAW: ossia score (https://ossia.io) - I'm the developer
  • YSFX version: git head
@jcelerier jcelerier added the bug Something isn't working label Jan 4, 2025
@JoepVanlier JoepVanlier self-assigned this Jan 5, 2025
@JoepVanlier
Copy link
Owner

JoepVanlier commented Jan 7, 2025

Hello!

I'm developing https://ossia.io/ which uses ysfx.

Looks like a great project!

So far I've been using jpcima's version, but I am looking into upgrading to this fork

Cool! As you can probably tell, this fork isn't very mature yet. I've also had to make a few breaking changes to support some newer JSFX features unfortunately.

But here SIGFPE is raised very often

Thanks for bringing this to my attention. I've managed to see some of the FPEs you mention on my M1 (running MacOS though, so they are sent as SIGILL instead of SIGFPE) when explicitly enabling them. I don't have access to an AArch64 linux box at the moment though. Do you see a big difference in number of exceptions raised between the jpcima version and this fork?

ysfx_init(fx.get()); // Here I see SIGFPE thrown when instantiating saike_protosynth

Reproduced, thanks. The issue with protosynth was a bug in protosynth itself (division by zero in part of the init code).

I also get some very regularly with Saike_Pitch_Shift, when playing with the semitones slider.

The pitch shifter also had an issue where a division by zero could occur when running it with no audio (haven't been able to reproduce what's happening when moving the semitones slider). Note that this would be triggered every time ysfx_init is called on this fork, while it wouldn't on the original version, since that one doesn't reinitialize variables on init. The REAPER implementation resets all non built-in variables for JSFX that do not specify an @serialize section.

AFAIK, floating point division by zero should be safe to ignore in the context of JSFX, since NaN and infinities get flushed to zero by EEL after each operation. That said, I agree that YSFX should not allow those exceptions to bubble up, so I will look into silencing them and restoring the exception env and fp exception flags when handing control back. I'll report back when I make some progress on this. I'm pretty new to this type of multi-platform development, so I would really appreciate some feedback on this once I have a working prototype.

I can chain only a few plug-ins while I can chain dozens with the jpcima version.

I would expect a small performance penalty due to more channels being exposed by default and more sliders being available, but nothing quite so extreme. My guess is I'm not able to reproduce yet what you are seeing here. Which plugins are you comparing performance with? Are they also the ones that issue many SIGFPEs?

@jcelerier
Copy link
Author

AFAIK, floating point division by zero should be safe to ignore in the context of JSFX, since NaN and infinities get flushed to zero by EEL after each operation. That said, YSFX should not allow those exceptions to bubble up, so I will look into silencing them and restoring the exception env and fp exception flags when handing control back. I'll report back when I make some progress on this. I'm pretty new to multi-platform development, so I would really appreciate some feedback on this once I have a working prototype.

If that can be useful here is what I implemented in ossia after seeing this from a few hours of stackoverflow-ing, as this is I believe a generally useful feature to have anyways. Note that this does both disabling FPE and forcing flush to zero / disabling denormals. Feel free to reuse! It passes on CI on mac (x64 & arm), window (x64), linux (x64 & arm), and freebsd (ossia/score#1663)

https://github.com/ossia/libossia/blob/master/src/ossia/detail/disable_fpe.cpp

Which plugins are you comparing performance with? Are they also the ones that issue many SIGFPEs?

I'll try to make a proper reproduced and gather some numbers in the next few days, keeping you posted. Great work with the new features in any case ! especially support for the EEL preprocessor was becoming quite critical for many effects.

@JoepVanlier
Copy link
Owner

If that can be useful here is what I implemented in ossia after seeing this from a few hours of stackoverflow-ing, as this is I believe a generally useful feature to have anyways. Note that this does both disabling FPE and forcing flush to zero / disabling denormals. Feel free to reuse! It passes on CI on mac (x64 & arm), window (x64), linux (x64 & arm), and freebsd (ossia/score#1663)

https://github.com/ossia/libossia/blob/master/src/ossia/detail/disable_fpe.cpp

Thank you! Very helpful! I'll have a closer look at it next time I have a solid chunk of time to work on this.

I'll try to make a proper reproduced and gather some numbers in the next few days, keeping you posted.

Much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants