oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
73.13k stars 2.68k forks source link

Memory usage keeps increasing while heapSize remains unchanged #3065

Closed dimka3553 closed 11 months ago

dimka3553 commented 1 year ago

What version of Bun is running?

0.6.3

What platform is your computer?

Darwin 22.4.0 arm64 arm

What steps can reproduce the bug?

import Bao from "baojs";
import { heapStats, memoryUsage } from "bun:jsc";
import { createPublicClient, http } from "viem";
import { bscTestnet } from "viem/chains";
const app = new Bao();

const client = createPublicClient({
  chain: bscTestnet,
  transport: http(),
});

app.get("/", (ctx) => {
  const heap = heapStats();
  const mem = memoryUsage();
  const res = {
    mem,
    heap,
  };
  return ctx.sendJson(res);
});

app.listen({ port: 8012 });

let memoryUsages: number[] = [0];
let blockTimes: number[] = [0];

client.watchBlocks({
  onBlock: (block) => {
    update(parseInt(block.timestamp.toString()));
    console.log(memoryUsages.join(","));
    console.log(blockTimes.join(","));
    console.log("----");
    Bun.DO_NOT_USE_OR_YOU_WILL_BE_FIRED_mimalloc_dump();
    Bun.gc(true);
  },
});

let appendDelay = 1;
let chartLen = 10;
let delaysSoFar = 0;

const update = async (blockTime: number) => {
  const mem = memoryUsage().current;

  if (memoryUsages.length < chartLen) {
    if (delaysSoFar == appendDelay) {
      memoryUsages.push(mem);
      blockTimes.push(blockTime);
      delaysSoFar = 0;
    } else {
      memoryUsages[memoryUsages.length - 1] = mem;
      blockTimes[blockTimes.length - 1] = blockTime;
    }
    delaysSoFar += 1;
  }

  if (memoryUsages.length == chartLen) {
    memoryUsages = memoryUsages.filter((element, i) => i % 2 == 0);
    blockTimes = blockTimes.filter((element, i) => i % 2 == 0);
    appendDelay *= 2;
    memoryUsages.push(mem);
  }
};

Also need to run:

bun i viem baojs

What is the expected behavior?

Memory usage staying constant

What do you see instead?

The memoryUsage().current keeps gradually increasing and according to Bun.DO_NOT_USE_OR_YOU_WILL_BE_FIRED_mimalloc_dump(); The amount of commits keeps going up.

Screenshot 2023-05-25 at 13 08 29

Additional information

No response

dimka3553 commented 1 year ago

here is an issue in viem that is also related to this:

-> https://github.com/wagmi-dev/viem/issues/583

dimka3553 commented 1 year ago
setInterval(async () => {
  const b = await fetch("https://data-seed-prebsc-1-s1.binance.org:8545", {
    method: "POST",
    body: JSON.stringify({
      jsonrpc: "2.0",
      id: 1,
      method: "eth_getBlockByNumber",
      params: ["latest", false],
    }),
  }).then((r) => r.json());

  console.log(b.result.number);
  Bun.DO_NOT_USE_OR_YOU_WILL_BE_FIRED_mimalloc_dump();
  Bun.gc(true);
}, 1000);

Apparently just doing this causes the number of commits to go up every time a new block appears

Jarred-Sumner commented 1 year ago

@dimka3553 can you try setting this envinroment variable? BUN_JSC_forceRAMSize="1073741"

dimka3553 commented 1 year ago

@dimka3553 can you try setting this envinroment variable? BUN_JSC_forceRAMSize="1073741"

will do now

dimka3553 commented 1 year ago
Screenshot 2023-06-18 at 22 07 50

@Jarred-Sumner Ram usage and commits still going up over time, it will take a few days to reach a gigabyte though at which point the container will probably restart, maybe it will reach 1gb and stay there, will need to keep it there for some time to see. This is the latest version of Bun btw.

Jarred-Sumner commented 1 year ago

Could try setting it to 512 MB or 256 MB. 256 is probably a good bet

dimka3553 commented 1 year ago
Screenshot 2023-06-20 at 00 05 43

