Skip to content

Conversation

matthargett
Copy link

@matthargett matthargett commented Sep 28, 2025

Bump the NDK just below unstable r29 and SDK API level 36 that appear to be necessary for building for Android XR devices. Note that targeting API Level 35 is mandatory for publishing new apps to Google Play since August 2025, so this is a bit overdue for those reasons as well.

The C++20 incompatibility highlighted by newer clang in NDK 28 was highlighted by a similar downstream PR in BabylonNative: BabylonJS/BabylonNative#1552

I tried to also use the Android system-installed v8 library, rather than building from a separate source, so that profile-optimized system version that received security updates keeps apps performant. The performance aspect is key for 3D/XR workloads, and the security aspect is key when BabylonJS is processing user-supplied data by opening and rendering splats, behavior graphs, WGSL shaders, etc. It was not straightforward, and so I'm leaving that as a separate step in the future PR.

matthargett and others added 27 commits September 27, 2025 17:40
This change updates the Azure Pipelines CI for Android to use NDK version 28.2.13676358 and SDK API level 35.

The previous configuration used an outdated NDK and SDK, causing the build to fail with an "NDK is not installed" error.

This commit addresses the issue by:
- Updating the `ndkVersion` variable in `azure-pipelines.yml`.
- Modernizing the Android job in `jobs/android.yml` to:
  - Use the `macos-latest` VM image.
  - Install the correct NDK and SDK versions.
  - Create an AVD with the new system image.
  - Use JDK 17 for the Gradle build.
Fix: Update Android CI to use NDK 28 and API 35
This change updates the Azure Pipelines CI for Android to use NDK version 28.2.13676358 and SDK API level 35.

The previous configuration used an outdated NDK and SDK, causing the build to fail. This commit addresses the issue by:
- Updating the `ndkVersion` variable in `azure-pipelines.yml`.
- Modernizing the Android job in `jobs/android.yml` to:
  - Use the `macos-latest` VM image.
  - Install the correct NDK and SDK versions.
  - Create an AVD with the new system image.
  - Use JDK 17 for the Gradle build.
  - Add `_JAVA_OPTIONS` to fix `NoClassDefFoundError` with `sdkmanager` on newer JDKs.
Updated required tools and minimum Android version in README. Added link to Android OS measured usages globally. Note specifically Android-base XR device coverage that Android 10 and up includes. This *does* exclude Oculus Go (which was left on Android 7 before EOL), which we can discuss further if coverage of that device is deemed critical.
…it explicitly now. Update README to be specific about minimums and why.
… size cost to downstream users, as this JSC includes full intl build. It might still be worth it based on the JIT and GC performance improvements that shoudl give noticeable uplift to downstream users.
… which should also allow some of the explicit VM flags to be eliminated
…lding everything else back. It should be discussed why Chakra support is still needed in the OSS repo.
…g out in CI. this worked okay on my local macbook before, so I'm assuming swap was the problem. this makes it faster on my local macbook, hopefully fixes it in CI.
…simulator. TypedArray test has some endianness specifics, skipping it temporarily. use the globalThis polyfill more consistently.
@CedricGuillemet
Copy link
Contributor

test failing on Android is

