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

refactor: optimizing the logic for retrieving free memory on iOS 13+ #552

Closed
wants to merge 2 commits into from

Conversation

94haox
Copy link

@94haox 94haox commented Sep 3, 2024

In iOS 13 and later, Apple has provided an API to obtain available memory.
link: https://developer.apple.com/documentation/os/3191911-os_proc_available_memory

Copy link
Collaborator

@GLinnik21 GLinnik21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can understand, while os_proc_available_memory() is a more accurate measure for app-specific available memory, it represents a fundamentally different value than the VM stats approach:

  1. Different metric: os_proc_available_memory() shows memory available to the process, while VM stats show system-wide free memory. These are not directly comparable.

  2. Inconsistent reporting: Using os_proc_available_memory() for iOS 13+ and VM stats for earlier versions would create a discontinuity in reported values across iOS versions, potentially confusing users of the crash reporting system.

What do you think?

Sources/KSCrashRecording/Monitors/KSCrashMonitor_System.m Outdated Show resolved Hide resolved
Sources/KSCrashRecording/Monitors/KSCrashMonitor_System.m Outdated Show resolved Hide resolved
@94haox
Copy link
Author

94haox commented Sep 3, 2024

As far as I can understand, while os_proc_available_memory() is a more accurate measure for app-specific available memory, it represents a fundamentally different value than the VM stats approach:

  1. Different metric: os_proc_available_memory() shows memory available to the process, while VM stats show system-wide free memory. These are not directly comparable.
  2. Inconsistent reporting: Using os_proc_available_memory() for iOS 13+ and VM stats for earlier versions would create a discontinuity in reported values across iOS versions, potentially confusing users of the crash reporting system.

What do you think?

I believe your points are valid, but I have a different opinion:

Metric Standards: From the perspective of solving problems, for systems like iOS/tvOS/watchOS that only support single-process apps, the idle memory at the process level is more valuable compared to the system level idle memory (I guess this is also the reason why the API does not support macOS?);

Inconsistency in Reporting: Indeed, such problems may exist, but the current logic, I guess, is a compromise strategy when precise process memory information cannot be obtained (I'm not quite sure)? Compared to accurate information, the priority of report value continuity should be relatively low?

@GLinnik21
Copy link
Collaborator

Thanks for your perspective. The recent implementation in KSCrash seems to address our concerns effectively. You might want to check out these files for more details:

https://github.com/kstenerud/KSCrash/blob/962934327eb42ebee883eca7258df1fd03e2cf89/Sources/KSCrashRecording/KSCrashAppMemoryTracker.m
https://github.com/kstenerud/KSCrash/blob/962934327eb42ebee883eca7258df1fd03e2cf89/Sources/KSCrashRecording/KSCrashAppMemory.m

What do you think about this approach?

@94haox
Copy link
Author

94haox commented Sep 4, 2024

That's great, the feature you mentioned is very useful. It seems I haven't been paying attention to KSCrash for quite a while 😄.

@94haox 94haox closed this Sep 4, 2024
This pull request was closed.
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.

2 participants