samizdatco / skia-canvas

A GPU-accelerated 2D graphics environment for Node.js
MIT License
1.67k stars 63 forks source link

garbage collection / memory deallocation gone wrong? #120

Open nitreojs opened 1 year ago

nitreojs commented 1 year ago

tried to render my project and ran into some issues:

below i posted code which renders simple rectangle one time (!) and then renders to buffer as JPG 100 times. in skia-canvas@1.0.0, this process takes from 150 MiB up to 333 MiB. just a rectangle! in skia-canvas@0.9.30, though, memory usage increases from 50 MiB to 130 MiB. :thinking:

anyways, here's the code:

// node --expose-gc leakage-test.js
const { Canvas } = require('skia-canvas')

async function printMem(name) {
  await new Promise((r) => setTimeout(r, 1000))

  global.gc()

  const mem = process.memoryUsage()
  const rss = (mem.rss / 1024 / 1024).toFixed(0).padStart(3)
  const heap = (mem.heapUsed / 1024 / 1024).toFixed(0).padStart(3)
  const external = (mem.external / 1024 / 1024).toFixed(0).padStart(3)

  console.log(`${rss} MiB / ${heap} MiB / ${external} MiB / ${name}`)
}

const run = async () => {
  const canvas = new Canvas(1920, 640)
  const context = canvas.getContext('2d')

  context.fillStyle = 'red'
  context.fillRect(300, 500, 250, 170)

  for (let i = 0; i < 100; i++) {
    await canvas.toBuffer('jpg')

    await printMem(`iteration #${i}`)
  }
}

run().catch(console.error)

i tried many methods: using one shared canvas, clearing that canvas after every iteration via context.clearRect(0, 0, canvas.width, canvas.height), forcing global.gc() every iteration — nothing worked

AugustGSP commented 1 year ago

I have the same issue with buffers. My application quickly runs out of memory.

I tried your code using node-canvas, and no leaks. I will be switching to that.

nitreojs commented 1 year ago

I have the same issue with buffers. My application quickly runs out of memory.

I tried your code using node-canvas, and no leaks. I will be switching to that.

the problem is that node-canvas actually sucks, it can't do a lot of things skia-canvas can. for example, simple and basic gaussian blur of the photo is nearly impossible (if not impossible at all) to do with node-canvas, but with skia-canvas it's as easy as two or three lines of code =D

AugustGSP commented 1 year ago

the problem is that node-canvas actually sucks, it can't do a lot of things skia-canvas can. for example, simple and basic gaussian blur of the photo is nearly impossible (if not impossible at all) to do with node-canvas, but with skia-canvas it's as easy as two or three lines of code =D

I know what you mean ha. The blur was the main reason i opted for skia-canvas over node-canvas in the first place.

But since your issue was given no attention what so ever, there was no guessing when or even if this issue will ever be resolved. I had to either accept that there would be no blur, or accept that the app would experience a fatal crash 20 times per day. I can't speak for you, but for my use-case that choice was easy.

It's unfortunate for anyone where the blur is a more necessary feature though, because afaik there really are no alternatives.

fyt0215 commented 1 year ago

I have the same issue. But I tried to set canvas.gpu = false, and no leaks.

aaronnuu commented 1 year ago

I have the same issue. But I tried to set canvas.gpu = false, and no leaks.

Can confirm that this worked for me, solved the memory leak issues. I imagine this limits some performance in certain situation but for my case I wasn't taking advantage of the GPU acceleration of the library anyway so totally fine.