@Jarred-Sumner I've set it to 200MB and now it's using 370MB, still experiencing the memory leak. It's probably an issue with the fetch as I have tested the same program with the API just recursively calling itself with very aggressive GC (every iteration), while significantly slower than the example provided, the memory usage still increases over time.

philosofonusus commented 1 year ago

I have the same issue with fetch requests permanently taking memory bun v0.7.0, any info on this?

Jarred-Sumner commented 1 year ago

@dimka3553 @philosofonusus does passing --smol to Bun address the issue? This configures the heap to free memory more frequently.

dimka3553 commented 1 year ago

@dimka3553 @philosofonusus does passing --smol to Bun address the issue? This configures the heap to free memory more frequently.

The thing is the heapSize does not increase, in the original code it was 3.6MB and stayed there for the duration of the test, the memoryUsage goes up tho, but I will test it now anyways to see how it affects the memory usage.

dimka3553 commented 1 year ago

@Jarred-Sumner Just tested, on 0.7.1, with the --smol the memory usage is still going up every fetch request.

philosofonusus commented 1 year ago

@Jarred-Sumner I have also made a test on the same version as @dimka3553. The same for me, the memory goes up with every request. In my case, I'm just sending a lot of parallel requests with proxy and request-retry wrapped fetch to get some data from API

BTW I'm running my docker container in ubuntu

philosofonusus commented 1 year ago
Снимок экрана 2023-07-29 в 13 47 21

I think it pretty much sums it up, the memory has increased permanently, this is my minimal reproduction.

import { memoryUsage } from "bun:jsc";

const mem = memoryUsage();
console.log("init", mem.current);

for (const i of [1, 2]) {
  await Promise.allSettled(
    Array.from({ length: 50 }).map(async () => {
      try {
        const result = await fetch(
          "https://www.randomnumberapi.com/api/v1.0/random?min=0&max=1"
        ).then((res) => res.json());
        if (result[0] === 0) {
          throw new Error("error");
        }
      } catch (e) {}
    })
  );
  console.log(`after fetch ${i}`, memoryUsage().current);
  await Bun.gc(true);
  console.log("after gc", memoryUsage().current);
}

await new Promise((resolve) => setTimeout(resolve, 10000));
console.log("after sleep", memoryUsage().current);
await Bun.gc(true);
console.log("after sleep gc", memoryUsage().current);

POSIX of environment this test run in: Darwin Kernel Version 20.3.0: Thu Jan 21 00:06:51 PST 2021; root:xnu-7195.81.3~1/RELEASE_ARM64_T8101

P.S. Not an engineer who works a lot with memory and hardware related things

Jarred-Sumner commented 1 year ago

@dimka3553 relevant thing I just remembered: process.memoryUsage.rss() reports RSS more accurately than bun:jsc's memoryUsage().current function. The RSS reported by mi_process_info is actually peak rss whereas process.memoryUsage.rss() is read from a different & more specific API. In https://github.com/oven-sh/bun/pull/3876, I made them consistent

@philosofonusus here is a tweaked version of the script that can be run in both a recent version of Node.js and Bun:

var gc = globalThis.gc || (() => globalThis.Bun.gc(true));
console.log("init", process.memoryUsage.rss());

for (const i of [1, 2]) {
  await Promise.allSettled(
    Array.from({ length: 50 }).map(async () => {
      try {
        const result = await fetch(
          "https://www.randomnumberapi.com/api/v1.0/random?min=0&max=1"
        ).then((res) => res.json());
        if (result[0] === 0) {
          throw new Error("error");
        }
      } catch (e) {}
    })
  );
  console.log(`after fetch ${i}`, process.memoryUsage.rss());
  gc();
  console.log("after gc", process.memoryUsage.rss());
}

await new Promise((resolve) => setTimeout(resolve, 10000));
console.log("after sleep", process.memoryUsage.rss());
gc();
console.log("after sleep gc", process.memoryUsage.rss());

bun:

> bun memory.mjs
init 20561920
after fetch 1 33718272
after gc 33996800
after fetch 2 35504128
after gc 35520512
after sleep 35553280
after sleep gc 35569664

node:

