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 monitoring is not so accurate (using RSS instead of the more accurate PSS) #11

Open
shijiannihao opened this issue Aug 17, 2022 · 9 comments · May be fixed by #35
Open

Memory monitoring is not so accurate (using RSS instead of the more accurate PSS) #11

shijiannihao opened this issue Aug 17, 2022 · 9 comments · May be fixed by #35
Labels
enhancement New feature or request

Comments

@shijiannihao
Copy link

It is found that memory monitoring is less accurate than Android studio profiler memory monitoring?

@Almouro
Copy link
Member

Almouro commented Aug 17, 2022

Hi @shijiannihao, thanks for posting an issue!

Do you have any examples of reporting that is inaccurate to show? What's the device/os version you're using?

In my experience, Android Studio profiler actually increases the app memory consumption by a few dozens MB so it's usally a bit higher than the reality.

Could also be related to #13 which could completely mess up measures for some devices

@shijiannihao
Copy link
Author

image

image

Hello, the first screenshot is the memory value of Android studio profiler, and the second screenshot is the memory value viewed on flipper using your plug-in. The device operating system version used is Android 11, not Android 13

@Almouro
Copy link
Member

Almouro commented Aug 18, 2022

Thanks @shijiannihao for the screenshots! 🙏

Right, this is quite off in this case. I think this is because we report RSS (Residential Set Size, reported also by top and ps) but Android Studio reports PSS (Proportional Set Size)

Basically Android processes can share memory with other processes, so the PSS takes that into account by proportionally dividing the shared memory between the process that share it, while the RSS does not.

So RSS > PSS (see https://elinux.org/Android_Memory_Usage#procrank for more explanation)

Sure but which is the most accurate?

PSS is said to be the most accurate representation of the program memory footprint (and is the one used by Android Studio, so hopefully they're not wrong)

The Flipper plugin with RSS should still give a good value and should help detect potential memory leaks, however the baseline will always be higher than the value reported by Android Studio.

I think we should still move to use PSS instead, however we have some issues:

  • RSS was easy to get in a performant way (we just have to read one file)
  • PSS requires to read some files like /proc/{pid}/smaps which seem to be denying access (starting with Android 8)
  • the only way we can get this value for a production app seems thus to be via dumpsys meminfo which is exceedingly slow (takes almost 1s!! on my Samsung J3)

Plan

When the C++ profiler gets released, I think we can then, periodically run dumpsys meminfo in a separate thread and report the most recent value since even though the reporting will be slow, memory usage usually doesn't change drastically every 100ms

All in all, stay tuned! 🤞

@shijiannihao
Copy link
Author

Thank you very much for your reply
In our opinion, PSS is more accurate in representing the memory usage of a process

@Almouro Almouro changed the title Memory monitoring is not so accurate Memory monitoring is not so accurate (using RSS instead of the more accurate PSS) Aug 30, 2022
@Almouro Almouro linked a pull request Oct 7, 2022 that will close this issue
2 tasks
@Almouro Almouro added the enhancement New feature or request label May 4, 2023
@MalcolmTomisin
Copy link
Contributor

I looked into #35 and got the gist that getting the PSS report had a significant impact on the device. I used the time command to profile the adb command and it does look like it has negligible impact on device resources, as low as 3% on the CPU. The dumpsys command is not CPU-bound but might be constrained by network latency or device response time, that should explain the 1s execution time.

image

@bamlab bamlab deleted a comment from shijiannihao Sep 6, 2024
@Almouro
Copy link
Member

Almouro commented Sep 6, 2024

Hey @MalcolmTomisin, thanks for taking the time to try that out!

Correct me if I'm wrong, but in your case, would time not measure the CPU load on the host machine and not on the Android phone?
So far, Flashlight has been designed in a way to have minimal performance impact on the Android device used, but doesn't care as much about performance on the machine running the flashlight command

@MalcolmTomisin
Copy link
Contributor

Hi @Almouro,
I made a possibly incorrect assumption that the reason PSS isn't reported on flashlight using dumpsys meminfo is the expensive load on device resources it would be profiling. Thanks for catching my above error...image

Still looks like there's minimal impact on CPU, but what would be the key condition for Flashlight to start reporting memory usage based on PSS?

@Almouro
Copy link
Member

Almouro commented Sep 6, 2024

@MalcolmTomisin oh very interesting!
It's still not negligible (first occurrence is around 60ms), but much less than I previously thought.

I think the original idea from #35 might still hold then, aka running a background thread capturing the PSS memory every 1s, though I don't necessarily think it was super well implemented

The only remaining issue would be what to do for the first measures. At the moment, Flashlight polls measures every 500ms, but if the first RAM measure takes more than 1 or 2s to come, that could be annoying. We might just be able

So technical strat should be something like:

  • on C++ side
    • main thread starts a "meminfo" thread"
    • the "meminfo" thread reads from dumpsys meminfo every 1s and store the output in a std::string value
    • the main thread reads from that value every 500ms and outputs it in a printPssMemoryStats function
    • I think we can still continue to output RSS if needed, since it's just reading one file
  • on JS side, we parse that value properly like in feat(profiler): use pss ram instead of rss #35
  • on JS side, we find a way to handle undefined value for PSS at the beginning, maybe we just don't display the empty value in the graph (the first point would start at 1500ms for instance)

@MalcolmTomisin
Copy link
Contributor

@MalcolmTomisin oh very interesting! It's still not negligible (first occurrence is around 60ms), but much less than I previously thought.

I think the original idea from #35 might still hold then, aka running a background thread capturing the PSS memory every 1s, though I don't necessarily think it was super well implemented

The only remaining issue would be what to do for the first measures. At the moment, Flashlight polls measures every 500ms, but if the first RAM measure takes more than 1 or 2s to come, that could be annoying. We might just be able

So technical strat should be something like:

  • on C++ side

    • main thread starts a "meminfo" thread"
    • the "meminfo" thread reads from dumpsys meminfo every 1s and store the output in a std::string value
    • the main thread reads from that value every 500ms and outputs it in a printPssMemoryStats function
    • I think we can still continue to output RSS if needed, since it's just reading one file
  • on JS side, we parse that value properly like in feat(profiler): use pss ram instead of rss #35

  • on JS side, we find a way to handle undefined value for PSS at the beginning, maybe we just don't display the empty value in the graph (the first point would start at 1500ms for instance)

I tried the above and understood why you didn't choose this route; the impact on performance of running a separate thread getting snapshots of PSS reports was not insignificant. Furthermore, it introduced noise to the live profiling. 😞 I wonder how Android studio is able to report PSS memory. 🤔

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

Successfully merging a pull request may close this issue.

3 participants