Conversation
Replaces intermediate string allocations (`alloc::format!`) with the `write!` macro to write directly into the pre-allocated string buffer. This eliminates multiple dynamic allocations and frees per line during `maps` generation, reducing overhead and memory fragmentation. Co-authored-by: muou000 <77525792+muou000@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR optimizes ChangesString Formatting Optimization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes string formatting in a #![no_std] environment within pulse_core/src/task/mod.rs by replacing alloc::format! with the core::fmt::Write trait's write! macro to reduce heap allocations in a hot loop. The review feedback suggests further optimizing this process by formatting directly inside the match block when the path is available, which would allow using path.as_str() directly as a &str and completely avoiding the heap allocation of a temporary String.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| let mut is_shared = false; | ||
| let mut offset = 0; | ||
| let mut path_str = String::new(); | ||
| let mut path_str = None; | ||
| let mut inode = 0; | ||
| let mut dev_str = "00:00".to_string(); | ||
| let mut dev_major = 0; | ||
| let mut dev_minor = 0; |
There was a problem hiding this comment.
We can completely avoid the heap allocation for path_str by formatting directly inside the match block when the path is available, and returning early from the closure. This eliminates the need for path_str entirely.
| let mut is_shared = false; | |
| let mut offset = 0; | |
| let mut path_str = String::new(); | |
| let mut path_str = None; | |
| let mut inode = 0; | |
| let mut dev_str = "00:00".to_string(); | |
| let mut dev_major = 0; | |
| let mut dev_minor = 0; | |
| let mut is_shared = false; | |
| let mut offset = 0; | |
| let mut inode = 0; | |
| let mut dev_major = 0; | |
| let mut dev_minor = 0; |
| let loc = cached_file.location(); | ||
| if let Ok(meta) = loc.metadata() { | ||
| inode = meta.inode; | ||
| let major = meta.device >> 8; | ||
| let minor = meta.device & 0xff; | ||
| dev_str = alloc::format!("{:02x}:{:02x}", major, minor); | ||
| dev_major = meta.device >> 8; | ||
| dev_minor = meta.device & 0xff; | ||
| } | ||
| if let Ok(path) = loc.absolute_path() { | ||
| path_str = path.as_str().to_string(); | ||
| path_str = Some(path.as_str().to_string()); | ||
| } | ||
| } | ||
| _ => {} | ||
| } | ||
|
|
||
| let p_char = if is_shared { "s" } else { "p" }; | ||
| if path_str.is_empty() { | ||
| out.push_str(&alloc::format!( | ||
| "{:x}-{:x} {}{}{}{} {:08x} {} {}\n", | ||
| if let Some(path) = path_str { | ||
| let _ = write!( | ||
| &mut out, | ||
| "{:x}-{:x} {}{}{}{} {:08x} {:02x}:{:02x} {:<7} {}\n", | ||
| start.as_usize(), | ||
| end.as_usize(), | ||
| r, w, x, p_char, | ||
| offset, | ||
| dev_str, | ||
| inode | ||
| )); | ||
| dev_major, dev_minor, | ||
| inode, | ||
| path | ||
| ); | ||
| } else { | ||
| out.push_str(&alloc::format!( | ||
| "{:x}-{:x} {}{}{}{} {:08x} {} {:<7} {}\n", | ||
| let _ = write!( | ||
| &mut out, | ||
| "{:x}-{:x} {}{}{}{} {:08x} {:02x}:{:02x} {}\n", | ||
| start.as_usize(), | ||
| end.as_usize(), | ||
| r, w, x, p_char, | ||
| offset, | ||
| dev_str, | ||
| inode, | ||
| path_str | ||
| )); | ||
| dev_major, dev_minor, | ||
| inode | ||
| ); | ||
| } |
There was a problem hiding this comment.
By formatting directly inside the if let Ok(path) = loc.absolute_path() block and returning early from the closure, we can use path.as_str() directly as a &str without allocating a temporary String via .to_string(). This achieves the goal of zero heap allocations inside the hot loop.
let loc = cached_file.location();
if let Ok(meta) = loc.metadata() {
inode = meta.inode;
dev_major = meta.device >> 8;
dev_minor = meta.device & 0xff;
}
if let Ok(path) = loc.absolute_path() {
let p_char = if is_shared { "s" } else { "p" };
let _ = write!(
&mut out,
"{:x}-{:x} {}{}{}{} {:08x} {:02x}:{:02x} {:<7} {}\n",
start.as_usize(),
end.as_usize(),
r, w, x, p_char,
offset,
dev_major, dev_minor,
inode,
path.as_str()
);
return;
}
}
_ => {}
}
let p_char = if is_shared { "s" } else { "p" };
let _ = write!(
&mut out,
"{:x}-{:x} {}{}{}{} {:08x} {:02x}:{:02x} {}\n",
start.as_usize(),
end.as_usize(),
r, w, x, p_char,
offset,
dev_major, dev_minor,
inode
);There was a problem hiding this comment.
Pull request overview
This PR optimizes /proc/<pid>/maps generation in pulse_core by removing per-iteration heap allocations from intermediate alloc::format! calls and formatting directly into the output String via core::fmt::Write.
Changes:
- Replaced
alloc::format!+push_strwithwrite!(&mut out, ...)to avoid temporaryStringallocations in themaps()hot loop. - Reworked device/path handling to avoid allocating a default
"00:00"string and to only store a path when present. - Added a short Bolt note documenting the optimization rationale for future reference.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
pulse_core/src/task/mod.rs |
Writes /proc/<pid>/maps lines directly into the existing output buffer, reducing allocation churn in the hot loop. |
.jules/bolt.md |
Documents the allocation-avoidance approach and rationale for #![no_std] environments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
💡 What: Replaced
alloc::format!intermediate string allocations in the memory mapping parsing logic withcore::fmt::Write::write!to format directly into the existing string buffer.🎯 Why: Generating the
/proc/<pid>/mapsfile runs within a hot loop iterating over memory mappings. Usingalloc::format!multiple times per iteration creates and drops numerous intermediate strings on the heap, causing significant allocation overhead and memory fragmentation in this#![no_std]environment.📊 Impact: Eliminates two temporary string allocations per memory region mapping iteration. Provides a measurable reduction in memory allocations and faster reading of the maps file.
🔬 Measurement: Verified by running
make testfor the PulseOS target to ensure formatting output is identical while confirming system behavior is unaffected.PR created automatically by Jules for task 4381754972197807908 started by @muou000
Summary by CodeRabbit
Refactor
Documentation