> node --expose-gc memory.js
init 29900800
after fetch 1 82526208
after gc 82624512
after fetch 2 84623360
after gc 84787200
after sleep 77103104
after sleep gc 77185024

As you can see, Bun uses about half the memory of Node.js in this snippet's case. I'm not convinced the code snippet demonstrates a problem (that doesn't mean there isn't a problem here, to be clear). JavaScript is not a reference-counted language which means that deciding when to free the memory is somewhat non-deterministic.

dimka3553 commented 1 year ago

@Jarred-Sumner the problem here is not the memory usage itself but the fact that over time as more fetch requests are made, the usage goes up way above what Node's usage is.

Screenshot 2023-07-29 at 15 42 28

The example might indeed be inconclusive as it takes tens if not hundreds of thousands of fetch requests over a prolonged period to properly demonstrate the problem.

Jarred-Sumner commented 1 year ago

@dimka3553 that graph is helpful, thank you.

This branch https://github.com/oven-sh/bun/pull/3880 switches Bun to use WebKit's memory allocator instead of mimalloc. My hunch is that mimalloc's heap is gradually growing and then never shrinking.

But judging by the test failures, the branch is probably too unstable to use for now. If you want to give it a try, you can download the build from the GitHub action runner for bun-linux-x64

Jarred-Sumner commented 1 year ago

Given it's specific to fetch() requests, it's probably a somewhat small memory leak. Something like 64 bytes.

philosofonusus commented 1 year ago

The example might indeed be inconclusive as it takes tens if not hundreds of thousands of fetch requests over a prolonged period to properly demonstrate the problem.

Снимок экрана 2023-07-29 в 20 33 46

In my actual production context around 52-58 request to the API has led to a permanent memory rise by 21mb going from 207mb to 228mb

subsequent run without env restart has led to 38mb permanent increase in memory usage from 228mb to 266mb.

The next request didn't spike any memory usage instead it started to gradually drop from 266mb to 262mb.

After it I have tried 240requests to the API and from there the permanent memory increase was only from 262mb to 273mb, subsequently I have made the same run which resulted in rise from 273mb to 301mb

All the runs can be seen as spikes on the chart

I'm using fetch wrapped with fetch-retry and proxy with custom headers. It might be something to do with it

Jarred-Sumner commented 1 year ago

Here is a snippet which I think more clearly reproduces the issue:

const batchSize = 20;
const COUNT = 100_000;
var reptitions = 5;
async function run() {
  for (let i = 0; i < COUNT / batchSize; i++) {
    const batch = new Array(batchSize);
    for (let j = 0; j < batchSize; j++) {
      batch[j] = fetch("http://localhost:3000");
    }
    await Promise.all(batch);
  }
  Bun.gc(true);
  console.log(
    "Memory usage: ",
    Math.trunc(process.memoryUsage.rss() / 1024 / 1024),
    "MB"
  );

  if (reptitions--) {
    setTimeout(run, 1);
    return;
  }
}
setTimeout(run, 1);

We can see that memory usage grows by about 8-9 MB per 100,000 requests sent.

Memory usage:  50 MB
Memory usage:  59 MB
Memory usage:  67 MB
Memory usage:  75 MB
Memory usage:  83 MB
Memory usage:  91 MB

This is more conclusive since the volume of the requests is higher, the amounts are higher

Jarred-Sumner commented 1 year ago

alright, I think I have a fix. The first one is my local build and the 2nd one is 0.7.1

image
dimka3553 commented 1 year ago

just FYI, if you run the snippet and use headers in the request, the memory usage goes up by 10 - 11 MB per 100000 requests, that is also something that could be exacerbating the issue over time.

Screenshot 2023-07-30 at 09 16 04

first example is running it with headers, the other without.

const batchSize = 20;
const COUNT = 100_000;
var reptitions = 5;
async function run() {
  for (let i = 0; i < COUNT / batchSize; i++) {
    const batch = new Array(batchSize);
    for (let j = 0; j < batchSize; j++) {
      batch[j] = fetch("http://localhost:3000", {
        headers: {
          "huge-number": "12345679",
        },
      });
    }
    await Promise.all(batch);
  }
  Bun.gc(true);
  console.log(
    "Memory usage: ",
    Math.trunc(process.memoryUsage.rss() / 1024 / 1024),
    "MB"
  );

  if (reptitions--) {
    setTimeout(run, 1);
    return;
  }
}
setTimeout(run, 1);
Jarred-Sumner commented 1 year ago

