search

neon-bindings / neon

Rust bindings for writing safe and fast native Node.js modules.
https://www.neon-bindings.com/
Apache License 2.0
8k stars 283 forks source link

Lifecycle supports 32-bit architectures #977

Closed staltz closed 1 year ago

staltz commented 1 year ago

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

https://github.com/neon-bindings/neon/blob/fe4642e7c126b4bffb16be5c655e57bc0ef3432d/crates/neon/src/sys/mod.rs#L34-L35

and

https://github.com/neon-bindings/neon/blob/fe4642e7c126b4bffb16be5c655e57bc0ef3432d/crates/neon/src/lifecycle.rs#L15

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 commented 1 year ago

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.