deps: update v8 to 4.4.63.8

[bnoordhuis]

The V8 upgrade is the first commit, then follow two commits to make io.js build and tests pass.

The first fix-up can be squashed into the upgrade commit if everyone thinks that's a good idea. I kept it separate for easier review. I had to make some questionable changes that I'll point out in the diff.

The second fix-up updates just the message tests. I had to tweak the expected output because v8::Message::GetEndColumn() returns a different value for throw statements now.

R=@trevnorris?

CI: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/59/

[bnoordhuis]

The centos5 and centos6 buildbots are unhappy because deps/v8/tools/js2c.py uses argparse now, which is a python2.7 module. @jbergstroem @rvagg Anything that can be done about that?

[trevnorris]

@bnoordhuis Don't want to be bugging with comments when you aren't done with your fix-ups. Ping me when you are.

Two things off the top of my head, can combine the two macros in buffer calls down to one. Did it that way for the conditional for old buffer mode. And looks like we can drop the IsSmiCheck and only use kMaxLength as now defined in the header.

Thanks for your work on this.

[bnoordhuis]

CL for the js2c.py change: https://codereview.chromium.org/1192973004/

[bnoordhuis]

The CI is pretty happy this time around. There are some test failures on win2008 but I'm positive they aren't related to this PR; it seems to be an issue with openssl-cli.exe. Jenkins falls over on the win2012 bot but fortunately after it runs the test suite first.

@trevnorris Ready for review.

[ChALkeR]

Is it just me or has the memory usage dropped by a few MiBs (around 3 after startup)?

[jbergstroem]

@bnoordhuis (re upgrading to python 2.7) haven't seen a way that's supported by upstream package managers.

Not super keen on floating a patch for backing out argparse, but it feels like the gain by switching is insignificant (at least for us). I'll see if there's an easy way to upgrade.

[ChALkeR]

There is a huge memory leak when using anything ArrayBuffer-related (typed arrays, buffers, etc).
It looks like those are not freed.
Testcase:

'use strict';

var count = 0, runsLimit = 500000, interval = 1000, parallel = 20;
var memoryLimit = 4 * 1024 * 1024 * 1024;

// Uncomment to get scavenges instead of mark-sweeps.
// var something = [];
// for (var j = 0; j < 2000; j++) something.push(Math.random());

function call() {
    var buffer = new ArrayBuffer(64 * 1024);
    // var bufferx = something.slice(); // Uncomment to get scavenges instead of mark-sweeps.
    count++;
    if (count % interval === 0) {
        var memory = process.memoryUsage();
        console.log(count + ": " + JSON.stringify(memory));
        if (memory.rss > memoryLimit) {
            console.log("Reached RSS memory limit of " + memoryLimit + " bytes!");
            process.exit();
        }
    }
    if (count > runsLimit) {
        console.log("Number of runs limit reached, test complete.");
        process.exit();
    }
    setImmediate(call);
}

for (var i = 0; i < parallel; i++) {
    call();
}

Warning: this has a 4 GiB memory limit (and it reaches that in 3 seconds on my system), if your amount of free memory is less than that, you would probably want to change the memoryLimit.

For Buffer and typed arrays it's the same, but the size of each object (the number in new ArrayBuffer(64 * 1024)) and/or limit might have to be altered a bit.

This is related to scavenges vs mark-sweeps (see #1671), if scavenges are triggered instead of mark-sweeps, there is no leak (for example in new ArrayBuffer(1 * 1024) here, or if you create an extra Array each time that pushes the scavenges threshold). The leak is there only when mark-sweeps are used instead of scavenges.
Update: I inserted the (commented out) code that makes v8 use scavenges into the testcase, uncomment that to get scavenges.

[bnoordhuis]

Playing around with @ChALkeR's test case, it looks like V8 doesn't call ArrayBufferAllocator::Free() for arraybuffers collected in the new space during the mark-sweep phase (whereas scavenges do.)

This one small patch that is probably gloriously wrong fixes it:

diff --git a/deps/v8/src/heap/mark-compact.cc b/deps/v8/src/heap/mark-compact.cc
index 5b29c21..e6cf151 100644
--- a/deps/v8/src/heap/mark-compact.cc
+++ b/deps/v8/src/heap/mark-compact.cc
@@ -4468,6 +4468,7 @@ void MarkCompactCollector::SweepSpaces() {

   EvacuateNewSpaceAndCandidates();

+  heap()->FreeDeadArrayBuffers(true);
   heap()->FreeDeadArrayBuffers(false);

   // ClearNonLiveReferences depends on precise sweeping of map space to

I'll file a V8 bug for it. I'll try to ping a V8 team member to check if fixing it up like this in the mean time is sensible.

(EDIT: Updated the patch to call heap()->FreeDeadArrayBuffers(true) after EvacuateNewSpaceAndCandidates().)

[bnoordhuis]

V8 bug: https://code.google.com/p/v8/issues/detail?id=4201

[trevnorris]

@bnoordhuis Until this is fixed do we want to hold off landing 4.4? Or maybe just float that patch in the meantime.

[bnoordhuis]

@trevnorris I would suggest we hold off the upgrade for now because the patch is almost certainly incorrect. Let's wait a day or two and see what the V8 people say.

[jeisinger]

Fix here: https://codereview.chromium.org/1199913002

[ChALkeR]

@jeisinger Seems to work.

One extra thing that I noticed:

• With 4.2 (iojs 2.3.0) the peak memory usage for the above testcase is ~213 MiB as-is and ~43 MiB with the array-slicing hack.
• With 4.4+fix (this PR+fix) the peak memory usage for the above testcase is ~405 MiB as-is and ~39 MiB with the array-slicing hack.

[jeisinger]

I guess that's mostly due to array buffer backing stores, right?

[ChALkeR]

@jbergstroem Correct. Almost all of that (99.7%) is inside v8::internal::Runtime_ArrayBufferInitialize(int, v8::internal::Object**, v8::internal::Isolate*).

[ChALkeR]

@jeisinger I just noted that the peak memory usage in a pure ArrayBuffer-based test increased two times when compared to 4.2. Apart from that, the problem with ArrayBuffers and garbage collection is described in #1671 (comment).

[trevnorris]

@bnoordhuis A fix seems to have landed in the V8 4.5 branch. So looks like we'll have to backport to 4.4?

[ChALkeR]

@trevnorris I doubt that v8 devs will leave 4.4 leaking. I would expect a backport on v8 side and a newer patch release of 4.4 branch.

[trevnorris]

@bnoordhuis Review complete.

[jeisinger]

I'll merge the fix once it had some time on canary

On Mon, Jun 22, 2015, 7:07 PM Trevor Norris notifications@github.com
wrote:

@bnoordhuis https://github.com/bnoordhuis Review complete.

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

[bnoordhuis]

Feedback incorporated, PTAL.

CI: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/75/

[ChALkeR]

Could you update this to 4.4.63.9? The memory issue seems fixed there.
Thanks, @jeisinger! ☺

[bnoordhuis]

@ChALkeR This PR contains the patch, @jeisinger did the cherry-pick right when I updated the PR.

[rvagg]

I believe I should be able to release an RC as soon as this lands

rubber-stamp LGTM

[trevnorris]

Whoops. Revised earlier but forgot to sign off. LGTM