10-04 00:11:49.226  3921  4067 I JavaScript: [log]       1) should detect JavaScript engine type
10-04 00:11:49.227  3921  4067 I JavaScript: [error]
10-04 00:11:49.227  3921  4067 I JavaScript: [FAILED] JavaScript Engine Compatibility Engine Detection should detect JavaScript engine type
10-04 00:11:49.228  3921  4067 I JavaScript: [error]   File: unknown
10-04 00:11:49.228  3921  4067 I JavaScript: [error]   Error: expected false to be true
10-04 00:11:49.228  3921  4067 I JavaScript: [error]   Stack: AssertionError: expected false to be true
10-04 00:11:49.228  3921  4067 I JavaScript:              at Context.<anonymous> (app:///Scripts/tests.js:73:84)
10-04 00:11:49.228  3921  4067 I JavaScript:              at callFn (app:///Scripts/tests.js:19712:21)

… apps that are testing deployments of different JS engines can include which runtime is being used in their JS-based telemetry (which is preferable over native telemetry because it can be updated OTA and while native app container is backgrounded), but it's proven to be too volatile. I could delete it, but preferring to leave it skipped here as a placeholder.
@CedricGuillemet
Copy link
Contributor

  1. Yes, we still want to support Chakra
  2. None that I remember @bghgary @SergioRZMasson
  3. No problem to me

@bghgary
Copy link
Contributor

bghgary commented Oct 10, 2025

I'm working on providing PRs to BabylonNative's current sources for v8 and JSC binaries

I don't know what this means. Can you elaborate?

…or leak checker) in NDK 28 fails on the unit tests because the AndroidLogger leaks file descriptors when ::Start() is called twice. AndroidLogger also has no way to query if its already been started. So, we workaround with a process-wide global to prevent multiple ::Start() calls on the logger
@matthargett
Copy link
Author

  1. Yes, we still want to support Chakra

can we narrow to newer Chakra by setting the pipeline to test on windows-2022 image (and later)? Windows 10 1809 Chakra feature set (globalThis), etc would have made it easier to expand the suite with fewer fallback acrobatics.

@matthargett
Copy link
Author

I'm working on providing PRs to BabylonNative's current sources for v8 and JSC binaries

I don't know what this means. Can you elaborate?

Bumping the v8 version basis for:
https://www.npmjs.com/package/v8-android-jit-nointl-nosnapshot
https://github.com/Kudo/v8-android-buildscripts

and the JSC/WebKit basis for jsc-android upstream packages:
https://www.npmjs.com/package/jsc-android
https://github.com/react-native-community/jsc-android-buildscripts

The goal being, on Android (XR), get the v8 and JSC runtimes aligned in capability as a pre-step to bringing in Hermes and its capabilities. The spread of what JS transpile works results in a lowest-common denominator that impeded BabylonJS app performance on standalone XR hardware. With the VMs more aligned in capability, the vite/rspack/webpack configuration can be better optimized for modern built-ins and language capabilities.

…ller service isn't ready to receive and apk install over adb yet. Synchronize on installer service being ready, increase the overall timeout a bit, and retry.
…he unit tests contact numerous public internet services that sometimes time out or return an error. This time, we ere getting a 502 return instead of the expected 200. In a future PR, I'll change this so it runs local temporary services that totally removes the flakiness of the public internet so this CI failure noise is eliminated.
@bghgary
Copy link
Contributor

bghgary commented Oct 10, 2025

  1. Yes, we still want to support Chakra

can we narrow to newer Chakra by setting the pipeline to test on windows-2022 image (and later)? Windows 10 1809 Chakra feature set (globalThis), etc would have made it easier to expand the suite with fewer fallback acrobatics.

We will have to check with our partners, but globalThis is pretty easy to polyfill with node-api. Something like:

Env().Global().Set("globalThis", Env().Global());

Is there an issue with this?

@bghgary
Copy link
Contributor

bghgary commented Oct 10, 2025

I'm working on providing PRs to BabylonNative's current sources for v8 and JSC binaries

I don't know what this means. Can you elaborate?

Bumping the v8 version basis for: https://www.npmjs.com/package/v8-android-jit-nointl-nosnapshot https://github.com/Kudo/v8-android-buildscripts

and the JSC/WebKit basis for jsc-android upstream packages: https://www.npmjs.com/package/jsc-android https://github.com/react-native-community/jsc-android-buildscripts

The goal being, on Android (XR), get the v8 and JSC runtimes aligned in capability as a pre-step to bringing in Hermes and its capabilities. The spread of what JS transpile works results in a lowest-common denominator that impeded BabylonJS app performance on standalone XR hardware. With the VMs more aligned in capability, the vite/rspack/webpack configuration can be better optimized for modern built-ins and language capabilities.

In BabylonNative, we only bring in the binaries for testing the apps folder. Do you mean you are going to add code that depends on new features of JSC or V8?

@matthargett
Copy link
Author

  1. Yes, we still want to support Chakra

can we narrow to newer Chakra by setting the pipeline to test on windows-2022 image (and later)? Windows 10 1809 Chakra feature set (globalThis), etc would have made it easier to expand the suite with fewer fallback acrobatics.

We will have to check with our partners, but globalThis is pretty easy to polyfill with node-api. Something like:

Env().Global().Set("globalThis", Env().Global());

Is there an issue with this?

I added new tests and made some gross assumptions about 2019-era JS features being available, and had to iterate a lot to make the Server 2019 version of Chakra (which is a different version/update than consumer/desktop Windows 10) work. I appreciate its only for local tests and maybe doesn't affect downstream app builders, but it's was a friction in this contribution. I did polyfill it in JS, in the meantime (which is what took a lot of iteration).

@matthargett
Copy link
Author

I'm working on providing PRs to BabylonNative's current sources for v8 and JSC binaries

I don't know what this means. Can you elaborate?

Bumping the v8 version basis for: https://www.npmjs.com/package/v8-android-jit-nointl-nosnapshot https://github.com/Kudo/v8-android-buildscripts
and the JSC/WebKit basis for jsc-android upstream packages: https://www.npmjs.com/package/jsc-android https://github.com/react-native-community/jsc-android-buildscripts
The goal being, on Android (XR), get the v8 and JSC runtimes aligned in capability as a pre-step to bringing in Hermes and its capabilities. The spread of what JS transpile works results in a lowest-common denominator that impeded BabylonJS app performance on standalone XR hardware. With the VMs more aligned in capability, the vite/rspack/webpack configuration can be better optimized for modern built-ins and language capabilities.

In BabylonNative, we only bring in the binaries for testing the apps folder. Do you mean you are going to add code that depends on new features of JSC or V8?

If the JS code I can idiomatically write in my own BabylonJS app can be aligned with the JS code I contribute in the testing apps, it would lower the cognitive load for myself (and hopefully) other users to contribute.

@bghgary
Copy link
Contributor

bghgary commented Oct 10, 2025

I added new tests and made some gross assumptions about 2019-era JS features being available, and had to iterate a lot to make the Server 2019 version of Chakra (which is a different version/update than consumer/desktop Windows 10) work. I appreciate its only for local tests and maybe doesn't affect downstream app builders, but it's was a friction in this contribution. I did polyfill it in JS, in the meantime (which is what took a lot of iteration).

I guess I'm a bit confused or I'm missing something. All of our pipelines that I can find run on the windows-latest image. Where are there pipelines that are running on 2019?

@bghgary
Copy link
Contributor

bghgary commented Oct 10, 2025

If the JS code I can idiomatically write in my own BabylonJS app can be aligned with the JS code I contribute in the testing apps, it would lower the cognitive load for myself (and hopefully) other users to contribute.

We are in the process of converting all the app JavaScript code to use TypeScript. We will likely need use babel to translate to something that Chakra can understand. Then you should be able to contribute code without thinking about the downlevel support in Chakra.

@matthargett
Copy link
Author

I added new tests and made some gross assumptions about 2019-era JS features being available, and had to iterate a lot to make the Server 2019 version of Chakra (which is a different version/update than consumer/desktop Windows 10) work. I appreciate its only for local tests and maybe doesn't affect downstream app builders, but it's was a friction in this contribution. I did polyfill it in JS, in the meantime (which is what took a lot of iteration).

I guess I'm a bit confused or I'm missing something. All of our pipelines that I can find run on the windows-latest image. Where are there pipelines that are running on 2019?

I assume this was the case, because Chakra has had globalThis feature flag enabled since the ~2020 Windows 10 release build I mentioned. Since you mentioned that the TypeScript setup will transpile down to whatever the Chakra runtime used will support, that solves the problem also.

@matthargett
Copy link
Author

Integration tested in BabylonNative PR BabylonJS/BabylonNative#1552

tested macOS native build and both Android variants in a Pixel 2 simulator running Android 10, unit tests pass and test app works as expected in both places.

@bghgary
Copy link
Contributor

bghgary commented Oct 13, 2025

I assume this was the case, because Chakra has had globalThis feature flag enabled since the ~2020 Windows 10 release build I mentioned. Since you mentioned that the TypeScript setup will transpile down to whatever the Chakra runtime used will support, that solves the problem also.

I'm running Chakra locally on my Windows 11 box with Babylon Native and globalThis is not defined. I don't know why it would not be available if there is a feature flag that enables it. I have not heard of this flag before. For me, Chakra has never had globalThis. Can you show me the source for where this feature flag is enabled for the Windows build?

I haven't looked into what babel does with globalThis if used in TypeScript. I suspect it does something reasonable, but I don't know much about it.

@matthargett
Copy link
Author

I assume this was the case, because Chakra has had globalThis feature flag enabled since the ~2020 Windows 10 release build I mentioned. Since you mentioned that the TypeScript setup will transpile down to whatever the Chakra runtime used will support, that solves the problem also.

I'm running Chakra locally on my Windows 11 box with Babylon Native and globalThis is not defined. I don't know why it would not be available if there is a feature flag that enables it. I have not heard of this flag before. For me, Chakra has never had globalThis. Can you show me the source for where this feature flag is enabled for the Windows build?

chakra-core/ChakraCore@3517001

It's totally possible that my colleagues at BlueJeans (RIP) enabled it in the local build (we had to ship on Windows 7 and built our own profile-optimized DLL), and it was never enabled in Windows.

I haven't looked into what babel does with globalThis if used in TypeScript. I suspect it does something reasonable, but I don't know much about it.

Regardless, I've workaround it in the new unit tests and me and my team are technical enough to get into micro details on bundling our app so it's optimized for whichever modern VM we deploy. I'm cool if you're cool :)

