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

Memory Leak when Navigating on iOS #1403

Closed
thomas-coldwell opened this issue Mar 4, 2023 · 12 comments · Fixed by #1420 or #1467
Closed

Memory Leak when Navigating on iOS #1403

thomas-coldwell opened this issue Mar 4, 2023 · 12 comments · Fixed by #1420 or #1467
Labels
bug Something isn't working

Comments

@thomas-coldwell
Copy link

thomas-coldwell commented Mar 4, 2023

Description

It appears as though Skia has a memory leak which is particularly noticeable when navigating back and forth between a screen with a canvas rendering a Skia image. As you can see in the below screen recording the RAM usage continuously grows and this is also apparent when running the Allocations instrument from XCode (happy to send the trace file if you can't repro it on your end).

I've been able to repro this on a physical iPhone 13 Pro and on an iPhone 14 emulator

Simulator.Screen.Recording.-.iPhone.14.-.2023-03-04.at.16.07.31.mp4

Screenshot 2023-03-04 at 16 21 41

Version

0.1.176 (but I've tested as far back as 0.1.172 and can repro it there too)

Steps to reproduce

  • Build the iOS Example app
  • Repeatedly navigation onto the "Filter Example" and back to the home screen
  • See the total RAM usage on the perf monitor continuously increase
  • Can also run the "Allocations" instrument from XCode and profile to see RNSkia items are not deallocated

Snack, code example, screenshot, or link to a repository

  • Use the "Filter Example" in the example app on v0.1.176 to reproduce
@thomas-coldwell thomas-coldwell added the bug Something isn't working label Mar 4, 2023
@thomas-coldwell thomas-coldwell changed the title Memory Leak on iOS Memory Leak when Navigating on iOS Mar 4, 2023
@thomas-coldwell
Copy link
Author

Commented out the useImage and <ImageShader /> on the "Filter Example" and everything seems to allocate and deallocate properly with no growth in memory usage:

// const image = useImage(require("../../assets/oslo.jpg"));
  // if (image === null) {
  //   return null;
  // }
  return (
    <Canvas style={{ width, height }} onTouch={() => setState((i) => i + 1)}>
      {/* <Fill>
        <Shader source={source} uniforms={uniforms}>
          <ImageShader
            image={image}
            fit="cover"
            x={0}
            y={0}
            tx="repeat"
            ty="repeat"
            width={width}
            height={height}
          />
        </Shader>
      </Fill> */}
    </Canvas>
  );

@chrfalch
Copy link
Contributor

chrfalch commented Mar 5, 2023

Awesome find, @thomas-coldwell - will investigate asap.

@chrfalch
Copy link
Contributor

@thomas-coldwell - just adding some info here based on our offline discussions.

A branch has been created that should fix this issue. The problem was basically a reference to a jsi::Object in the native dependency manager that ended up keeping nodes and property values alive.

The problem was basically that the image property of the Image (or ImageShader) was not released correctly resulting in memory growing. We could see this by adding a simple constructor to the JsiSkImage class (used for the image property) and observing that this code was never hit.

After the implemented fix we could see that the destructor started to be called again, showing that we are now releasing images correctly.

@thomas-coldwell
Copy link
Author

Thanks Christian 🙌 Just a few observations I've made while running the branch so far:

  • The dtor for the skGroup & skImage isn't always called and when it is it seems to be called navigating back to the screen with the skia canvas, not when navigating away from that screen (and it being unmounted) e.g. returning to the home screen in the example app
  • It also seems that even when the dtor is called for the skImage this does not release the JsiSkImage HostObject - I'm taking a bit of a further look at the memory graph for this and the call stack that creates anything that holds onto this HostObject - will let you know what I find and report back here 😃

@thomas-coldwell
Copy link
Author

Did a little bit more debugging and when disabling hermes (so using JSC) everything got deallocated properly if that helps us pin down the issue any further 😃

@chrfalch
Copy link
Contributor

Thanks for the insights! We've seen that hermes GC is a bit less deterministic than JSC, maybe this is the reason? The findings you see above (both that some of these nodes not being released and that JSC seems to be working correctly) should imply that this has something to do with how Hermes captures and releases references?

@thomas-coldwell
Copy link
Author

Yes I think so - in the memory graph there also seems to be a lot of "non-object from hermes" nodes holding onto the JsiSkImage HostObject too, even after the dtor gets called for the skImage so that would definitely check out

@chrfalch
Copy link
Contributor

Seen the same thing here - but it is super hard to know what Hermes is doing here since everything is just "non-object from hermes" - without any additional information.

@chrfalch
Copy link
Contributor

In the client code you were working on - can you now still reproduce the problem (preferably on device in release mode)? If so lets get together again and track it down!

@thomas-coldwell
Copy link
Author

Yeah I can still reproduce it in release mode on a physical device (iPhone 13 Pro) in the example app by opening and closing the filter example (I've actually removed the filter so it just shows the image). It seems much better in release mode and the image HostObjects do actually get deallocated at points, but overall the memory usage is still growing so there is definitely some persistent objects.

Will ping you on slack 🙌

@amitchaudhary140
Copy link

I am facing the same memory leak issue when using image. When can I expect fix on this?

@kospaa
Copy link

kospaa commented Jan 10, 2025

Hello is there any update on this issue ? Still experiencing this with the latest release on ios, everythings smooth on android

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants