Avoid cloned strtab offset map
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
43a662acdd8322063c62818599c29abbb71066c0- Parents
-
d608670 - Tree
0735ea5
43a662a
43a662acdd8322063c62818599c29abbb71066c0d608670
0735ea5| Status | File | + | - |
|---|---|---|---|
| M |
src/macho/writer.rs
|
14 | 30 |
| M |
src/string_table.rs
|
66 | 0 |
src/macho/writer.rsmodified@@ -27,7 +27,7 @@ use crate::resolve::{AtomId, InputId}; | ||
| 27 | 27 | use crate::resolve::{Symbol, SymbolId, SymbolTable}; |
| 28 | 28 | use crate::section::is_executable; |
| 29 | 29 | use crate::string_table::StringTableBuilder; |
| 30 | -use crate::symbol::{write_nlist_table, InputSymbol, RawNlist, SymKind}; | |
| 30 | +use crate::symbol::{write_nlist_table, InputSymbol, RawNlist, SymKind, NLIST_SIZE}; | |
| 31 | 31 | use crate::synth::tlv::THREAD_VARIABLE_DESCRIPTOR_SIZE; |
| 32 | 32 | use crate::synth::{ |
| 33 | 33 | code_sig::CodeSignaturePlan, |
@@ -927,7 +927,7 @@ fn build_linkedit_plan_profiled( | ||
| 927 | 927 | timings.symbol_plan_locals += symbol_plan_timings.locals; |
| 928 | 928 | timings.symbol_plan_globals += symbol_plan_timings.globals; |
| 929 | 929 | timings.symbol_plan_strtab += symbol_plan_timings.strtab; |
| 930 | - let mut symtab_bytes = Vec::new(); | |
| 930 | + let mut symtab_bytes = Vec::with_capacity(symbol_plan.symbols.len() * NLIST_SIZE); | |
| 931 | 931 | write_nlist_table(&symbol_plan.symbols, &mut symtab_bytes); |
| 932 | 932 | |
| 933 | 933 | let mut indirect_symbols = Vec::new(); |
@@ -1898,8 +1898,10 @@ fn build_output_symbols_profiled( | ||
| 1898 | 1898 | Vec::new() |
| 1899 | 1899 | }; |
| 1900 | 1900 | |
| 1901 | - let phase_started = std::time::Instant::now(); | |
| 1902 | 1901 | let local_count = if strip_locals { 0 } else { locals.len() }; |
| 1902 | + let external_defined_count = external_defineds.len(); | |
| 1903 | + let undefined_count = undefineds.len(); | |
| 1904 | + let phase_started = std::time::Instant::now(); | |
| 1903 | 1905 | let mut specs = Vec::with_capacity(local_count + external_defineds.len() + undefineds.len()); |
| 1904 | 1906 | if !strip_locals { |
| 1905 | 1907 | specs.extend(locals); |
@@ -1907,27 +1909,11 @@ fn build_output_symbols_profiled( | ||
| 1907 | 1909 | specs.extend(external_defineds); |
| 1908 | 1910 | specs.extend(undefineds); |
| 1909 | 1911 | |
| 1910 | - let mut strtab = StringTableBuilder::new(); | |
| 1911 | - for spec in &specs { | |
| 1912 | - strtab.insert(&spec.name); | |
| 1913 | - } | |
| 1914 | - let (strtab_bytes, strx_by_name) = strtab.finish(); | |
| 1915 | - | |
| 1916 | - let nlocalsym = specs | |
| 1917 | - .iter() | |
| 1918 | - .filter(|spec| spec.partition == OutputSymbolPartition::Local) | |
| 1919 | - .count() as u32; | |
| 1920 | - let nextdefsym = specs | |
| 1921 | - .iter() | |
| 1922 | - .filter(|spec| spec.partition == OutputSymbolPartition::ExternalDefined) | |
| 1923 | - .count() as u32; | |
| 1924 | - let nundefsym = specs | |
| 1925 | - .iter() | |
| 1926 | - .filter(|spec| spec.partition == OutputSymbolPartition::Undefined) | |
| 1927 | - .count() as u32; | |
| 1912 | + let (strtab_bytes, strx_by_spec) = | |
| 1913 | + StringTableBuilder::build_with_name_offsets(specs.iter().map(|spec| spec.name.as_str())); | |
| 1928 | 1914 | |
| 1929 | 1915 | let mut symbols = Vec::with_capacity(specs.len()); |
| 1930 | - let mut symbol_indices = HashMap::new(); | |
| 1916 | + let mut symbol_indices = HashMap::with_capacity(specs.len()); | |
| 1931 | 1917 | let map_symbols = specs |
| 1932 | 1918 | .iter() |
| 1933 | 1919 | .filter(|spec| spec.partition != OutputSymbolPartition::Undefined) |
@@ -1939,9 +1925,7 @@ fn build_output_symbols_profiled( | ||
| 1939 | 1925 | }) |
| 1940 | 1926 | .collect(); |
| 1941 | 1927 | for (idx, spec) in specs.into_iter().enumerate() { |
| 1942 | - let strx = *strx_by_name | |
| 1943 | - .get(&spec.name) | |
| 1944 | - .expect("string table offset missing for output symbol"); | |
| 1928 | + let strx = strx_by_spec[idx]; | |
| 1945 | 1929 | symbols.push(InputSymbol::from_raw(RawNlist { |
| 1946 | 1930 | strx, |
| 1947 | 1931 | n_type: spec.n_type, |
@@ -1964,11 +1948,11 @@ fn build_output_symbols_profiled( | ||
| 1964 | 1948 | exports, |
| 1965 | 1949 | dysymtab: DysymtabCmd { |
| 1966 | 1950 | ilocalsym: 0, |
| 1967 | - nlocalsym, | |
| 1968 | - iextdefsym: nlocalsym, | |
| 1969 | - nextdefsym, | |
| 1970 | - iundefsym: nlocalsym + nextdefsym, | |
| 1971 | - nundefsym, | |
| 1951 | + nlocalsym: local_count as u32, | |
| 1952 | + iextdefsym: local_count as u32, | |
| 1953 | + nextdefsym: external_defined_count as u32, | |
| 1954 | + iundefsym: (local_count + external_defined_count) as u32, | |
| 1955 | + nundefsym: undefined_count as u32, | |
| 1972 | 1956 | ..DysymtabCmd::default() |
| 1973 | 1957 | }, |
| 1974 | 1958 | }, |
src/string_table.rsmodified@@ -108,6 +108,12 @@ struct RootString { | ||
| 108 | 108 | offset: u32, |
| 109 | 109 | } |
| 110 | 110 | |
| 111 | +#[derive(Debug, Clone, Copy, PartialEq, Eq)] | |
| 112 | +struct BorrowedRootString<'a> { | |
| 113 | + name: &'a str, | |
| 114 | + offset: u32, | |
| 115 | +} | |
| 116 | + | |
| 111 | 117 | impl StringTableBuilder { |
| 112 | 118 | pub fn new() -> Self { |
| 113 | 119 | Self::default() |
@@ -117,6 +123,41 @@ impl StringTableBuilder { | ||
| 117 | 123 | self.offsets.entry(name.to_string()).or_insert(0); |
| 118 | 124 | } |
| 119 | 125 | |
| 126 | + pub fn build_with_name_offsets<'a, I>(names: I) -> (Vec<u8>, Vec<u32>) | |
| 127 | + where | |
| 128 | + I: IntoIterator<Item = &'a str>, | |
| 129 | + { | |
| 130 | + let mut entries: Vec<_> = names | |
| 131 | + .into_iter() | |
| 132 | + .enumerate() | |
| 133 | + .map(|(index, name)| (name, index)) | |
| 134 | + .collect(); | |
| 135 | + let mut offsets = vec![0; entries.len()]; | |
| 136 | + entries.sort_by(|(lhs, lhs_index), (rhs, rhs_index)| { | |
| 137 | + reverse_suffix_order(lhs, rhs).then_with(|| lhs_index.cmp(rhs_index)) | |
| 138 | + }); | |
| 139 | + | |
| 140 | + let mut raw = vec![0u8]; | |
| 141 | + let mut roots = Vec::new(); | |
| 142 | + for (name, index) in entries { | |
| 143 | + if let Some(offset) = find_borrowed_suffix_offset(&roots, name) { | |
| 144 | + offsets[index] = offset; | |
| 145 | + continue; | |
| 146 | + } | |
| 147 | + | |
| 148 | + let offset = raw.len() as u32; | |
| 149 | + raw.extend_from_slice(name.as_bytes()); | |
| 150 | + raw.push(0); | |
| 151 | + roots.push(BorrowedRootString { name, offset }); | |
| 152 | + offsets[index] = offset; | |
| 153 | + } | |
| 154 | + | |
| 155 | + while !raw.len().is_multiple_of(8) { | |
| 156 | + raw.push(0); | |
| 157 | + } | |
| 158 | + (raw, offsets) | |
| 159 | + } | |
| 160 | + | |
| 120 | 161 | pub fn finish(mut self) -> (Vec<u8>, HashMap<String, u32>) { |
| 121 | 162 | let mut names: Vec<String> = self.offsets.keys().cloned().collect(); |
| 122 | 163 | names.sort_by(|lhs, rhs| reverse_suffix_order(lhs, rhs)); |
@@ -157,6 +198,16 @@ impl StringTableBuilder { | ||
| 157 | 198 | } |
| 158 | 199 | } |
| 159 | 200 | |
| 201 | +fn find_borrowed_suffix_offset(roots: &[BorrowedRootString<'_>], name: &str) -> Option<u32> { | |
| 202 | + if name.is_empty() { | |
| 203 | + return Some(0); | |
| 204 | + } | |
| 205 | + let insert_at = roots.partition_point(|root| reverse_suffix_order(root.name, name).is_lt()); | |
| 206 | + let existing = roots.get(insert_at.checked_sub(1)?)?; | |
| 207 | + (existing.name.len() >= name.len() && existing.name.ends_with(name)) | |
| 208 | + .then(|| existing.offset + (existing.name.len() - name.len()) as u32) | |
| 209 | +} | |
| 210 | + | |
| 160 | 211 | fn reverse_suffix_order(lhs: &str, rhs: &str) -> std::cmp::Ordering { |
| 161 | 212 | let mut lhs_rev = lhs.bytes().rev(); |
| 162 | 213 | let mut rhs_rev = rhs.bytes().rev(); |
@@ -277,4 +328,19 @@ mod tests { | ||
| 277 | 328 | assert_eq!(table.get(offsets["_alpha"]).unwrap(), "_alpha"); |
| 278 | 329 | assert_eq!(table.get(offsets["_beta"]).unwrap(), "_beta"); |
| 279 | 330 | } |
| 331 | + | |
| 332 | + #[test] | |
| 333 | + fn builder_returns_offsets_in_input_order_without_cloning_keys() { | |
| 334 | + let names = ["_helper", "_afs_helper", "_helper", ""]; | |
| 335 | + let (bytes, offsets) = StringTableBuilder::build_with_name_offsets(names); | |
| 336 | + let table = StringTable::from_bytes(bytes); | |
| 337 | + | |
| 338 | + assert_eq!(table.get(offsets[0]).unwrap(), "_helper"); | |
| 339 | + assert_eq!(table.get(offsets[1]).unwrap(), "_afs_helper"); | |
| 340 | + assert_eq!(table.get(offsets[2]).unwrap(), "_helper"); | |
| 341 | + assert_eq!(table.get(offsets[3]).unwrap(), ""); | |
| 342 | + assert_eq!(offsets[0], offsets[2]); | |
| 343 | + assert_eq!(offsets[0], offsets[1] + 4); | |
| 344 | + assert_eq!(table.as_bytes().len() % 8, 0); | |
| 345 | + } | |
| 280 | 346 | } |