@bghgary
Copy link
Contributor

bghgary commented Oct 15, 2025

chakra-core/ChakraCore@3517001

It's totally possible that my colleagues at BlueJeans (RIP) enabled it in the local build (we had to ship on Windows 7 and built our own profile-optimized DLL), and it was never enabled in Windows.

ChakraCore is not Chakra. ChakraCore is the open-source version of Chakra. Chakra ships with the OS. Sometimes changes to ChakraCore will be ported back to Chakra but I'm guessing it's not likely nowadays. Babylon Native only supports Chakra right now. We could in theory also support ChakraCore but that's not currently available.

@matthargett
Copy link
Author

chakra-core/ChakraCore@3517001
It's totally possible that my colleagues at BlueJeans (RIP) enabled it in the local build (we had to ship on Windows 7 and built our own profile-optimized DLL), and it was never enabled in Windows.

ChakraCore is not Chakra. ChakraCore is the open-source version of Chakra. Chakra ships with the OS. Sometimes changes to ChakraCore will be ported back to Chakra but I'm guessing it's not likely nowadays. Babylon Native only supports Chakra right now. We could in theory also support ChakraCore but that's not currently available.

Sorry, I was confused about which had which feature enabled. I'm not asking to change, and if your internal customers are okay with it, eliminating Chakra would help simplify things a bit. It's not a big deal, so if there's any complications that make keeping it around a business necessity, that's okay.

