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

Add native system semaphore and Windows shared memory #7212

Merged
merged 6 commits into from
Apr 20, 2024

Conversation

DomClark
Copy link
Member

The primary goal here is to eliminate the Qt requirement for RemoteVstPlugin. This helps compatibility with Qt 6, which does not provide 32-bit binaries any more. It also helps with the more general effort to remove Qt from the core, and simplifies the CMake scripts for RemoteVstPlugin a bit. RemoteZynAddSubFx still needs Qt for XML processing, but that can be dealt with separately.

Changes:

  • Remove Qt shared memory implementation.
  • Add native Windows shared memory implementation.
  • Add native system semaphore with POSIX and Windows implementations. (The POSIX version is currently unused, as a different IPC mechanism is used on POSIX systems that doesn't use semaphores. I have tested it on a different branch in the past, however.)
  • Tidy up some of the existing shared memory code.
  • Remove Qt from RemoteVstPlugin CMake scripts, and thus install only a single Qt version in Windows CI.
  • Copy the fix from Fix generator expression in strip command #6762 into the RemoteVstPlugin CMake script, as I forgot to do so at the time.

Copy link
Contributor

@Veratil Veratil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing stands out. Haven't tested but LGTM.

@Rossmaxx
Copy link
Contributor

Yes, finally. Don't know what to test or review for in this but I'm glad to help here.

@DomClark DomClark merged commit bda042e into LMMS:master Apr 20, 2024
9 checks passed
@DomClark DomClark deleted the native-semaphore-and-shm branch April 20, 2024 22:21
@Rossmaxx
Copy link
Contributor

Rossmaxx commented Jul 7, 2024

A user on discord has stated that CPU usage has increased in the nightly builds when using VSTs compared to 1.2. Is this a regression from this change?

@DomClark
Copy link
Member Author

DomClark commented Jul 7, 2024

I don't know - there's so little information about the problem and so many changes since 1.2 that there's no way to confidently answer with "yes" or "no". However, it seems unlikely to me, given that the only code in this PR that runs many times per second (in particular, acquiring and releasing semaphores) is very simple.

If they are automating VST parameters, then the likely cause is #5321, which is a known issue. That feature is difficult to implement efficiently with the current IPC system, which is why I'm working on #6511. Otherwise, we need more information and more careful investigation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants