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

libandroidaudioplugin.so causes freezes at Preset extension on plugins with -fvectorize in -O2 optimization flag #194

Closed
atsushieno opened this issue Feb 5, 2024 · 9 comments
Labels

Comments

@atsushieno
Copy link
Owner

Current libandroidaudioplugin.so in androidaudioplugin.aar seems to cause problem when it is built with the default AGP/NDK setup for the release build (CMAKE_BUILD_TYPE=RelWithDebInfo, where CMAKE_CXX_FLAGS is initialized as -O2 -g -DNDEBUG). With the default build, aap-juce-hera, for example, does not let us get presets on either aap-juce-simple-host or aaphostsample.

The problem here is, interestingly, -O2.

Adding this externalNativeBuild block fixes the problem i.e. changing the optimization flag from -O2 to -O1 (in other words, changing the snippet below from -O1 to -O2 brings back the problem):

android {
    buildTypes {
        release {
            externalNativeBuild {
                cmake {
                    arguments ("-DCMAKE_CXX_FLAGS_RELWITHDEBINFO=-O1 -g -DNDEBUG")
                }
            }
<snip>

It is similar to #174, but unlike that issue which was basically due to our faulty code, it is only about optimization differences that seems more likely the compiler issue.

atsushieno added a commit that referenced this issue Feb 5, 2024
context: #194

It would disable some optimizations in libandroidaudioplugin.so...
@atsushieno
Copy link
Owner Author

I added the workaround ^, but would leave this issue open until we figure out how to get some repro code or have this issue fixed somehow.

@atsushieno
Copy link
Owner Author

Thanks to this SO thread https://stackoverflow.com/questions/15548023/clang-optimization-levels I could figure out it is -fvectorize

@atsushieno atsushieno changed the title libandroidaudioplugin.so causes freezes at Preset extension on aap-juce plugins with -O2 optimization flag libandroidaudioplugin.so causes freezes at Preset extension on aap-juce plugins with -fvectorize in -O2 optimization flag Feb 5, 2024
@atsushieno
Copy link
Owner Author

It is most likely a regression in NDK 24.0.8215888. 23.2.8568313 runs fine with -fvectorize.

This experiment requires the following changes:

  • r23 does not compile the AIDL generated code by build-tools 33.0.1. Therefore, when I tried r23 I downgraded build-tools to 30.0.3 and ran androidaudioplugin/src/main/update-binder.sh.
  • -Wunused-but-set-variable does not exist in the clang version bundled in r23 and we need to comment it out in aap_midi2_helper.h at least.
  • Then r24 started to treat it as warning, and with -Werror it is treated as an error. Since r24 and r25 behaves differently on what to warn out, I simply disabled -Werror throughout the experiment.

Switching only between r23 and r24 shows the difference.

atsushieno added a commit that referenced this issue Feb 5, 2024
Build and install to mavenLocal with: `./gradlew publishToMavenLocal`

(no `build` this time, as it will fail at aapinstrumentsample)

Build and run `aapinstumentsample` on Android Studio (any version; I used
Jellyfish)

Select the only one item in the list and tap "Start" -> "-- Presets --"

with ndk r23 (as indicated in gradle/libs.versions.toml), it shows the items.

with ndk r24 the UI freezes.
@atsushieno atsushieno changed the title libandroidaudioplugin.so causes freezes at Preset extension on aap-juce plugins with -fvectorize in -O2 optimization flag libandroidaudioplugin.so causes freezes at Preset extension on plugins with -fvectorize in -O2 optimization flag Feb 5, 2024
@atsushieno
Copy link
Owner Author

It is reproducible without aap-juce; aapinstrumentsample, when AAP dependencies are resolved from mavenLocal instead of project(...), can also trigger the same problem.

@atsushieno
Copy link
Owner Author

According to toolchains/llvm/prebuit/darwin-x86_64/AndroidVersion.txt files, NDK r23 is based on llvm 12.0.9 and r24 is based on 14.0.1.

As long as I look at git history for llvm-project/llvm/lib/Transforms/Vectorize between the 12 tag and the 14 tag, it was likely introduced somewhere between 28410d1 and e188aae in llvm-project. The history diffs are huge and I don't think I can look for the cause without any knowledge about the codebase.

@atsushieno
Copy link
Owner Author

Thanks to this SO thread https://stackoverflow.com/questions/44635314/how-to-compile-with-different-compile-options-for-different-files-in-cmake I could narrow down the trigger source file to just one file: e375f03

@atsushieno
Copy link
Owner Author

^ was enough only for aapinstrumentsample, not enough for aap-juce plugins (at least on aap-juce-hera)...

atsushieno added a commit that referenced this issue Feb 6, 2024
With this change, most of libandroidaudioplugin.so code are still optimized
just like -O2.
@atsushieno
Copy link
Owner Author

Actually just de-optimizing aap_midi2_helper.cpp was fine. It looked wrong because my build.gradle.kts change experiment was wrong. Now main branch optimizes most of the sources back.

atsushieno added a commit that referenced this issue Feb 6, 2024
…as #pragma.

It seems we can specify -fvectorize without conditionally applying to the
sources, now that we have general vectorization avoidance in cmidi2.h (which
seemed to be the occurrence point of this issue).

I thought we could simply go back to -O2, but such a build results in the
same UI freeze problem, so I leave it as -O1 + a bunch of optimization options.
@atsushieno
Copy link
Owner Author

It is now shrunk to only two functions in cmidi2.h: atsushieno/cmidi2@06919c7

This would be good enough to give a bug report to the NDK team. I did good job by myself enough.

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

No branches or pull requests

1 participant