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

Lifecycle supports 32-bit architectures #977

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

staltz
Copy link
Contributor

@staltz staltz commented Mar 21, 2023

Context

I'm using Neon to write a Node.js native module that will be run in nodejs-mobile, on mobile architectures such as armeabi-v7a (32 bit), aarch64 and so forth.

Problem

With neon 0.10.1:

Cross-compilation for Android armv7 (32-bit) fails because AtomicU64 is not supported for that architecture. This is assuming that neon's napi-6 feature is enabled, because of

#[cfg(feature = "napi-6")]
pub mod lifecycle;

and

atomic::{AtomicU64, Ordering},

If I use napi-5 feature, compilation succeeds.

Solution

This PR enables napi-6 to support 32-bit architectures, by using AtomicU32 instead. Arguably, we lose a lot more identifiers for InstanceId, but U32 gives us 4 billion identifiers, and maybe this is sufficient?

A follow-up to this PR would switch the AtomicU* type depending on the architecture.

@kjvalencik
Copy link
Member

This seems reasonable. Since each of these represent a type of data that lives as long as the VM, it doesn't make sense to have this many.

@kjvalencik kjvalencik merged commit 3e83d73 into neon-bindings:main Mar 23, 2023
@staltz staltz deleted the support-32-archs branch March 23, 2023 17:29
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.

None yet

2 participants