-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
fix(ext/node): fix os.freemem #21347
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a test failure to existing test because of this change :(
Oh, deno/runtime/ops/os/sys_info.rs Line 191 in 4ed9278
|
It looks like there's no reliable way to calculate @littledivy @mmastrac Do you happen to know a good way to calculate/simulate MemAvailable value only using |
249af87
to
dc13197
Compare
@kt3k We could implement #[cfg(target_os = "linux")]
{
let mut info = std::mem::MaybeUninit::uninit();
// SAFETY: `info` is a valid pointer to a `libc::sysinfo` struct.
let res = unsafe { libc::sysinfo(info.as_mut_ptr()) };
if res == 0 {
// SAFETY: `sysinfo` initializes the struct.
let info = unsafe { info.assume_init() };
let mem_unit = info.mem_unit as u64;
mem_info.swap_total = info.totalswap * mem_unit;
mem_info.swap_free = info.freeswap * mem_unit;
mem_info.total = info.totalram * mem_unit;
mem_info.free = info.freeram * mem_unit;
mem_info.buffers = info.bufferram * mem_unit;
}
let meminfo = std::fs::read_to_string("/proc/meminfo");
let line = meminfo.lines().find(|l| l.starts_with("MemAvailable:"));
if let Some(line) = line {
let mem = line.split_whitespace().nth(1);
let mem = mem.and_then(|v| v.parse::<i32>().ok());
mem_info.available = mem.unwrap_or(0);
}
} |
Makes sense. Updated |
@@ -211,8 +211,20 @@ pub fn mem_info() -> Option<MemInfo> { | |||
mem_info.swap_free = info.freeswap * mem_unit; | |||
mem_info.total = info.totalram * mem_unit; | |||
mem_info.free = info.freeram * mem_unit; | |||
mem_info.available = mem_info.free; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assigned free
to available
as a fallback for the case when MemAvailable:
is unavailable in /proc/meminfo
. This is the same as what libuv (uv_get_free_memory
) does on linux https://github.com/libuv/libuv/blob/a5c01d4de3695e9d9da34cfd643b5ff0ba582ea7/src/unix/linux.c#L2064
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Can you also update runtime/ops/os/README.md
? -
`mem_info`
| Target family | Syscall
| Description |
| ------------- | ------------------------------------------------------------------------
--------------------------------------------------------------------- | ----------- |
-| Linux | sysinfo
+| Linux | sysinfo and `/proc/meminfo`
| - |
| Windows | `sysinfoapi::GlobalMemoryStatusEx`
| - |
| macOS | <br> <pre> sysctl([CTL_HW, HW_MEMSIZE]); <br> sysctl([CTL_VM,
VM_SWAPUSAGE]); <br> host_statistics64(mach_host_self(), HOST_VM_INFO64)
</pre> | - |
Sure! |
Thank you, @kt3k! |
This PR updates
os.freemem()
.MemAvailable:
of/proc/meminfo
.Deno.systemMemoryInfo().free
).See #21148 (comment) for details.
closes #21148