Added SWIG #define to call v8 garbage collector

[Zion-ICS]

The destructors in our nodeJS-wrapped C++ classes weren’t being invoked— even on termination.
v8 doesn’t guarantee gc— even on scope closing or program termination.
We need to call gc manually.
Adding a #define lets us call 1 SWIG function that will be maintained— instead of having many #ifdefs.

[ojwb]

Is this meant to be called automatically from the generated code, or is it something that users are expected to use?

If the former, then you need to actually generate calls to it in the appropriate places.

If the latter, it needs adding to the documentation so that users can know how to use it.

[Zion-ICS]

We're currently using it as the latter. We're wrapping v8 GC in a global-scope function in our .i file. Then the JS can call that function whenever it wants to clear dead memory.

So yeah-- I should also contribute to the SWIG documentation on this! Though I'm not sure where to start documenting this. Do you want some comment about gc in 26.2.3 Known Issues? Do you want a new section "26.5.6 Handling Garbage Collection in SWIG v8"?

This PR could be the first step towards the former-- adding some type of gc to swig JS.

Possible future addition ideas I'm bouncing around:
Every class would get a cleanUp function in their top node.
SWIG could call gc automatically on exit. This would be fantastic.
SWIG could call gc automatically even on scope exit with a usr-added command-line parameter. I'm not sure we should always add the burden of calling gc every time a function exits. But this is just a thought in-progress, not a possible contribution yet....

[ojwb]

A new section sounds reasonable to me.

[Zion-ICS]

I have a question!
I figured out how to automatically include a v8 garbage collection function in every SWIG-generated nodeJS library.

It involves adding a new SWIG feature: allowing per-language auto-included code into .i files.

If a language has a file called "interface_lang_defaults.i" in its library folder, the code will be processed as if a user added it at the end of his/her .i file.

For example: I put a v8 gc function in interface_lang_defaults.i and then moved it to Lib/javascript/v8/. Now everybody wrapping C++ classes with nodeJS can call v8 garbage collection. Note that this should also let you destroy C++ primitives such as int16 or uint16 that you send to JS using %array_class.
I'll also include a parallel no-op version for jsc so running cleanUp won't cause any exceptions to be thrown.

Is "per-language auto-included code" a desired feature?
If so, I'll...

1. Modify the JavaScript examples to use this code
2. Document garbage collection and v8
3. Document "per-language auto-included code"
4. PR the new code and documentation

Thank you very much!!!!

[wsfulton]

Can you share what is going into the auto-included code? Without more details, the "per-language auto-included code" feature sounds like an extra complication to an already complex tool! Can you look at using the current directives - typemaps, %feature, %insert, %include %pythoncode (equivalent needed in JS) to add additional code? It sounds like this garbage collection should be generated by default to me though or maybe turned on via a %feature.

Is gc implemented for node and jsc? If so, inspiration for turning it on should be taken from these implementations.

@oliver---- is this missing garbage collection just a bug / oversight in the current implementation? @Richie765 any comments given your v8 usage?

[Zion-ICS]

The code I'd auto-include would be the following:

%{
// Because there's no guarantee that v8 will ever call garbage collection,
// we're adding a function that will allow a user to call it manually
void cleanup()
{
// Call the v8 garbage collector as long as there is memory to clean up
while (!SWIGV8_GC())
{;}
}
%}
void cleanup();

Having sat on this a bit, this has developed a bit.
I'd rename SWIGV8_GC to SWIGV8_GC_FINISHED, so the above function is while
(!SWIGV8_GC_FINISHED())
I also found a more exact v8 build where the GC call was changed
(0x032838), so I'd change that in javascriptrun.swg.
And.... v8::Isolate::GetCurrent()->IdleNotification() needs an int
parameter. I've been using 1000 b/c of this:
https://codereview.chromium.org/412163003;
Yeah, another change to the javascriptrun.swg I PR'd.

I included the above cleanup code in
Lib/javascript/v8/interface_lang_defaults.i
That got auto-added in Source/Modules/main.cxx on line 1082 in my local swig
(the last 2 lines I included here are current SWIG code. The first 3 are
new)
String *s = Swig_include("interface_lang_defaults.i");
if (s)
Printf(fs, "\n%%include "interface_lang_defaults.i"\n");
Seek(fs, 0, SEEK_SET);
cpps = Preprocessor_parse(fs);

Any language that has a file interface_lang_defaults.i would get the code
therein auto-included at the end.

If this makes more sense as a %feature, then I'm all for that!
I feel that:

1. v8 GC should be included in JS SWIG in a way that fits the SWIG system
2. v8 GC should be used in the SWIG JS class example

I haven't yet been able to get this to work as a %feature.... But I tried
working on it.

I'm still working on getting my head around GC and JavaScriptCore, and
finding a way to get a jsc environment (tried rhino-jsc... unsuccessfully
so far. This needs more work).
My apologies again....

Thank you very much!!!!
--Zion

On Tue, Feb 24, 2015 at 6:38 PM, William S Fulton notifications@github.com
wrote:

Can you share what is going into the auto-included code? Without more
details, the "per-language auto-included code" feature sounds like an extra
complication to an already complex tool! Can you look at using the current
directives - typemaps, %feature, %insert, %include %pythoncode (equivalent
needed in JS) to add additional code? It sounds like this garbage
collection should be generated by default to me though or maybe turned on
via a %feature.

Is gc implemented for node and jsc? If so, inspiration for turning it on
should be taken from these implementations.

@oliver---- https://github.com/oliver---- is this missing garbage
collection just a bug / oversight in the current implementation?
@Richie765 https://github.com/richie765 any comments given your v8
usage?

—
Reply to this email directly or view it on GitHub
#343 (comment).

[wsfulton]

Closing with the suggestion that anyone who wants to call the IdleNotification() method can simply add these calls into their own interface wrappers if they are required using the usual SWIG code generation mechanims. Also I didn't see anything in this issue indicating that SWIG should be providing GC calls in the generated code by default.