feat: improve the value of memory in the return data of getSystemMemoryInfo()
#47628
Conversation
There was a problem hiding this comment.
I'd prefer to expose new numbers, not change existing ones as people will have existing metrics that we'd be breaking.
Also cc @felixrieseberg because I know you've stared at how memory metrics work a lot in the past, any thoughts here
cucbin
changed the title
cucbin
changed the title
|
@MarshallOfSound I agree with you. I have changed the code. |
There was a problem hiding this comment.
Thanks! Could you please also adjust the docs?
Line 206 in 655037f
Could you please also update the PR title and release notes to reflect your most recent change?
nikwen
changed the title
getSystemMemoryInfo()
|
@nikwen |
nikwen
changed the title
getSystemMemoryInfo()getSystemMemoryInfo()
| @@ -183,8 +183,11 @@ v8::Local<v8::Value> ElectronBindings::GetSystemMemoryInfo( | |||
| #endif | |||
| dict.Set("free", free); | |||
|
|
|||
| #if BUILDFLAG(IS_MAC) | |||
| dict.Set("cached", mem_info.file_backed); | |||
There was a problem hiding this comment.
Should this be named fileBacked instead? I see another property named cached and want to make sure this won't conflict in the future.
https://source.chromium.org/chromium/chromium/src/+/main:base/process/process_metrics.h;l=365;drc=17b4c42cfb3512836655464b0d41d319e819c0b6
There was a problem hiding this comment.
Yes, this field name might be better, I have considered using this name before.
Originally I wanted to make the name of this field consistent with what is displayed in the Activity Monitor.
Co-authored-by: Will Anderson <will@itsananderson.com>
Description of Change
On macOS, the free value in the memory data obtained from process.getSystemMemoryInfo() differs significantly from the value displayed in Activity Monitor.
The reason is that the free value is calculated from free_count in vm_statistics64_data_t.
The relevant code is located in: https://source.chromium.org/chromium/chromium/src/+/main:base/process/process_metrics_apple.mm;drc=bcbe20a25707502c2ba340cfd4dbe8cd015fa864;l=255
To handle this issue, I propose two approaches:
Calculate and return the adjusted value directly in electron_bindings.cc while maintaining the original data format.
Add two new fields: cached and purgeable, to provide more granular memory usage details.
Checklist
npm testpassesRelease Notes
Notes: Optimize the value of memory.free in the return data of getSystemMemoryInfo()