@matthargett
Copy link
Author

@CedricGuillemet any more comments on these changes or requests for additional testing before approving/merging?

@bghgary
Copy link
Contributor

bghgary commented Oct 15, 2025

ChakraCore is not Chakra. ChakraCore is the open-source version of Chakra. Chakra ships with the OS. Sometimes changes to ChakraCore will be ported back to Chakra but I'm guessing it's not likely nowadays. Babylon Native only supports Chakra right now. We could in theory also support ChakraCore but that's not currently available.

Sorry, I was confused about which had which feature enabled. I'm not asking to change, and if your internal customers are okay with it, eliminating Chakra would help simplify things a bit. It's not a big deal, so if there's any complications that make keeping it around a business necessity, that's okay.

We can't remove support for Chakra. This is used by a number of partners. Once we have babel in the apps build process, there should be minimal difference.

Copy link
Contributor

@bghgary bghgary left a comment

Choose a reason for hiding this comment

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

Did a quick pass.

Comment on lines 38 to 51
static constexpr int compare(const char_type* s1, const char_type* s2, size_t n) noexcept {
for (size_t i = 0; i < n; ++i) {
if (s1[i] < s2[i]) return -1;
if (s1[i] > s2[i]) return 1;
}
return 0;
}

static constexpr const char_type* find(const char_type* s, size_t n, const char_type& c) noexcept {
for (size_t i = 0; i < n; ++i) {
if (s[i] == c) return s + i;
}
return nullptr;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I only see length being used. Where are these two functions used?

Copy link
Author

Choose a reason for hiding this comment

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

I thought I needed the statics to complete the template specialization, but maybe not?

Copy link
Contributor

Choose a reason for hiding this comment

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

This code is adding a template specialization, but it is not a specialization of std::char_traits. This is just a custom struct with some static functions. You can probably just not use templates at all since it's just a simple function call to get the length.

Comment on lines 20 to 21
// Temporarily disable StdoutLogger due to fdsan issue with NDK 28
// android::StdoutLogger::Start();
Copy link
Contributor

Choose a reason for hiding this comment

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

Temporary in what way? Will this come back?

Copy link
Author

Choose a reason for hiding this comment

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

This will require a fix in AndroidExtensions repo to avoid the fd sanitizer issues (which were likely causing intermittent instability during app shutdown). Happy to do it.

});
});

