Possible memory leak?

[upsala]

This results in a possible memory leak:

$v8 = new V8Js();
for($i=0; $i<10000000; ++$i) {
  $v8->executeString('1;', 'script.js', V8Js::FLAG_FORCE_ARRAY, 1000, 10000000);
}

PHP Fatal error: Uncaught exception 'V8JsMemoryLimitException' with message 'Script memory limit of 10000000 bytes exceeded'

Tested with PHP5 and the master-branch

[stesie]

I doubt this is a memory leak, the problem with the memory limit is, that it simply tests the V8 heap size and kills V8 if it gets bigger than the limit. Yet it does not influence V8's internal garbage collection (or whatever else)

That said and given the pretty low limit of just 10 MB, a machine-dependent "new space size" of V8 of probably 8 MB and a target "old space size" of your V8 of 1.5 GB ... V8 just doesn't trigger the garbage collector even once ... and then goes well beyond your 10 MB.

Simply ini_set('v8js.flags', '--trace_gc'); before the V8 instanciation and have a look whether it triggers collection (and when - if you run ith without the limit).

I've never done it, but if you really want to limit V8's memory usage so hard, then try something like ini_set('v8js.flags', '--max_old_space_size=20') to limit V8's internal sense of available memory

[upsala]

The memory limit of 10MB was only an example. The same happens with 500MB.

If I use php ini_set('v8js.flags', '--trace_gc'); I get many of this messages: [1104:0x255a1c0] 47101 ms: Scavenge 475.7 (519.0) -> 474.7 (519.0) MB, 0.3 / 0 ms (+ 0.0 ms in 16 steps since last GC) [allocationfailure].`

If I use php ini_set('v8js.flags', '--max_old_space_size=20'), everything works fine. Wouldn't it be useful if executeString would set this parameter automatically?

[stesie]

There are multiple garbage collection runs in V8, especially "Scavenge collection" and "Mark-sweep collection", where the former targets the "new space" and the latter the "old space". Compiled code directly ends up in the old space, hence the scavenge collections don't do much.

If you set both --trace_gc --max_old_space_size=20, then the output looks like this:

stesie@hahnschaaf:~/Projekte/v8js$ php  -n -d extension_dir=./modules -d extension=v8js.so foo.php 
[8836:0x1de5e30]      244 ms: Scavenge 4.2 (40.0) -> 3.2 (40.0) MB, 0.1 / 0 ms [allocation failure].
[8836:0x1de5e30]      488 ms: Scavenge 6.4 (42.0) -> 5.4 (42.0) MB, 0.1 / 0 ms [allocation failure].
[8836:0x1de5e30]      738 ms: Scavenge 8.6 (44.0) -> 7.6 (44.0) MB, 0.1 / 0 ms [allocation failure].
[8836:0x1de5e30]      991 ms: Scavenge 10.8 (46.0) -> 9.8 (46.0) MB, 0.1 / 0 ms (+ 0.7 ms in 12 steps since last GC) [allocation failure].
[8836:0x1de5e30]     1242 ms: Scavenge 13.0 (49.0) -> 12.1 (49.0) MB, 0.1 / 0 ms (+ 0.0 ms in 16 steps since last GC) [allocation failure].
[8836:0x1de5e30]     1492 ms: Scavenge 15.3 (51.0) -> 14.3 (51.0) MB, 0.1 / 0 ms (+ 0.0 ms in 16 steps since last GC) [allocation failure].
[8836:0x1de5e30]     1743 ms: Scavenge 17.5 (53.0) -> 16.5 (53.0) MB, 0.1 / 0 ms (+ 0.0 ms in 15 steps since last GC) [allocation failure].
[8836:0x1de5e30]     1908 ms: Mark-sweep 18.5 (54.0) -> 1.0 (39.0) MB, 0.4 / 0 ms (+ 0.8 ms in 69 steps since start of marking, biggest step 0.1 ms) [allocation failure] [GC in old space requested].
[8836:0x1de5e30]     2155 ms: Scavenge 4.2 (40.0) -> 3.2 (40.0) MB, 0.1 / 0 ms [allocation failure].
[8836:0x1de5e30]     2406 ms: Scavenge 6.4 (42.0) -> 5.5 (42.0) MB, 0.1 / 0 ms [allocation failure].
[8836:0x1de5e30]     2652 ms: Scavenge 8.7 (44.0) -> 7.7 (44.0) MB, 0.1 / 0 ms [allocation failure].
[8836:0x1de5e30]     2898 ms: Scavenge 10.9 (46.0) -> 9.9 (46.0) MB, 0.1 / 0 ms [allocation failure].
[8836:0x1de5e30]     3150 ms: Scavenge 13.1 (49.0) -> 12.1 (49.0) MB, 0.1 / 0 ms (+ 0.9 ms in 16 steps since last GC) [allocation failure].
[8836:0x1de5e30]     3399 ms: Scavenge 15.3 (51.0) -> 14.3 (51.0) MB, 0.1 / 0 ms (+ 0.0 ms in 16 steps since last GC) [allocation failure].
[8836:0x1de5e30]     3649 ms: Scavenge 17.5 (53.0) -> 16.5 (53.0) MB, 0.1 / 0 ms (+ 0.0 ms in 16 steps since last GC) [allocation failure].
[8836:0x1de5e30]     3804 ms: Mark-sweep 18.5 (54.0) -> 1.0 (39.0) MB, 0.4 / 0 ms (+ 0.9 ms in 57 steps since start of marking, biggest step 0.1 ms) [allocation failure] [GC in old space requested].
[8836:0x1de5e30]     4053 ms: Scavenge 4.2 (40.0) -> 3.2 (40.0) MB, 0.1 / 0 ms [allocation failure].
[8836:0x1de5e30]     4304 ms: Scavenge 6.4 (42.0) -> 5.5 (42.0) MB, 0.0 / 0 ms [allocation failure].

... notice the "mark-sweep" lines, where allocated memory drops from 18.5 to 1.0 megs.

Wouldn't it be useful if executeString would set this parameter automatically?

The problem is that the imposed memory limit is dynamic (you can apply different limits on different V8Js objects or even executeString calls) whereas the v8js.flags can only be set before the first V8Js object instaciation (and then all those instaces share the same limit)

But still I agree that the current behaviour is problematic (or even not tolerable). I'll have a look whether I can force a collection from V8Js once the memory limit is hit, and then only abort if the memory limit is still violated

[stesie]

Bugfix merged, see #222