Add GC callbacks, and a mechanism to fix the threshold

[KaruroChori]

For context: saghul/txiki.js#516 & saghul/txiki.js#520

I implemented a draft of some new core features in quickjs to expose more of an interface for the GC to its runtime. This way it is easier for the application embedding quickjs to control when GC events can run and some related side effects.
This additional control is helpful in the context of systems with some real time requirements.

The extended interface basically consists of:

• A getter for the cg threshold to complete its interface. added in a separate PR
• A flag to fix the threshold, disabling the autoscaling feature which is currently forced.
• Callbacks before and after the CG event (not to be run for manually triggered ones).
  The before callback has a boolean return value to further specify if the event should be allowed to run or not.

Tests are not provided yet since they might be a bit tricky to write, but if there is consensus on this or some alternative variant I will add some.

[saghul]

In general, I'm supportive of the approach.

I didn't comment on the style issues since they are not relevant just yet ;-)

@chqrlie thoughts?

[KaruroChori]

Sadly style is about what you can expect by someone deeply unfamiliar with the codebase and who does not generally work with C :D.
In my defense I was unable to find specific guidelines in the repo, and the naming conventions are not always consistent across the codebase, so every time I had to name a variable it was a bit of a pickle :D
If you have feedback about style (or if there is a clang formatter I missed) it is welcome as well.

[saghul]

Ignore the style for now. It should just look like the surrounding code.

We have no linter alas.

[chqrlie]

In general, I'm supportive of the approach.

I didn't comment on the style issues since they are not relevant just yet ;-)

@chqrlie thoughts?

Hello @KaruroChori, sorry for the lag, I am focused on some tricky issues ATM, I won't have much time to investigate your approach until next week.

[KaruroChori]

No problem!

[KaruroChori]

I fixed this PR to not duplicate #424 & reworked some of the naming.

[ammarahm-ed]

It would be great if the GC callbacks are called with runtime pointer so one can trigger any necessary cleanups.

JS_SetGCAfterCallback(runtime->runtime, [](JSRuntime* rt) {
            auto env = (napi_env) JS_GetRuntimeOpaque(rt);
            if (env->gcAfter != nullptr) {
                env->gcAfter->finalizeCallback(env, env->gcAfter->data, env->gcAfter->finalizeHint);
            }
    });

Which I use like this eventually:

napi_set_gc_finish_callback(env, [](napi_env env, void *data, void* hint) {
        auto* objectManager = reinterpret_cast<ObjectManager*>(data);
        if (!objectManager->m_weakObjectIds.empty()) {
            JEnv jEnv;
            std::for_each(objectManager->m_weakObjectIds.begin(),
                          objectManager->m_weakObjectIds.end(), [&](int id) {
                    auto itFound = objectManager->m_markedAsWeakIds.find(id);
                    if (itFound == objectManager->m_markedAsWeakIds.end()) {
                        objectManager->m_buff.Write(id);
                        objectManager->m_markedAsWeakIds.emplace(id);
                    }
            });
            if (objectManager->m_buff.Size()) {
                int length = objectManager->m_buff.Size();
                jboolean keepAsWeak = JNI_TRUE;
                jEnv.CallVoidMethod(objectManager->m_javaRuntimeObject, objectManager->MAKE_INSTANCE_WEAK_BATCH_METHOD_ID,
                                    (jobject) objectManager->m_buff, length, keepAsWeak);
                objectManager->m_buff.Reset();
            }
        }
        DEBUG_WRITE("%s", "GC completed");
        }, this);

[bnoordhuis]

Pre/post GC hooks seem unobjectionable to me. I'd maybe change it to a single hook that's called with enum JSGCEvent { JS_GC_PRE, JS_GC_POST } or something like that.

I'm not entirely sold on this fix_malloc_gc_threshold thing. That should probably be a separate pull request.

[saghul]

Agreed FWIW.

[KaruroChori]

I'm not entirely sold on this fix_malloc_gc_threshold thing. That should probably be a separate pull request.

Ok, once I get back from my vacations I will split this PR :).

[KaruroChori]

I'd maybe change it to a single hook that's called with enum JSGCEvent { JS_GC_PRE, JS_GC_POST } or something like that.

Did you mean this just for registration of the different callbacks, or are you expecting one single callback to handle multiple events at once switching between cases?

Because I think this style of dispatching when registering hooks/events/callbacks is perfectly valid.
But merging the functionality of the two logically split events into a single function does not sit right with me to be honest :D.

[saghul]

I think he meant both.

[bnoordhuis]

one single callback to handle multiple events at once switching between cases?

This ⬆️

Something along the lines of:

typedef void (*JSGCEventListener)(enum JSGCEvent event, void *arg);
void JS_AddGCEventListener(JSRuntime *rt, JSGCEventListener *l, void *arg);

[saghul]

I like it. It's also flexible in case we need new events later down the line.