just FYI, if you run the snippet and use headers in the request, the memory usage goes up by 10 - 11 MB per 100000 requests, that is also something that could be exacerbating the issue over time.

Screenshot 2023-07-30 at 09 16 04

first example is running it with headers, the other without.

const batchSize = 20;
const COUNT = 100_000;
var reptitions = 5;
async function run() {
  for (let i = 0; i < COUNT / batchSize; i++) {
    const batch = new Array(batchSize);
    for (let j = 0; j < batchSize; j++) {
      batch[j] = fetch("http://localhost:3000", {
        headers: {
          "huge-number": "12345679",
        },
      });
    }
    await Promise.all(batch);
  }
  Bun.gc(true);
  console.log(
    "Memory usage: ",
    Math.trunc(process.memoryUsage.rss() / 1024 / 1024),
    "MB"
  );

  if (reptitions--) {
    setTimeout(run, 1);
    return;
  }
}
setTimeout(run, 1);

With #3887 on macOS arm64 with --smol:

❯ bun --smol client-fill.mjs
Memory usage:  40 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB

With #3887 on macOS arm64 without --smol:

❯ bun client-fill.mjs
Memory usage:  40 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB

If we remove the call to Bun.gc(true):

❯ bun client-fill.mjs
Memory usage:  40 MB
Memory usage:  42 MB
Memory usage:  43 MB
Memory usage:  44 MB
Memory usage:  46 MB
Memory usage:  47 MB
❯ bun --smol client-fill.mjs
Memory usage:  40 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Jarred-Sumner commented 1 year ago

I'll hold off on marking this as fixed until @dimka3553 @philosofonusus you confirm that it is fixed. Sometime after the canary finishes building (~40 minutes), can you give it another try?

dimka3553 commented 1 year ago

@Jarred-Sumner, sure

Jarred-Sumner commented 1 year ago

Linux x64:

❯ bun client-fill.mjs
Memory usage:  59 MB
Memory usage:  61 MB
Memory usage:  61 MB
Memory usage:  63 MB
Memory usage:  64 MB
Memory usage:  65 MB
Memory usage:  66 MB
Memory usage:  67 MB
Memory usage:  68 MB
Memory usage:  69 MB
Memory usage:  70 MB
Memory usage:  70 MB
Memory usage:  72 MB
Memory usage:  71 MB
Memory usage:  72 MB
Memory usage:  73 MB
Memory usage:  73 MB
Memory usage:  74 MB
Memory usage:  75 MB
Memory usage:  77 MB
Memory usage:  78 MB
Memory usage:  79 MB
Memory usage:  80 MB
Memory usage:  82 MB
Memory usage:  83 MB
Memory usage:  84 MB
Memory usage:  86 MB
Memory usage:  87 MB
Memory usage:  88 MB
Memory usage:  90 MB
Memory usage:  91 MB
Memory usage:  92 MB
Memory usage:  93 MB
Memory usage:  95 MB
Memory usage:  96 MB
Memory usage:  97 MB
Memory usage:  98 MB
Memory usage:  99 MB
Memory usage:  101 MB
Memory usage:  102 MB
Memory usage:  104 MB
Memory usage:  105 MB
Memory usage:  106 MB
Memory usage:  107 MB
Memory usage:  108 MB
Memory usage:  110 MB
Memory usage:  111 MB
Memory usage:  112 MB
Memory usage:  113 MB
Memory usage:  115 MB
Memory usage:  116 MB
Memory usage:  117 MB
Memory usage:  118 MB
Memory usage:  120 MB
Memory usage:  121 MB
Memory usage:  122 MB
Memory usage:  124 MB
Memory usage:  125 MB
Memory usage:  126 MB
Memory usage:  127 MB
Memory usage:  129 MB
Memory usage:  130 MB
Memory usage:  131 MB
Memory usage:  132 MB
Memory usage:  134 MB
Memory usage:  135 MB
Memory usage:  136 MB
Memory usage:  138 MB
Memory usage:  139 MB
Memory usage:  140 MB
Memory usage:  141 MB
Memory usage:  142 MB
Memory usage:  143 MB
Memory usage:  144 MB
Memory usage:  146 MB
Memory usage:  147 MB
Memory usage:  148 MB
Memory usage:  150 MB
Memory usage:  151 MB
Memory usage:  152 MB
Memory usage:  153 MB
Memory usage:  154 MB
Memory usage:  156 MB
Memory usage:  157 MB
Memory usage:  158 MB
Memory usage:  159 MB
Memory usage:  161 MB
Memory usage:  162 MB
Memory usage:  162 MB
Memory usage:  164 MB
Memory usage:  165 MB
Memory usage:  165 MB
Memory usage:  165 MB
Memory usage:  165 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
    Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB
