Repository navigation
Memory leak in simple test case involving WeakSet #6180
Description
Version: 4.2.1, 5.1.10 confirmed.
Platform: at least Windows 10 and Linux
`Following code reproduces error:
'use strict';
const s = new WeakSet();
let j = 0;
function f(){
for (let i = 0 ; i<100000; i++) {
s.add({});
}
console.log(`${j++} :${0|(process.memoryUsage().heapUsed/(1024*1024))}MB`);
return Promise.resolve(null).then(f);
}
f();It ends with following output
...
110 :853MB
<--- Last few GCs --->
10993 ms: Scavenge 825.2 (856.1) -> 823.2 (863.1) MB, 49.7 / 0 ms [allocation failure].
11126 ms: Scavenge 832.1 (863.1) -> 830.1 (870.1) MB, 43.4 / 0 ms [allocation failure].
11255 ms: Scavenge 839.0 (870.1) -> 837.0 (877.1) MB, 44.6 / 0 ms [allocation failure].
11390 ms: Scavenge 845.8 (877.1) -> 843.9 (884.1) MB, 47.5 / 0 ms [allocation failure].
11517 ms: Scavenge 852.7 (884.1) -> 850.8 (891.1) MB, 42.5 / 0 ms [allocation failure].
<--- JS stacktrace --->
==== JS stack trace =========================================
Security context: 0000029D869E3AD1 <JS Object>
1: add [native weak-collection.js:~92] [pc=0000036347B55F9B] (this=000003208D656691 <JS WeakSet>,k=00000167E7F946E9 <an Object with map 0000027C0AC172F1>)
2: f(aka f) [C:\Users\michal.wadas\Documents\test_gc.js:7] [pc=0000036347B536C1] (this=0000029D86904189 <undefined>)
3: arguments adaptor frame: 1->0
4: /* anonymous */(aka /* anonymous */) [native promise.js:221] [pc=000003...
Objects are properly garbage collected in Chrome 49 (process.memoryUsage().heapUsed is changed to performance.memory.usedJSHeapSize).
Source: StackOverflow question.
Activity
ChALkeR commentedon Apr 13, 2016
The problem is that somewhy only scavenges are triggered here, but mark-sweeps aren't.
And scavenges can't collect those elements (for some reason).
In most real-world apps this wouldn't be a problem, because there are many things that push mark-sweep runs.
This is most likely already fixed in v8, if Chrome 49 is fine.
We should check how master is doing, I will be able to check that a bit later.
vkurchatkin commentedon Apr 13, 2016
@ChALkeR is there write-up on which heuristics are used by v8 for different types of GC? It seems that in node v5 and earlier
WeakMapandWeakSetdon't apply any pressure on garbage collector at allChALkeR commentedon Apr 13, 2016
@vkurchatkin That, I don't know.
/cc @jeisinger
dnalborczyk commentedon Apr 13, 2016
No garbage collection in master with v8 4.9.385.35 and in vee-eight-5.0 with v8 5.0.71.25 on OSX El Capitan 10.11.5 Beta 1, XCode v7.3
Daniel
ChALkeR commentedon Apr 13, 2016
@dnalborczyk Same as in it gets garbage collected or not?
jeisinger commentedon Apr 13, 2016
Are you running with 64 bit or 32 bit?
dnalborczyk commentedon Apr 13, 2016
@ChALkeR Oh, sorry, same as in the original issue description. No garbage collection. I corrected the above.
dnalborczyk commentedon Apr 13, 2016
Btw, if it helps, Chrome 51.0.2700.0 dev is eventually smoking up as well - though the heap size seems to remain the same ( if the output happens to be correct ). Firefox 47, Edge 14 (37), and Safari 9.1.1 keep running.
jeisinger commentedon Apr 13, 2016
filed https://bugs.chromium.org/p/v8/issues/detail?id=4909 to track this.
jeisinger commentedon Apr 14, 2016
btw, can you share a non-artificial example? I.e. something where you manage to fill up a weak set in a meaningful way without generating enough garbage anyways to trigger a full gc?
benjamingr commentedon Apr 15, 2016
@ChALkeR dumb question - but can't/should't node trigger a full GC before throwing an out of memory error? I'd actually expect v8 to do that.
ChALkeR commentedon Apr 15, 2016
@benjamingr That's v8 job and somewhy it doesn't do it here. It's mentioned in the issue that @jeisinger filed to v8 issue tracker:
jeisinger commentedon Apr 15, 2016
landed a fix in v8. turns out there was no code to actually free up GC'd slots in a weak collection... gives you an idea how much this feature is used on the web.
anyways, should probably be backmerged to node's lts version of v8
31 remaining items
deps: backport e093a04, 09db540 from upstream V8
deps: cherry-pick b93c80a from v8 upstream
deps: backport a76d133 from v8 upstream
deps: backport e093a04, 09db540 from upstream V8
deps: cherry-pick b93c80a from v8 upstream
deps: backport a76d133 from v8 upstream
deps: backport e093a04, 09db540 from upstream V8
deps: cherry-pick b93c80a from v8 upstream
deps: backport a76d133 from v8 upstream
use mock weakset until nodejs/node#6180 is fixed
Merge pull request #2 from BitClock/feature
deps: cherry-pick 09db540,686558d from V8 upstream
deps: cherry-pick 09db540,686558d from V8 upstream
deps: cherry-pick 09db540,686558d from V8 upstream
deps: cherry-pick 09db540,686558d from V8 upstream