describe("N-API Compatibility", function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

How are these tests related to N-API? I don't see any code here that is related to N-API. Can you explain?

});
});

describe("Unicode and String Encoding", function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we testing JavaScript features?

Copy link
Author

Choose a reason for hiding this comment

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

this is meant to cover some aspects of integration that other JS runtime proxies have had issues with in the past. after the N-API crash bug in this repo's code that only occurred when JavaScriptCore was used, I wanted to add some small regression tests to get more coverage when running the local tests under the sanitizers. I think these will be useful when testing integrations against newer version of v8, JSC, etc as well. Still, if you think they're orthogonal, I can remove them.

Copy link
Contributor

@bghgary bghgary Oct 16, 2025

Choose a reason for hiding this comment

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

Maybe I need more context on the sanitizers. What sanitizers are we talking about?

Also, I think you mean something different when you say N-API (or Node-API). Node-API is the contract between C++ and JavaScript, as well as the implementation of Node-API against a JavaScript engine.

If we want to test Node-API, it should be using something in the Node-API contract (i.e., something needs to cross the native to js or js to native bridge). There is actually a good set of tests for Node-API in Hermes when Node-API was introduced there. It would be great to reuse those tests here, but that should be a completely separate PR and is somewhat of a big lift.

Unless I'm missing something, these tests are strictly testing different JavaScript engines which is not necessarily bad in of itself, but they have nothing to do with Node-API. There are already conformance tests for ECMAScript that do this kind of test, such as https://compat-table.github.io/compat-table/es6.

Comment on lines 20 to 32
#ifdef __ANDROID__
// Global flag to track if StdoutLogger has been initialized
static bool s_stdoutLoggerInitialized = false;

void EnsureStdoutLoggerStarted()
{
if (!s_stdoutLoggerInitialized)
{
android::StdoutLogger::Start();
s_stdoutLoggerInitialized = true;
}
}
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this. It looks like this logic was moved from JNI.cpp to here. Why? This is a shared file. It is not intended to have platform specific logic.

Copy link
Author

Choose a reason for hiding this comment

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

sorry -- yes, you're correct. I was trying to figure out how to avoid the fdsan crash without adding code into AndroidExtensions repo, which doesn't seem to have its own tests, CI, or other feedback loops. I'll bite the bullet and prepare a PR for AndroidExtensions to fix the root problem where it acutally lives.

Comment on lines 38 to 51
static constexpr int compare(const char_type* s1, const char_type* s2, size_t n) noexcept {
for (size_t i = 0; i < n; ++i) {
if (s1[i] < s2[i]) return -1;
if (s1[i] > s2[i]) return 1;
}
return 0;
}

static constexpr const char_type* find(const char_type* s, size_t n, const char_type& c) noexcept {
for (size_t i = 0; i < n; ++i) {
if (s[i] == c) return s + i;
}
return nullptr;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This code is adding a template specialization, but it is not a specialization of std::char_traits. This is just a custom struct with some static functions. You can probably just not use templates at all since it's just a simple function call to get the length.

@@ -1 +1,2 @@
/Build
/Tests/UnitTests/dist/
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if this is from your PR, but we typically avoid generating files in the source tree. Generated files should go in the CMake binary folder which we typically put in Build.

});
});

describe("Unicode and String Encoding", function () {
Copy link
Contributor

@bghgary bghgary Oct 16, 2025

Choose a reason for hiding this comment

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

Maybe I need more context on the sanitizers. What sanitizers are we talking about?

Also, I think you mean something different when you say N-API (or Node-API). Node-API is the contract between C++ and JavaScript, as well as the implementation of Node-API against a JavaScript engine.

If we want to test Node-API, it should be using something in the Node-API contract (i.e., something needs to cross the native to js or js to native bridge). There is actually a good set of tests for Node-API in Hermes when Node-API was introduced there. It would be great to reuse those tests here, but that should be a completely separate PR and is somewhat of a big lift.

Unless I'm missing something, these tests are strictly testing different JavaScript engines which is not necessarily bad in of itself, but they have nothing to do with Node-API. There are already conformance tests for ECMAScript that do this kind of test, such as https://compat-table.github.io/compat-table/es6.

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.

3 participants