Memory usage:  166 MB

Linux x64 with --smol:

❯ bun client-fill.mjs
Memory usage:  57 MB
Memory usage:  58 MB
Memory usage:  57 MB
Memory usage:  57 MB
Memory usage:  57 MB
Memory usage:  57 MB
Memory usage:  56 MB
Memory usage:  56 MB
Memory usage:  56 MB
Memory usage:  56 MB
Memory usage:  57 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  55 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  53 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  55 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  54 MB
Memory usage:  53 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  53 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
Memory usage:  54 MB
dimka3553 commented 1 year ago
Screenshot 2023-07-30 at 12 06 15

@Jarred-Sumner Am I doing something wrong?

Jarred-Sumner commented 1 year ago

macOS arm64 hasn't finished compiling yet

dimka3553 commented 1 year ago
Screenshot 2023-07-30 at 12 59 13

I'll put it on the server now to check how it affects long-term memory usage

philosofonusus commented 1 year ago
Снимок экрана 2023-07-30 в 13 08 33

Canary also works on my machine now checking on the server

philosofonusus commented 1 year ago

I have run the same test on my remote machine and it worked just fine, but for some reason, the real app memory keeps increasing. This was run in a canary build with --smol:

Снимок экрана 2023-07-30 в 13 50 33

However, if I force gc after a request in general it seems to be better. The test of 52 parallel requests has resulted in a permanent memory increase 2-3mb. But one thing is that if I do 300-600 parallel requests then it results in 40-50mb requests

Снимок экрана 2023-07-30 в 14 40 50

It can be seen overall that this patch has improved the situation, previously after the 52 parallel requests test there was permanent increase in 20-30mb

philosofonusus commented 1 year ago

What I also see on the canary build, it has started on the redeploy around 30 minutes ago, that the memory keeps jumping and going down even if no processes are running, but overall it can be seen that the memory leak with fetch requests persists because after each run there is a new max permanent memory usage

Снимок экрана 2023-07-30 в 16 54 32
Jarred-Sumner commented 1 year ago

What I also see on the canary build, it has started on the redeploy around 30 minutes ago, that the memory keeps jumping and going down even if no processes are running, but overall it can be seen that the memory leak with fetch requests persists because after each run there is a new max permanent memory usage

Снимок экрана 2023-07-30 в 16 54 32

I think this is consistent with how most GC languages work. The garbage collector's heap grows as the amount of memory consumed grows. If you do many many allocations then the heap size won't shrink a whole lot. If you do fewer, larger allocations then it might shrink more.

Here's a slightly tweaked version of the above script to run 600 requests in Node, wait 200ms between each batch, and then print out the initial rss before each run. Note how Node's memory usage doesn't go back down to the 30 MB or so it started with, instead hovers around 280 MB.

image
philosofonusus commented 1 year ago

What I also see on the canary build, it has started on the redeploy around 30 minutes ago, that the memory keeps jumping and going down even if no processes are running, but overall it can be seen that the memory leak with fetch requests persists because after each run there is a new max permanent memory usage

Снимок экрана 2023-07-30 в 16 54 32

I think this is consistent with how most GC languages work. The garbage collector's heap grows as the amount of memory consumed grows. If you do many many allocations then the heap size won't shrink a whole lot. If you do fewer, larger allocations then it might shrink more.

