Port java-spaghetti-gen to cafebabe; cache method/field IDs; fix jclass memory leaks

[wuwbobo2021]

Commit 1: Port java-spaghetti-gen to cafebabe

Introduced cafebabe as an alternative to the unmaintained jreflection crate. However, there are some workarounds for staktrace/cafebabe#52.

Difference of execution speed (release build) cannot be realized.

The generated bindings.rs for the whole android.jar keeps the same, except these differences:

• java.flags comment: SYNCRONIZED -> SYNCHRONIZED
• documentation URL comment: bool%5B%5D -> boolean%5B%5D (bool was a bug)
• const values for NaN and INFINITY: __jni_bindgen namespace is removed

Commit 2: Fix java-spaghetti-gen reference URL generation

Documentation URL comment generation: changed %5B%5D to [], because the android documentation website uses [] directly in URLs for now, and links like https://developer.android.com/reference/android/icu/util/Currency.html#getName(java.util.Locale,%20int,%20boolean%5B%5D) cannot jump to the correct item.

Commit 3: Cache method/field ID; fix jclass memory leaks

This is a significant performance improvement. Caching method/field IDs may encounter validity issues: https://mostlynerdless.de/blog/2023/07/17/jmethodids-in-profiling-a-tale-of-nightmares/; I try to avoid the problem by holding a global reference of the class object for each class being used.

Test case:

include = [
    "java/lang/Object",
    "java/lang/Throwable",
    "java/lang/StackTraceElement",
    "java/lang/String",
    "java/lang/Integer"
]

[[documentation.pattern]]
class_url_pattern           = "https://developer.android.com/reference/{CLASS}.html"
method_url_pattern          = "https://developer.android.com/reference/{CLASS}.html#{METHOD}({ARGUMENTS})"
constructor_url_pattern     = "https://developer.android.com/reference/{CLASS}.html#{CLASS.INNER}({ARGUMENTS})"
field_url_pattern           = "https://developer.android.com/reference/{CLASS}.html#{FIELD}"
argument_seperator          = ",%20"

[logging]
verbose = true

[input]
files = [
    # To be modified
    "E:\\android\\platforms\\android-30\\android.jar",
]

[output]
path = "bindings.rs"

[codegen]
method_naming_style             = "java"
method_naming_style_collision   = "java_short_signature"
keep_rejected_emits             = false

[codegen.field_naming_style]
const_finals    = true
rustify_names   = false
getter_pattern  = "{NAME}"
setter_pattern  = "set_{NAME}"

#![feature(arbitrary_self_types)]
mod bindings;
use bindings::java;

pub fn get_vm() -> java_spaghetti::VM {
    let vm = ndk_context::android_context().vm();
    if vm.is_null() {
        panic!("ndk-context is unconfigured: null JVM pointer, check the glue crate.");
    }
    unsafe { java_spaghetti::VM::from_raw(vm.cast()) }
}

#[no_mangle]
fn android_main(_: android_activity::AndroidApp) ->  Result<(), Box<dyn std::error::Error>> {
    android_logger::init_once(
        android_logger::Config::default()
            .with_max_level(log::LevelFilter::Info)
            .with_tag("java_spaghetti_test".as_bytes()),
    );

    let vm = get_vm();
    let t_exec = vm.with_env(|env| {
        measure_exec_seconds(|| {
            let integer = java::lang::Integer::valueOf_int(env, 31)?.unwrap();
            for _ in 0..100*1000*1000 {
                let n = integer.intValue().unwrap();
                assert_eq!(n, 31);
            }
            Ok::<(), java_spaghetti::Local<'_, java::lang::Throwable>>(())
        })
    });

    log::info!("Time elapsed: {t_exec} s");
    Ok(())
}

fn measure_exec_seconds<T, F: FnOnce() -> T>(fn_exec: F) -> f64 {
    use std::time::Instant;
    let t_start = Instant::now();
    let _ = fn_exec();
    Instant::now().duration_since(t_start).as_secs_f64()
}

# For cargo-apk

[package]
name = "java-spaghetti-test"
version = "0.1.0"
edition = "2021"
publish = false

