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 opening react native view controller in iOS #11961

Closed
byohay opened this issue Jan 18, 2017 · 24 comments
Closed

Memory leak when opening react native view controller in iOS #11961

byohay opened this issue Jan 18, 2017 · 24 comments
Labels
Platform: iOS iOS applications. Resolution: Locked This issue was locked by the bot.

Comments

@byohay
Copy link

byohay commented Jan 18, 2017

Description

In my app there's a part with react native. I upgraded to react native 0.40, and noticed that every time I open the react native VC, the memory usage keeps increasing.

Reproduction

I was able to reproduce it even with this small example: https://gist.github.com/byohay/c147f80f0f0b44cdb0026b76980342a4

Additional Information

  • React Native version: 0.40.0
  • Platform: iOS
  • Operating System: MacOS
  • Tested in debug and release
@janicduplessis
Copy link
Contributor

Is this a regression in RN 0.40? What version did you upgrade from?

@byohay
Copy link
Author

byohay commented Jan 23, 2017

I upgraded from RN 0.33. I just checked it with RN 0.39 and it doesn't occur.

@alinz
Copy link

alinz commented Jan 23, 2017

The memory leak in ios v0.40 is pretty bad. also the frame rate on both UI and JS is extremely low. iOS used to be really fast compare to Android.

Evey time, the ios gets refreshed, it adds couple of megabytes into memory. This is a default scene.

memory_leak

@janicduplessis
Copy link
Contributor

cc @javache

@alinz
Copy link

alinz commented Jan 25, 2017

any update on this? @janicduplessis @javache, don't want to put any pressure or anything but releasing v0.41 without fixing this issue does not make any sense! Since we have to wait another month for the next patch!

@javache
Copy link
Member

javache commented Jan 26, 2017

I can see a leak related to bundle loading when running a debug instrumentation build in instruments. This shouldn't affect production builds since those use a bundle on disk.

@alinz
Copy link

alinz commented Jan 26, 2017

@javache what about the poor performance, I always get 25 to 30 frame per second!

@ide
Copy link
Contributor

ide commented Jan 26, 2017

@alinz are you testing on a real device? It's really important that you profile speed/FPS on iPhone/iPad and not the simulator since it's often very different.

@alinz
Copy link

alinz commented Jan 27, 2017

@ide as you said, the fps on the actual device is 60. I should have tested all cases. 😞

@byohay
Copy link
Author

byohay commented Feb 5, 2017

Does 0.41 release includes a fix for this? As @alinz said, it's not feasible for me to use it otherwise.

@julienfouilhe
Copy link

julienfouilhe commented Feb 9, 2017

I am having the same issue using the master branch of the repo, reusing the same view controller and changing the appProperties fixed my problem, but it would be good if it could be freed :)

Same as #11628

It doesn't add only a couple of Mb each time, my view usually takes 200Mb and everytime I was recreating it, 200Mb were added, so nothing from the context gets freed, probably just a strong reference but would anyone know where that would be?

@jasongrishkoff
Copy link

Similar issue from my side. With Android (testing on device), every time I hot refresh the memory piles up. Similarly, if I move to a different view on the app, the memory adds up. It never seems to be cleared.

@julienfouilhe
Copy link

Can someone take a look at this? @emilsjolander, could you maybe ping someone that could know about this? :)

Thank you! :)

@emilsjolander
Copy link
Contributor

If @javache has already looked I don't know who to ping

@alloy
Copy link
Contributor

alloy commented Feb 21, 2017

What isn’t clear to me from the thread is if the memory leak is also witnessed on a static build or if it might be related to the bundle loading, as suggested by @javache earlier ?

@julienfouilhe
Copy link

Oh sorry I didn't see @javache answer. If we are certain this doesn't affect release builds then fine with me.

@alloy
Copy link
Contributor

alloy commented Feb 21, 2017

@julienfouilhe Nobody has verified and responded back yet, so probably a good thing to double-check (and let us know once you’ve done so).

@jasongrishkoff
Copy link

I see this in multiple places in Android. I'm basing this off of adb shell dumpsys meminfo -a

  • When I reload the code, it doesn't clear any old memory or views
  • If I do a "pull to refresh" it loads all the same data, but just adds on to the memory
  • On infinite scroll, every time my ListView adds a new set of rows, memory increases and doesn't disappear
  • If I navigate to new views within my app, the prior views don't get cleared and memory keeps doing up and up and up

@julienfouilhe
Copy link

julienfouilhe commented Feb 21, 2017

@alloy @javache @emilsjolander I just tested by changing the scheme to release and it is still piling up.

Using 0.42.0-rc.3 and this code to init the subview:

static func getRootView() -> RCTRootView? {
    let jsCodeLocation : URL?
    #if DEBUG
        jsCodeLocation = URL(string: "http://localhost:8081/index.ios.bundle?platform=ios")
    #else
        jsCodeLocation = Bundle.main.url(forResource: "editor", withExtension: "jsbundle")
    #endif
    return RCTRootView(
        bundleURL: jsCodeLocation,
        moduleName: "Editor",
        initialProperties: nil,
        launchOptions: nil
    )
}

screen shot 2017-02-21 at 4 42 54 pm

@ide
Copy link
Contributor

ide commented Feb 21, 2017

This might be totally unrelated but I have seen leaks in the past (~ a year ago) when using frameworks instead of static libraries. Specifically the JSContext of the old RN bridge was not being totally cleaned up upon reload when using frameworks.

@haoziy
Copy link

haoziy commented Apr 24, 2017

@byoha the same issue;
I check memorny by Instruments @mkonicek
image

@hramos hramos added the Icebox label Jul 20, 2017
@hramos
Copy link
Contributor

hramos commented Jul 20, 2017

Hi there! This issue is being closed because it has been inactive for a while. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. Either way, we're automatically closing issues after a period of inactivity. Please do not take it personally!

If you think this issue should definitely remain open, please let us know. The following information is helpful when it comes to determining if the issue should be re-opened:

  • Does the issue still reproduce on the latest release candidate? Post a comment with the version you tested.
  • If so, is there any information missing from the bug report? Post a comment with all the information required by the issue template.
  • Is there a pull request that addresses this issue? Post a comment with the PR number so we can follow up.

If you would like to work on a patch to fix the issue, contributions are very welcome! Read through the contribution guide, and feel free to hop into #react-native if you need help planning your contribution.

@hramos hramos closed this as completed Jul 20, 2017
@drpiou
Copy link

drpiou commented Mar 6, 2018

@hramos I still have a same issue... RN0.51

@MikeSilvis
Copy link

@hramos i just updated to the latest, and am still seeing our memory sky rocket. Please take a look at this

@facebook facebook locked as resolved and limited conversation to collaborators Jul 20, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Platform: iOS iOS applications. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests