Skip to content

Commit

Permalink
Don't allocate unnecessarily in parse_address_data()
Browse files Browse the repository at this point in the history
It makes no sense for parse_address_data() to allocate a vec and collect
results into it, when an iterator can accomplish the same. Convert the
function to using an iterator.

Signed-off-by: Daniel Müller <[email protected]>
  • Loading branch information
d-e-s-o committed Aug 8, 2023
1 parent 05c00ae commit 856a1a2
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 29 deletions.
42 changes: 18 additions & 24 deletions src/gsym/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@
//! See <https://reviews.llvm.org/D53379>

use std::ffi::OsStr;
use std::iter;
use std::mem::align_of;
use std::os::unix::ffi::OsStrExt as _;

use crate::log::warn;
use crate::util::find_match_or_lower_bound;
use crate::util::Pod;
use crate::util::ReadRaw as _;
Expand All @@ -53,8 +53,6 @@ use super::types::AddrInfo;
use super::types::FileInfo;
use super::types::Header;
use super::types::InfoTypeEndOfList;
use super::types::InfoTypeInlineInfo;
use super::types::InfoTypeLineTableInfo;
use super::types::GSYM_MAGIC;
use super::types::GSYM_VERSION;

Expand Down Expand Up @@ -218,30 +216,26 @@ impl GsymContext<'_> {
/// # Arguments
///
/// * `data` - is the slice from AddrInfo::data.
pub fn parse_address_data(mut data: &[u8]) -> Option<Vec<AddrData>> {
let mut data_objs = vec![];

while !data.is_empty() {
pub fn parse_address_data(mut data: &[u8]) -> impl Iterator<Item = AddrData> {
iter::from_fn(move || {
let typ = data.read_u32()?;
let length = data.read_u32()?;
let d = data.read_slice(length as usize)?;
data_objs.push(AddrData {
typ,
length,
data: d,
});

#[allow(non_upper_case_globals)]
match typ {
InfoTypeEndOfList => break,
InfoTypeLineTableInfo | InfoTypeInlineInfo => {}
_ => {
warn!("unknown info type");
}
if typ == InfoTypeEndOfList {
// We are done.
return None
}
}

Some(data_objs)
let len = data.read_u32()?;
let d = data.read_slice(len as usize)?;

// We don't validate `typ` here, because callers will have to dispatch
// on it anyway.

Some(AddrData {
typ,
length: len,
data: d,
})
})
}


Expand Down
10 changes: 5 additions & 5 deletions src/gsym/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use crate::SrcLang;
use crate::SymResolver;

use super::linetab::run_op;
use super::linetab::LineTableRow;
use super::linetab::LineTableHeader;
use super::linetab::LineTableRow;
use super::linetab::RunResult;
use super::parser::parse_address_data;
use super::parser::GsymContext;
Expand Down Expand Up @@ -143,15 +143,15 @@ impl SymResolver for GsymResolver<'_> {
return None
}

let addrdatas = parse_address_data(addrinfo.data)?;
for adr_ent in addrdatas {
if adr_ent.typ != InfoTypeLineTableInfo {
let addrdatas = parse_address_data(addrinfo.data);
for addr_ent in addrdatas {
if addr_ent.typ != InfoTypeLineTableInfo {
continue
}
// Continue to execute all GSYM line table operations
// until the end of the buffer is reached or a row
// containing addr is located.
let mut data = adr_ent.data;
let mut data = addr_ent.data;
let lntab_hdr = LineTableHeader::parse(&mut data)?;
let mut lntab_row = LineTableRow::line_table_row_from(&lntab_hdr, symaddr);
let mut last_lntab_row = lntab_row.clone();
Expand Down

0 comments on commit 856a1a2

Please sign in to comment.