[dependencies]
log = "0.4"
java-spaghetti = { path = "../java-spaghetti" }
android-activity = { version = "0.6", features = ["native-activity"] }
android_logger = "0.14"
ndk-context = "0.1.1"

[build-dependencies]
java-locator = "0.1.8"

[lib]
name = "java_spaghetti_test"
crate-type = ["cdylib"]
path = "lib.rs"

[package.metadata.android]
package = "com.example.java_spaghetti_test"
build_targets = [ "aarch64-linux-android" ]

[package.metadata.android.sdk]
min_sdk_version = 16
target_sdk_version = 30

Before the change:

02-10 06:36:46.081  3016  3016 F DEBUG   : signal 6 (SIGABRT), code -1 (SI_QUEUE), fault addr --------
02-10 06:36:46.081  3016  3016 F DEBUG   : Abort message: 'JNI ERROR (app bug): local reference table overflow (max=16777216)
02-10 06:36:46.081  3016  3016 F DEBUG   : local reference table dump:
02-10 06:36:46.081  3016  3016 F DEBUG   :   Last 10 entries (of 16777216):
02-10 06:36:46.081  3016  3016 F DEBUG   :     16777215: 0x6f4aa250 java.lang.Class<java.lang.Integer>
02-10 06:36:46.081  3016  3016 F DEBUG   :     16777214: 0x6f4aa250 java.lang.Class<java.lang.Integer>
02-10 06:36:46.081  3016  3016 F DEBUG   :     16777213: 0x6f4aa250 java.lang.Class<java.lang.Integer>
...

After the change:

java_spaghetti_test: java_spaghetti_test: Time elapsed: 17.10836732 s

Note: it would be local reference table overflow (max=512) on Android 6.0.

Another change is possible: reduce DeleteLocalRef calls by pushing/popping local frames in VM::get_env

