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

fix: Potential memory leak issues in node #368

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tyutjohn
Copy link

@tyutjohn tyutjohn commented Nov 25, 2024

Adding this code is very effective in node. maybe fix #216
Before:
before

After:
after(1)

When running for a long time, it will have a small increase and then become stable. This may depend on the size of the default value of the resvg construction when it is released. I have not found a way to completely release the memory held by rust, but for most scenarios, this increase should be negligible. At present, this code seems to work well.

@tyutjohn
Copy link
Author

I really need it, can anyone take a look at it? Thanks!

@tyutjohn
Copy link
Author

I modified my test script because I got from @napi-rs/napi-rs#613 that GC might not be triggered correctly. After a long period of running, the memory has stabilized after a small increase.
That is my test code

/* eslint-disable no-console */
import { readFileSync, writeFileSync } from 'fs'
import { join } from 'path'

import { Resvg } from './index.js'

// Helper function to read an SVG file
function readSvgFile(filePath) {
  return readFileSync(filePath, 'utf8')
}

// Read an SVG file
const svgContent = readSvgFile(join('./example/bbox.svg'))

function logMemoryUsage(i, rss, heapTotal, heapUsed, external) {
  console.log(
    `${i.padStart(8, ' ')},` +
      `${rss.padStart(12, ' ')},` +
      `${heapTotal.padStart(12, ' ')},` +
      `${heapUsed.padStart(12, ' ')},` +
      `${external.padStart(12, ' ')},`,
  )
}

function sleep() {
  return new Promise((resolve) => setTimeout(resolve, 1000))
}

async function simulateMemoryUsage() {
  logMemoryUsage('i', 'rss', 'heapTotal', 'heapUsed', 'external')

  for (let i = 0; i < 1000000; i++) {
    const opts = {
      background: 'rgba(238, 235, 230, .9)',
      fitTo: {
        mode: 'width',
        value: 1200,
      },
      font: {
        fontFiles: ['./example/SourceHanSerifCN-Light-subset.ttf'],
        loadSystemFonts: false,
      },
    }
    const resvg = new Resvg(svgContent, opts)
    const pngData = resvg.render()
    const pngBuffer = pngData.asPng()
    pngData.free()
    resvg.free()

    if (i % 10 === 0) {
      // NOTE: Do GC before measuring memory usage because the garbage is not measured.
      global.gc()
      await sleep()
      const usage = process.memoryUsage()

      logMemoryUsage(
        i.toString(),
        usage.rss.toString(),
        usage.heapTotal.toString(),
        usage.heapUsed.toString(),
        usage.external.toString(),
      )
    }
  }
}
simulateMemoryUsage()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible Memory Leak
1 participant