Here's a slightly tweaked version of the above script to run 600 requests in Node, wait 200ms between each batch, and then print out the initial rss before each run. Note how Node's memory usage doesn't go back down to the 30 MB or so it started with, instead hovers around 280 MB.

image

Yes, I get now why the hovering is happening, but it is consistently growing with each new run and hovering on the level it has grown to

Jarred-Sumner commented 1 year ago

Yes, I get now why the hovering is happening, but it is consistently growing with each new run and hovering on the level it has grown to

Two possibilities come to mind: 1) There is some case where Bun is not scheduling the garbage collector enough 2) There is another memory leak, but smaller. Something like 4 bytes. Or 8 bytes.

If you can come up with a snippet that reproduces the memory growth we can mess with it some more

philosofonusus commented 1 year ago

Yes, I get now why the hovering is happening, but it is consistently growing with each new run and hovering on the level it has grown to

Two possibilities come to mind:

  1. There is some case where Bun is not scheduling the garbage collector enough
  2. There is another memory leak, but smaller. Something like 4 bytes. Or 8 bytes.

If you can come up with a snippet that reproduces the memory growth we can mess with it some more

Ok, I'll dig into it more and send you the minimal reproduction

philosofonusus commented 1 year ago

just FYI, if you run the snippet and use headers in the request, the memory usage goes up by 10 - 11 MB per 100000 requests, that is also something that could be exacerbating the issue over time.

Screenshot 2023-07-30 at 09 16 04

first example is running it with headers, the other without.

const batchSize = 20;
const COUNT = 100_000;
var reptitions = 5;
async function run() {
  for (let i = 0; i < COUNT / batchSize; i++) {
    const batch = new Array(batchSize);
    for (let j = 0; j < batchSize; j++) {
      batch[j] = fetch("http://localhost:3000", {
        headers: {
          "huge-number": "12345679",
        },
      });
    }
    await Promise.all(batch);
  }
  Bun.gc(true);
  console.log(
    "Memory usage: ",
    Math.trunc(process.memoryUsage.rss() / 1024 / 1024),
    "MB"
  );

  if (reptitions--) {
    setTimeout(run, 1);
    return;
  }
}
setTimeout(run, 1);

With #3887 on macOS arm64 with --smol:

❯ bun --smol client-fill.mjs
Memory usage:  40 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB

With #3887 on macOS arm64 without --smol:

❯ bun client-fill.mjs
Memory usage:  40 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB

If we remove the call to Bun.gc(true):

❯ bun client-fill.mjs
Memory usage:  40 MB
Memory usage:  42 MB
Memory usage:  43 MB
Memory usage:  44 MB
Memory usage:  46 MB
Memory usage:  47 MB
❯ bun --smol client-fill.mjs
Memory usage:  40 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB
Memory usage:  41 MB

If for @dimka3553 it works then I think that we can consider this issue resolved, since in particular. I haven't faced this issue on the canary build test. Seems like my issue is similar but not the same, it doesn't require to have a lot of requests to experience a memory leak

dimka3553 commented 1 year ago

@Jarred-Sumner it seems that the memory leak is still there:

Screenshot 2023-07-30 at 20 47 56

The first spike is 0.7.1 the second is the canary version, but, there is a probability that I did not install it correctly or something. I will need to try out again to be sure.

dimka3553 commented 1 year ago

@Jarred-Sumner Ok, I can now confirm that the latest canary version still has a memory leak in the fetch requests it appears to increase the memory usage slower than before, but still after a few days it will restart the container.. Could it be something to do with the number of commits going up in the Bun.DO_NOT_USE_OR_YOU_WILL_BE_FIRED_mimalloc_dump();?

leeoniya commented 1 year ago

Sometime after the canary finishes building (~40 minutes), can you give it another try?

i just ran the canary upgrade and it installs a commit prior to the memleak fix commit :thinking:

image

not sure why canary lags by 13h:

image

dimka3553 commented 1 year ago

@leeoniya good point, it's probably still the old version --> changelog

Jarred-Sumner commented 1 year ago

The canary version is currently 092ada6d. You can verify at runtime via Bun.revision.