This seems like a minor optimization (jni-rs/jni-rs#560 (comment)). Being capable of making this change, I will not achieve it unless you really want me to do so. The disadvantage of this optimization is about the complexity of maintaining a local frame tracker (stack) and an overall local reference counter in the thread local storage:

• VM::get_env needs to push a "frame" into the tracker stack before executing the closure, and pop a "frame" from that stack after executing the closure, reducing the overall local ref counter by the usage counter for the local frame being popped.
• Env wrapper created by VM::get_env should keep the corresponding local frame depth (it cannot be transparent) internally, to prevent the use of outer env in an inner frame. In callback functions, the env pointer passed to the function may be wrapped by an unsafe function maintaining the frame tracker just like what VM::get_env does.
• Each item of the local frame tracker needs to store a current local frame capacity worst-case estimation, a local frame usage counter, and a vector (trash can) of jobjects coming from dropped Locals.
• Every local reference needs to have a corresponding Local wrapper; every operation that creates a Local wrapper (probably including from_raw) needs to increase the frame usage counter of the stack top (as well as the overall usage counter), call EnsureLocalCapacity to increase capacity estimation if needed; if the overall counter reaches certain limit, it may even pop some jobjects from the "trash can" and call DeleteLocalRef for them, then decrease both counters.
• Local needs to store its corresponding frame depth too, because a Local created in an outer frame may be dropped in an inner frame: push it into the trash can of the correct level in the frame tracker.

[wuwbobo2021]

The automatic check has failed because of it doesn't use a nightly compiler: error[E0554]: #![feature] may not be used on the stable release channel.

[wuwbobo2021]

I have fixed the CI script and the check has passed.

Note: the JNI global reference capacity on Android is usually 51200, and I'm merely keeping global references for used classes (there are no more than 20,000 classes in android.jar) to make sure that these classes are not to be unloaded, so that cached method/field IDs will keep valid. The invocation speed will be 5~9x faster than not caching them.

[wuwbobo2021]

I would like to know if I am making any unacceptable change. (just ignore the post for this moment if you're busy)

Fixed another bug of java char constant emission

Previous errors look like this:

error[E0423]: expected function, found builtin type `u16`
     --> java-spaghetti-test\bindings.rs:42604:28
      |
42604 | pub const MENU_KEY : u16 = u16(115);
      |                            ^^^ not a function

With this fix, bindings.rs generated with the configuration below should pass cargo check after rust-lang/rust#137128 is resolved.

include = [
    "*",
]

[[documentation.pattern]]
class_url_pattern           = "https://developer.android.com/reference/{CLASS}.html"
method_url_pattern          = "https://developer.android.com/reference/{CLASS}.html#{METHOD}({ARGUMENTS})"
constructor_url_pattern     = "https://developer.android.com/reference/{CLASS}.html#{CLASS.INNER}({ARGUMENTS})"
field_url_pattern           = "https://developer.android.com/reference/{CLASS}.html#{FIELD}"
argument_seperator          = ",%20"

[logging]
verbose = true

[input]
files = [
    # To be modified
    "E:\\android\\platforms\\android-30\\android.jar",
]

[output]
path = "bindings.rs"

[codegen]
method_naming_style             = "java"
method_naming_style_collision   = "java_long_signature"
keep_rejected_emits             = false

[codegen.field_naming_style]
const_finals    = true
rustify_names   = false
getter_pattern  = "{NAME}"
setter_pattern  = "set_{NAME}"

This custom class has been tested

package com.example.test;

public class Test {
    public static int[] static_int_array = {4, 5, 6};
    public static final int[] INT_ARRAY_CONSTANT = {11, 12, 13};
    public int int_field = 11;

    public int method_test() {
        return 3;
    }

    public static int static_method_test() {
        return 5;
    }
}

    vm.with_env(|env| {
        use bindings::com::example::test::Test;
        use java_spaghetti::PrimitiveArray;

        for i in 0..1000 {
            assert_eq!(Test::static_method_test(env).unwrap(), 5);

            let test_obj = Test::new(env).unwrap();
            assert_eq!(test_obj.method_test().unwrap(), 3);

            test_obj.set_int_field(i);
            assert_eq!(test_obj.int_field(), i);
            test_obj.set_int_field(7);
            assert_eq!(test_obj.int_field(), 7);

            let mut arr = [0; 3];
            Test::INT_ARRAY_CONSTANT(env).unwrap().get_region(0, &mut arr);
            assert_eq!(arr, [11, 12, 13]);
            
            Test::static_int_array(env).unwrap().set_region(0, &[1, 3, i]);
            Test::static_int_array(env).unwrap().get_region(0, &mut arr);
            assert_eq!(arr, [1, 3, i]);
        }

        log::info!("Success...");
        Ok::<(), ()>(())
    })
    .unwrap();

[Dirbaio]

why cast to usize and back?

The semantics of usize<->ptr casts are a bit tricky, .addr() doesn't expose the provenance, so it can't be used to reconstruct the pointer with as casts. For this to be sound you'd have to use as casts in both directions, or use .expose_provenance(), .with_exposed_provenance_mut() (which are equivalent to as)

If the reason for usize is because the pointer isn't Send/Sync it'd be cleaner to make a newtype and unsafe impl Send/Sync on it. This way the pointer can stay as a pointer all the time.

[wuwbobo2021]

If the reason for usize is because the pointer isn't Send/Sync it'd be cleaner to make a newtype and unsafe impl Send/Sync on it. This way the pointer can stay as a pointer all the time.

I have modified the code according to your suggestion. I tested the newly generated bindings and it works. Thank you.

[wuwbobo2021]

I did another test to make sure these cached IDs can be used across threads.

The problem is about the JNI specification itself not describing clearly that these IDs are thread-safe. But some other articles (as well as jni-rs documentation) claim that it's no problem (with existing JVM implementations).

The Android JNI tips says: "Because there is a limit of one JavaVM per process, it's reasonable to store this data in a static local structure." (https://developer.android.com/training/articles/perf-jni#jclass,-jmethodid,-and-jfieldid) The word "local" used here is probably about the scope of the variable in C programs, but not about thread-local storage. Do you think so?

[Dirbaio]

yeah, it looks good to me. Everything seems to imply theyŕe global pointers.

Thanks for the PR!