Skip to content

⚡ Optimize integer formatting overhead in procfs loop #14

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.

Sign up for GitHub

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

Closed
muou000 wants to merge 1 commit into from
Closed

⚡ Optimize integer formatting overhead in procfs loop #14

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 35 additions & 5 deletions arceos/modules/axfs/src/fs/procfs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,34 @@ use spin::Mutex;

static PID_MAX: core::sync::atomic::AtomicU32 = core::sync::atomic::AtomicU32::new(32768);

fn u64_to_string(mut n: u64) -> String {
if n == 0 {
return "0".into();
}
let mut buf = [0u8; 20];
let mut i = buf.len();
while n > 0 {
i -= 1;
buf[i] = (n % 10) as u8 + b'0';
n /= 10;
}
unsafe { core::str::from_utf8_unchecked(&buf[i..]) }.into()
Comment on lines +29 to +33
}
Comment on lines +22 to +34

@gemini-code-assist gemini-code-assist Bot May 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

When using unsafe blocks in Rust, it is a best practice to document the safety requirements and justifications with a // SAFETY: comment. This helps future maintainers understand why the unsafe operations (such as from_utf8_unchecked) are guaranteed to be safe and do not cause undefined behavior.

fn u64_to_string(mut n: u64) -> String {
    if n == 0 {
        return "0".into();
    }
    let mut buf = [0u8; 20];
    let mut i = buf.len();
    while n > 0 {
        i -= 1;
        buf[i] = (n % 10) as u8 + b'0';
        n /= 10;
    }
    // SAFETY: The buffer is populated only with ASCII digits ('0'..='9'),
    // which are guaranteed to be valid UTF-8. Since `n` is a `u64`, it has
    // at most 20 decimal digits, so `i` will never go out of bounds (>= 0).
    unsafe { core::str::from_utf8_unchecked(&buf[i..]) }.into()
}


fn u32_to_string(mut n: u32) -> String {
if n == 0 {
return "0".into();
}
let mut buf = [0u8; 10];
let mut i = buf.len();
while n > 0 {
i -= 1;
buf[i] = (n % 10) as u8 + b'0';
n /= 10;
}
unsafe { core::str::from_utf8_unchecked(&buf[i..]) }.into()
Comment on lines +43 to +47
}
Comment on lines +36 to +48

@gemini-code-assist gemini-code-assist Bot May 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The u32_to_string function is completely redundant with u64_to_string. Since any u32 can be losslessly cast to u64 (via as u64), we can eliminate u32_to_string entirely to reduce code duplication and minimize the maintenance surface of unsafe code.


const ROOT_INO: u64 = 1;
const MEMINFO_INO: u64 = 2;
const MOUNTS_INO: u64 = 3;
Expand Down Expand Up @@ -448,7 +476,7 @@ impl ProcFilesystem {

if let Some(fds) = provider.process_fds(pid) {
for fd in fds {
let name = format!("{}", fd);
let name = u32_to_string(fd);

@gemini-code-assist gemini-code-assist Bot May 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Use u64_to_string with an as u64 cast instead of the redundant u32_to_string function.

                            let name = u64_to_string(fd as u64);

let child_ino = PID_INODE_START + (pid << PID_INODE_SHIFT) + SUB_INO_FD_BASE + fd as u64;
entries.insert(name.into(), InodeRef::new(child_ino));
}
Expand Down Expand Up @@ -610,7 +638,7 @@ fn render_proc_file(fs: &ProcFilesystem, kind: ProcLiveFileKind) -> String {
ProcLiveFileKind::SelfSymlink => {
if let Some(provider) = PROCESS_PROVIDER.get() {
if let Some(pid) = provider.current_pid() {
return format!("{}", pid);
return u64_to_string(pid);
}
}
"1".to_owned()
Expand Down Expand Up @@ -649,7 +677,9 @@ fn render_proc_file(fs: &ProcFilesystem, kind: ProcLiveFileKind) -> String {
fs.gid_map_map.lock().get(&pid).cloned().unwrap_or_default()
}
ProcLiveFileKind::PidMax => {
format!("{}\n", PID_MAX.load(core::sync::atomic::Ordering::Acquire))
let mut s = u32_to_string(PID_MAX.load(core::sync::atomic::Ordering::Acquire));

@gemini-code-assist gemini-code-assist Bot May 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Use u64_to_string with an as u64 cast instead of the redundant u32_to_string function.

            let mut s = u64_to_string(PID_MAX.load(core::sync::atomic::Ordering::Acquire) as u64);

s.push('\n');
s
}
ProcLiveFileKind::Tainted => {
"0\n".to_owned()
Expand Down Expand Up @@ -859,7 +889,7 @@ impl DirNodeOps for ProcNode {

if let Some(fds) = provider.process_fds(pid) {
for fd in fds {
let name = format!("{}", fd);
let name = u32_to_string(fd);

@gemini-code-assist gemini-code-assist Bot May 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Use u64_to_string with an as u64 cast instead of the redundant u32_to_string function.

                            let name = u64_to_string(fd as u64);

let child_ino = PID_INODE_START + (pid << PID_INODE_SHIFT) + SUB_INO_FD_BASE + fd as u64;
all_entries.push((name, child_ino));
}
Expand All @@ -875,7 +905,7 @@ impl DirNodeOps for ProcNode {
if self.ino == ROOT_INO {
if let Some(provider) = PROCESS_PROVIDER.get() {
for pid in provider.process_pids() {
let name = format!("{}", pid);
let name = u64_to_string(pid);
let child_ino = PID_INODE_START + (pid << PID_INODE_SHIFT) + SUB_INO_DIR;
all_entries.push((name, child_ino));
}
Expand Down