-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add unwind info manager implementation and use it #129
Draft
javierhonduco
wants to merge
3
commits into
main
Choose a base branch
from
persist-unwind-info-2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,201 @@ | ||
use lightswitch_object::ExecutableId; | ||
use std::collections::BinaryHeap; | ||
use std::fs; | ||
use std::io::BufWriter; | ||
use std::io::Read; | ||
use std::path::Path; | ||
use std::path::PathBuf; | ||
use std::time::Instant; | ||
use std::{fs::File, io::BufReader}; | ||
|
||
use super::persist::{Reader, Writer}; | ||
use crate::unwind_info::types::CompactUnwindRow; | ||
|
||
const DEFAULT_MAX_CACHED_FILES: usize = 1_000; | ||
|
||
#[derive(Debug, PartialEq, Eq)] | ||
struct Usage { | ||
executable_id: ExecutableId, | ||
instant: Instant, | ||
} | ||
|
||
// `BinaryHeap::pop()` returns the biggest element, so reverse it | ||
// to get the smallest one AKA oldest for both `PartialOrd` and `Ord`. | ||
impl PartialOrd for Usage { | ||
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> { | ||
Some(other.cmp(self)) | ||
} | ||
} | ||
|
||
impl Ord for Usage { | ||
fn cmp(&self, other: &Self) -> std::cmp::Ordering { | ||
other.instant.cmp(&self.instant) | ||
} | ||
} | ||
|
||
/// Provides unwind information with caching on the file system, expiring | ||
/// older files if there are more than `max_cached_files`. | ||
pub struct UnwindInfoManager { | ||
cache_dir: PathBuf, | ||
usage_tracking: BinaryHeap<Usage>, | ||
max_cached_files: usize, | ||
} | ||
|
||
impl UnwindInfoManager { | ||
pub fn new(cache_dir: &Path, max_cached_files: Option<usize>) -> Self { | ||
let max_cached_files = max_cached_files.unwrap_or(DEFAULT_MAX_CACHED_FILES); | ||
let _ = fs::create_dir(cache_dir); | ||
let mut manager = UnwindInfoManager { | ||
cache_dir: cache_dir.to_path_buf(), | ||
usage_tracking: BinaryHeap::with_capacity(max_cached_files), | ||
max_cached_files, | ||
}; | ||
let _ = manager.bump_already_present(); | ||
manager | ||
} | ||
|
||
pub fn fetch_unwind_info( | ||
&mut self, | ||
executable_path: &Path, | ||
executable_id: ExecutableId, | ||
) -> anyhow::Result<Vec<CompactUnwindRow>> { | ||
match self.read_from_cache(executable_id) { | ||
Ok(unwind_info) => { | ||
println!("unwind info found in cache {:x}", executable_id); | ||
Ok(unwind_info) | ||
} | ||
Err(_) => { | ||
// @todo: only do this on file not found, or if the file is from another ABI | ||
println!("unwind info not found in cache {:x}", executable_id); | ||
let unwind_info = self.write_to_cache(executable_path, executable_id); | ||
if unwind_info.is_ok() { | ||
self.bump(executable_id, None); | ||
} | ||
unwind_info | ||
} | ||
} | ||
} | ||
|
||
fn read_from_cache( | ||
&self, | ||
executable_id: ExecutableId, | ||
) -> anyhow::Result<Vec<CompactUnwindRow>> { | ||
let unwind_info_path = self.path_for(executable_id); | ||
let file = File::open(unwind_info_path)?; | ||
|
||
let mut buffer = BufReader::new(file); | ||
let mut data = Vec::new(); | ||
buffer.read_to_end(&mut data)?; | ||
let reader = Reader::new(&data)?; | ||
|
||
Ok(reader.unwind_info()?) | ||
} | ||
|
||
fn write_to_cache( | ||
&self, | ||
executable_path: &Path, | ||
executable_id: ExecutableId, | ||
) -> anyhow::Result<Vec<CompactUnwindRow>> { | ||
// If there's already a file and reading failed, for example | ||
// due to an incompatible version or any other issue. | ||
let _ = fs::remove_file(executable_path); | ||
let unwind_info_path = self.path_for(executable_id); | ||
let unwind_info_writer = Writer::new(executable_path); | ||
let mut file = BufWriter::new(File::create(unwind_info_path)?); | ||
unwind_info_writer.write(&mut file) | ||
} | ||
|
||
fn path_for(&self, executable_id: ExecutableId) -> PathBuf { | ||
self.cache_dir.join(format!("{:x}", executable_id)) | ||
} | ||
|
||
pub fn bump_already_present(&mut self) -> anyhow::Result<()> { | ||
for direntry in fs::read_dir(&self.cache_dir)?.flatten() { | ||
let name = direntry.file_name(); | ||
let Some(name) = name.to_str() else { continue }; | ||
let executable_id = ExecutableId::from_str_radix(name, 16)?; | ||
|
||
let metadata = fs::metadata(direntry.path())?; | ||
let modified = metadata.created()?; | ||
|
||
self.bump(executable_id, Some(Instant::now() - modified.elapsed()?)); | ||
} | ||
|
||
Ok(()) | ||
} | ||
|
||
fn bump(&mut self, executable_id: ExecutableId, instant: Option<Instant>) { | ||
let instant = instant.unwrap_or(Instant::now()); | ||
|
||
self.usage_tracking.push(Usage { | ||
executable_id, | ||
instant, | ||
}); | ||
|
||
self.maybe_evict() | ||
} | ||
|
||
fn maybe_evict(&mut self) { | ||
if self.usage_tracking.len() > self.max_cached_files { | ||
if let Some(evict) = self.usage_tracking.pop() { | ||
let _ = fs::remove_file(self.path_for(evict.executable_id)); | ||
} | ||
} | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
use crate::unwind_info::compact_unwind_info; | ||
use std::{path::PathBuf, time::Duration}; | ||
|
||
#[test] | ||
fn test_usage_ord() { | ||
let now = Instant::now(); | ||
let before = Usage { | ||
executable_id: 0xBAD, | ||
instant: now, | ||
}; | ||
let after = Usage { | ||
executable_id: 0xFAD, | ||
instant: now + Duration::from_secs(10), | ||
}; | ||
|
||
// `BinaryHeap::pop()` returns the max element so the ordering is switched. | ||
assert_eq!([before, after].iter().max().unwrap().executable_id, 0xBAD); | ||
} | ||
|
||
#[test] | ||
fn test_unwind_info_manager_unwind_info() { | ||
let unwind_info = compact_unwind_info("/proc/self/exe").unwrap(); | ||
let tmpdir = tempfile::TempDir::new().unwrap(); | ||
let mut manager = UnwindInfoManager::new(tmpdir.path(), None); | ||
|
||
// The unwind info fetched with the manager should be correct | ||
// both when it's a cache miss and a cache hit. | ||
for _ in 0..2 { | ||
let manager_unwind_info = | ||
manager.fetch_unwind_info(&PathBuf::from("/proc/self/exe"), 0xFABADA); | ||
assert!(manager_unwind_info.is_ok()); | ||
let manager_unwind_info = manager_unwind_info.unwrap(); | ||
assert!(!manager_unwind_info.is_empty()); | ||
assert_eq!(unwind_info, manager_unwind_info); | ||
} | ||
} | ||
|
||
#[test] | ||
fn test_unwind_info_manager_cache_eviction() { | ||
let tmpdir = tempfile::TempDir::new().unwrap(); | ||
let path = tmpdir.path(); | ||
|
||
// Creaty dummy cache entries. | ||
for i in 0..20 { | ||
File::create(path.join(format!("{:x}", i))).unwrap(); | ||
} | ||
|
||
assert_eq!(fs::read_dir(path).unwrap().collect::<Vec<_>>().len(), 20); | ||
UnwindInfoManager::new(path, Some(4)); | ||
assert_eq!(fs::read_dir(path).unwrap().collect::<Vec<_>>().len(), 4); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
mod convert; | ||
pub mod manager; | ||
mod optimize; | ||
pub mod pages; | ||
pub mod persist; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,27 +39,26 @@ unsafe impl Plain for Header {} | |
/// read path, in case the data is corrupted. | ||
unsafe impl Plain for CompactUnwindRow {} | ||
|
||
/// Writes compact information to a given writer. | ||
struct Writer { | ||
pub struct Writer { | ||
executable_path: PathBuf, | ||
} | ||
|
||
impl Writer { | ||
fn new(executable_path: &Path) -> Self { | ||
pub fn new(executable_path: &Path) -> Self { | ||
Writer { | ||
executable_path: executable_path.to_path_buf(), | ||
} | ||
} | ||
|
||
fn write<W: Write + Seek>(self, writer: &mut W) -> anyhow::Result<()> { | ||
pub fn write<W: Write + Seek>(self, writer: &mut W) -> anyhow::Result<Vec<CompactUnwindRow>> { | ||
let unwind_info = self.read_unwind_info()?; | ||
// Write dummy header. | ||
self.write_header(writer, 0, None)?; | ||
let digest = self.write_unwind_info(writer, &unwind_info)?; | ||
// Write real header. | ||
writer.seek(SeekFrom::Start(0))?; | ||
self.write_header(writer, unwind_info.len(), Some(digest))?; | ||
Ok(()) | ||
Ok(unwind_info) | ||
} | ||
|
||
fn read_unwind_info(&self) -> anyhow::Result<Vec<CompactUnwindRow>> { | ||
|
@@ -118,8 +117,7 @@ pub enum ReaderError { | |
Digest, | ||
} | ||
|
||
/// Reads compact information of a bytes slice. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops must have removed it while rebasing, will re-add it |
||
struct Reader<'a> { | ||
pub struct Reader<'a> { | ||
header: Header, | ||
data: &'a [u8], | ||
} | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Oops must have removed it while rebasing, will re-add it