open-simulation-platform/cosim4j

Callback is broken in conjunction with cse::execution.start()

Closed this issue · 3 comments

Step event listener is broken in conjunction with cse::execution.start().

Possible related: https://stackoverflow.com/questions/19092602/context-switched-native-thread-cant-attach-to-jvm

It seems you cannot do callbacks to Java when using boost::fibers.

@kyllingstad @mrindar Any insights?

This is my current code:

cse::step_event_listener::step_event_listener(JNIEnv* env, jobject listener)
{
    env->GetJavaVM(&jvm_);
    jclass cls = env->GetObjectClass(listener);
    mid_ = env->GetMethodID(cls, "post", "()V");
    listener_ = env->NewGlobalRef(listener);
}

void cse::step_event_listener::step_complete(cse::step_number lastStep, cse::duration lastStepSize, cse::time_point currentTime)
{

    JNIEnv* env;
    bool attach = false;
    jint getEnvStat = jvm_->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6);
    if (getEnvStat == JNI_EDETACHED) {
        jvm_->AttachCurrentThread(reinterpret_cast<void**>(&env), nullptr);
        attach = true;
    }

    env->CallVoidMethod(listener_, mid_);

    if (attach) {
        jvm_->DetachCurrentThread();
    }
    
}

cse::step_event_listener::~step_event_listener()
{
    
    JNIEnv* env;
    bool attach = false;
    jint getEnvStat = jvm_->GetEnv((void**)&env, JNI_VERSION_1_6);
    if (getEnvStat == JNI_EDETACHED) {
        jvm_->AttachCurrentThread((void**)&env, nullptr);
        attach = true;
    } 

    env->DeleteGlobalRef(listener_);

    if (attach) {
        jvm_->DetachCurrentThread();
    }
    
}

This seems to work:

void cse::step_event_listener::step_complete(cse::step_number lastStep, cse::duration lastStepSize, cse::time_point currentTime)
{
    std::thread t(&cse::step_event_listener::callback, this);
    t.join();
}

void cse::step_event_listener::callback()
{
    JNIEnv* env;
    bool attach = false;
    int getEnvStat = jvm_->GetEnv(reinterpret_cast<void **>(&env), JNI_VERSION_1_6);
    if (getEnvStat == JNI_EDETACHED) {
        attach = true;
        jvm_->AttachCurrentThread(reinterpret_cast<void **>(&env), nullptr);
    }
    env->CallVoidMethod(listener_, mid_);
    if (attach) {
        jvm_->DetachCurrentThread();
    }
}

But, seems costly :/ Could probably reuse the thread.

Damn, results from a simple test shows that adding this callback adds a 170x performance overhead...

Edit: That was on an empty execution, with 1 FMU it was "only" 3.4x

When reusing the thread with the help of condition_variable we almost doubled the performance vs. allocating a new thread per callback. I'm, happy.