Jarred-Sumner commented 1 year ago

@Jarred-Sumner Ok, I can now confirm that the latest canary version still has a memory leak in the fetch requests it appears to increase the memory usage slower than before, but still after a few days it will restart the container.. Could it be something to do with the number of commits going up in the Bun.DO_NOT_USE_OR_YOU_WILL_BE_FIRED_mimalloc_dump();?

Can you try with --smol?

dimka3553 commented 1 year ago

@Jarred-Sumner Ok, I can now confirm that the latest canary version still has a memory leak in the fetch requests it appears to increase the memory usage slower than before, but still after a few days it will restart the container.. Could it be something to do with the number of commits going up in the Bun.DO_NOT_USE_OR_YOU_WILL_BE_FIRED_mimalloc_dump();?

Can you try with --smol?

Both times I tried after the canary update were with --smol.

the revision that is currently on the server is 092ada6d2f31212754e17ab944168463c2d2d327 which is the same one as on my mac. Both pass the 600000 requests test but somehow when running for a prolonged period of time on the server, the memory usage is still going up.

Screenshot 2023-07-31 at 08 39 48
Jarred-Sumner commented 1 year ago

@Jarred-Sumner Ok, I can now confirm that the latest canary version still has a memory leak in the fetch requests it appears to increase the memory usage slower than before, but still after a few days it will restart the container.. Could it be something to do with the number of commits going up in the Bun.DO_NOT_USE_OR_YOU_WILL_BE_FIRED_mimalloc_dump();?

Can you try with --smol?

Both times I tried after the canary update were with --smol.

the revision that is currently on the server is 092ada6d2f31212754e17ab944168463c2d2d327 which is the same one as on my mac. Both pass the 600000 requests test but somehow when running for a prolonged period of time on the server, the memory usage is still going up.

Screenshot 2023-07-31 at 08 39 48

Thank you for trying again

is the complete code from the first comment in this thread? Or is there more code? If I can get the complete code then I should be able to reproduce the memory leak locally.

Are you running Bun.gc(true)? It makes it much slower, but it would help rule out the possibility that it’s a GC scheduling issue

dimka3553 commented 1 year ago

@Jarred-Sumner I sent you the complete code in discord

dimka3553 commented 1 year ago
Screenshot 2023-07-31 at 12 20 12

This is a weekly chart, as to be able to see the slopes more clearly.

Without --smol the memory usage increases by about 22MB per hour While with --smol it increases by 11.5 MB per hour

On the graph the last slope that is quite short is the --smol version it went from 130MB to 150MB in 104 minutes, so its quite conclusive the memory leak is still there.

Note: The previous trials did not include --smol due to a silly mistake, but it's good because it's possible to compare them.

Jarred-Sumner commented 1 year ago

My current three guesses: 1) #3909 is me wondering if maybe mimalloc os keeping some pages around in the different arenas and switching to a single global arena would both answer and help with that 2) JSC’s IncrementalSweeper might need to be scheduled. I think the changes to DeferredWorkTimer skip opportunistic scheduling of IncrementalSweeper 3) another leak somewhere.

Hebilicious commented 1 year ago

FWIW, I was open to open a new issue and stumbled here.

I'm experiencing what I believe is the same issue with an app that I moved from node to bun.

It's an app that sets severals setInterval on a 5s delay, which are making fetch calls and reporting some logs using discord.js channel.send() and codeBlock. The intervals also use nanoid() to generate an id, everything else is ordinary native js (regex, array functions etc, variables assignments ...).

image

On the left what you can see is the memory usage of a docker container running with oven/bun:latest and bun --smol , and on the right you can see the same app running with node:20-bullseye-slim (full command CMD ["dumb-init", "bun", "--smol", "dist/index.js"]).

The bun app crashes and restart because of the leak.

Reading through this comments, I'll try oven/bun:canary, or running bun upgrade before the init script, and report back.

Jarred-Sumner commented 1 year ago

this needs to be fixed before bun 1.0

Jarred-Sumner commented 1 year ago

@dimka3553 it might be a little better in canary, found & fixed a leak in Buffer.toString(“hex”)

https://github.com/oven-sh/bun/pull/4235