Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bufferTime with bufferCreationInterval leaks memory #4084

Closed
arielshaqed opened this issue Sep 3, 2018 · 5 comments
Closed

bufferTime with bufferCreationInterval leaks memory #4084

arielshaqed opened this issue Sep 3, 2018 · 5 comments

Comments

@arielshaqed
Copy link

Bug Report

Current Behavior
A clear and concise description of the behavior.

When I call bufferTime(10, 50) or bufferTime(50, 10), I get memory leaks (ts-node v4.1.0, node v9.11.1). Same code (modulo Node-specific heap size determination; I used Chrome debugger) does not appear to leak on Chrome Version 68.0.3440.106 (Official Build) (64-bit).

Reproduction

// Leak memory using Rx.bufferTime.
import * as Rx from 'rxjs';
import * as op from 'rxjs/operators';

declare var window: any;

function getHeap() {
  return process.memoryUsage().heapUsed;
}

const bufSize = 4096;

const startMsecs = Date.now();
const startHeap = getHeap();

let n = 0;

function repeat(s: string, n: number): string {
  return new Array<string>(s).fill(n).join('');
}

const s$ = Rx.interval(1).pipe(
  op.map((i) => repeat(i, 1000)),
  op.bufferTime(10, 10),
).forEach(() => {
  n++;
  if (n % 50 === 0) console.log(Date.now() - startMsecs, getHeap() - startHeap);
});

If I omit the second bufferTime argument, memory does not increase.

Expected behavior

Numbers should fluctuate around several hundred thousand, but remain bounded.

Actually, they increase without bound.

Environment

  • Runtime: ts-node v4.1.0, node v9.11.1
  • RxJS version: 6.3.1
  • (If bug is related) Loader, build configuration: [e.g webpack, angular-cli version, config]

Possible Solution

Specifying bufferCreationInterval takes the bufferTime code on a somewhat different path.

Additional context/Screenshots

When I run it on my MBP:

flutterby:LeakRx ariels$ ts-node --target es6 --module commonjs leaker.ts
534 -13957112
1060 -10093664
1587 -10248296
2101 -7352416
2623 -7611384
3143 180632
3657 3135656
4174 9960824
4692 4826968
5212 16375664
5744 18128216
6264 20596432
6784 25276480
7296 31470312
7806 39427360
8323 49389320
8842 50295592
9356 63838840
9884 68792288
10397 73684480
10922 80035688
11441 91109032
11963 100696920
12482 104292168
13003 118677824
13517 123799992
14036 138397864
14556 150215408
15087 158630392
15601 171176016
16139 182235160
16666 185696504
17192 205358072
17705 208343936
18233 220496096
@arielshaqed
Copy link
Author

Updated gist, please use this as the leaker:

// Leak memory using Rx.bufferTime.

import * as Rx from 'rxjs';
import * as op from 'rxjs/operators';

declare var window: any;

function getHeap() {
  return process.memoryUsage().heapUsed;
}

const bufSize = 4096;

const startMsecs = Date.now();
const startHeap = getHeap();

let n = 0;

function repeat(s: string, n: number): string {
  let ret: string = '';
  for (let i = 0; i < n; i++) ret += s;
  return ret;
}

const s$ = Rx.interval(1).pipe(
  op.map((i) => repeat(i.toString(), 1000)),
  op.bufferTime(10, 10),
).forEach(() => {
  n++;
  if (n % 50 === 0) console.log(Date.now() - startMsecs, getHeap() - startHeap);
});

@kwonoj
Copy link
Member

kwonoj commented Sep 6, 2018

Just double confirming, am I reading correct same code snippet doesn't leak on browser but only on node.js?

@arielshaqed
Copy link
Author

Just double confirming, am I reading correct same code snippet doesn't leak on browser but only on node.js?

I appreciate your skepticism, but yes. I can run the same code (in Chrome) and it does not leak. Possibly related: nodejs/node#7346, which appears to regress:

flutterby:xenu ariels$ node --version && node -e 'start = Date.now(); i = 0; cancel = setInterval(() => { if (++i % 50 === 0) console.log(Date.now() - (start + (i + 1) * 20)); }, 20)' | head
v10.9.0
121
245
351
488
629
749
877
1006
1146
1295

@kwonoj
Copy link
Member

kwonoj commented Sep 6, 2018

Sounds very promising if it's related to upstream nodejs side (as you mentioned), might need few verification over different node / browser targets.

@kwonoj
Copy link
Member

kwonoj commented Nov 19, 2018

Peeking this again, I came to feel this isn't bug actually : node, browser shows same behavior, and it's due to keep allocating resources - bufferTime creates new array each time given time window passes, and it just keep going on until source completes. In here, source never completes so there's no chance allocated resources being GC'ed. I still may wrong, if you can observe env specific (I could observe this in chrome as well), please feel free to open new issue with repro codes. Sorry for taking time to get back.

@kwonoj kwonoj closed this as completed Nov 19, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Dec 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants