cornerstone3D icon indicating copy to clipboard operation
cornerstone3D copied to clipboard

fix: Fix HTJ2K decoder leak

Open abustany opened this issue 1 year ago • 2 comments

Context

Commit 85fd19396762f54c6806fdbebf0235139a67629a changed decodeHTJ2K.js to allocate a new HTJ2KDecoder with each call to decodeAsync. The old decoders somehow stay alive, and are not cleaned from the WASM memory. Each new HTJ2KDecoder allocates in turns buffers for the encoded and decoded pixel data while decoding.

Changes & Results

The change to allocate a new decoder per frame argued that reusing the old decoder is "much slower", but I could not reproduce any slowdowns either in the browser or on Node. This commit therefore reverts the change, so that each call to decodeAsync reuses the same decoder instance.

Testing

Checklist

PR

  • [x] My Pull Request title is descriptive, accurate and follows the semantic-release format and guidelines.

Code

  • [] My code has been well-documented (function documentation, inline comments, etc.)

  • [] I have run the yarn build:update-api to update the API documentation, and have committed the changes to this PR. (Read more here https://www.cornerstonejs.org/docs/contribute/update-api)

Public Documentation Updates

  • [] The documentation page has been updated as necessary for any public API additions or removals.

Tested Environment

  • [x] "OS: Linux
  • [x] "Node version: 20.11
  • [x] "Browser: Chrome 126, Firefox 128

abustany avatar Jul 11 '24 10:07 abustany

Deploy Preview for cornerstone-3d-docs ready!

Name Link
Latest commit e72f82a2665a1d472ccea8a757f44d65a59516ba
Latest deploy log https://app.netlify.com/sites/cornerstone-3d-docs/deploys/668fb02986ad0c0008023b5a
Deploy Preview https://deploy-preview-1388--cornerstone-3d-docs.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Jul 11 '24 10:07 netlify[bot]

If you update this PR to include an openjph version change for the release after https://github.com/cornerstonejs/codecs/pull/46 then I will approve the PR and merge it.

wayfarer3130 avatar Aug 02 '24 13:08 wayfarer3130