Speed string table planning
- SHA
86f5fa3b080aaf62b882b22b15081a471ab4c778- Parents
-
23e8dad - Tree
f9dc1f0
86f5fa3
86f5fa3b080aaf62b882b22b15081a471ab4c77823e8dad
f9dc1f0| Status | File | + | - |
|---|---|---|---|
| M |
src/lib.rs
|
12 | 0 |
| M |
src/string_table.rs
|
42 | 9 |
| M |
tests/perf_baseline.rs
|
11 | 1 |
src/lib.rsmodified@@ -215,6 +215,9 @@ pub struct LinkPhaseTimings { | ||
| 215 | 215 | pub synth_sections: Duration, |
| 216 | 216 | pub synth_linkedit_finalize: Duration, |
| 217 | 217 | pub synth_linkedit_symbol_plan: Duration, |
| 218 | + pub synth_linkedit_symbol_plan_locals: Duration, | |
| 219 | + pub synth_linkedit_symbol_plan_globals: Duration, | |
| 220 | + pub synth_linkedit_symbol_plan_strtab: Duration, | |
| 218 | 221 | pub synth_linkedit_dyld_info: Duration, |
| 219 | 222 | pub synth_linkedit_metadata_tables: Duration, |
| 220 | 223 | pub synth_linkedit_code_signature: Duration, |
@@ -666,6 +669,9 @@ impl Linker { | ||
| 666 | 669 | let mut linkedit = None; |
| 667 | 670 | let mut synth_linkedit_finalize = Duration::ZERO; |
| 668 | 671 | let mut synth_linkedit_symbol_plan = Duration::ZERO; |
| 672 | + let mut synth_linkedit_symbol_plan_locals = Duration::ZERO; | |
| 673 | + let mut synth_linkedit_symbol_plan_globals = Duration::ZERO; | |
| 674 | + let mut synth_linkedit_symbol_plan_strtab = Duration::ZERO; | |
| 669 | 675 | let mut synth_linkedit_dyld_info = Duration::ZERO; |
| 670 | 676 | let mut synth_linkedit_metadata_tables = Duration::ZERO; |
| 671 | 677 | let mut synth_linkedit_code_signature = Duration::ZERO; |
@@ -682,6 +688,9 @@ impl Linker { | ||
| 682 | 688 | )?; |
| 683 | 689 | synth_linkedit_finalize += phase_started.elapsed(); |
| 684 | 690 | synth_linkedit_symbol_plan += linkedit_timings.symbol_plan; |
| 691 | + synth_linkedit_symbol_plan_locals += linkedit_timings.symbol_plan_locals; | |
| 692 | + synth_linkedit_symbol_plan_globals += linkedit_timings.symbol_plan_globals; | |
| 693 | + synth_linkedit_symbol_plan_strtab += linkedit_timings.symbol_plan_strtab; | |
| 685 | 694 | synth_linkedit_dyld_info += linkedit_timings.dyld_info; |
| 686 | 695 | synth_linkedit_metadata_tables += linkedit_timings.metadata_tables; |
| 687 | 696 | synth_linkedit_code_signature += linkedit_timings.code_signature; |
@@ -703,6 +712,9 @@ impl Linker { | ||
| 703 | 712 | let linkedit = linkedit.expect("finalize loop always runs at least once"); |
| 704 | 713 | phases.synth_linkedit_finalize = synth_linkedit_finalize; |
| 705 | 714 | phases.synth_linkedit_symbol_plan = synth_linkedit_symbol_plan; |
| 715 | + phases.synth_linkedit_symbol_plan_locals = synth_linkedit_symbol_plan_locals; | |
| 716 | + phases.synth_linkedit_symbol_plan_globals = synth_linkedit_symbol_plan_globals; | |
| 717 | + phases.synth_linkedit_symbol_plan_strtab = synth_linkedit_symbol_plan_strtab; | |
| 706 | 718 | phases.synth_linkedit_dyld_info = synth_linkedit_dyld_info; |
| 707 | 719 | phases.synth_linkedit_metadata_tables = synth_linkedit_metadata_tables; |
| 708 | 720 | phases.synth_linkedit_code_signature = synth_linkedit_code_signature; |
src/string_table.rsmodified@@ -98,10 +98,17 @@ impl StringTable { | ||
| 98 | 98 | |
| 99 | 99 | #[derive(Debug, Clone, Default, PartialEq, Eq)] |
| 100 | 100 | pub struct StringTableBuilder { |
| 101 | - roots: Vec<(String, u32)>, | |
| 101 | + roots: Vec<RootString>, | |
| 102 | + roots_by_last_byte: HashMap<u8, Vec<usize>>, | |
| 102 | 103 | offsets: HashMap<String, u32>, |
| 103 | 104 | } |
| 104 | 105 | |
| 106 | +#[derive(Debug, Clone, PartialEq, Eq)] | |
| 107 | +struct RootString { | |
| 108 | + name: String, | |
| 109 | + offset: u32, | |
| 110 | +} | |
| 111 | + | |
| 105 | 112 | impl StringTableBuilder { |
| 106 | 113 | pub fn new() -> Self { |
| 107 | 114 | Self::default() |
@@ -125,7 +132,17 @@ impl StringTableBuilder { | ||
| 125 | 132 | let offset = raw.len() as u32; |
| 126 | 133 | raw.extend_from_slice(name.as_bytes()); |
| 127 | 134 | raw.push(0); |
| 128 | - self.roots.push((name.clone(), offset)); | |
| 135 | + let root_index = self.roots.len(); | |
| 136 | + self.roots.push(RootString { | |
| 137 | + name: name.clone(), | |
| 138 | + offset, | |
| 139 | + }); | |
| 140 | + if let Some(&last_byte) = name.as_bytes().last() { | |
| 141 | + self.roots_by_last_byte | |
| 142 | + .entry(last_byte) | |
| 143 | + .or_default() | |
| 144 | + .push(root_index); | |
| 145 | + } | |
| 129 | 146 | self.offsets.insert(name, offset); |
| 130 | 147 | } |
| 131 | 148 | |
@@ -136,13 +153,16 @@ impl StringTableBuilder { | ||
| 136 | 153 | } |
| 137 | 154 | |
| 138 | 155 | fn find_suffix_offset(&self, name: &str) -> Option<u32> { |
| 139 | - self.roots.iter().find_map(|(existing, offset)| { | |
| 140 | - if existing.ends_with(name) { | |
| 141 | - Some(*offset + (existing.len() - name.len()) as u32) | |
| 142 | - } else { | |
| 143 | - None | |
| 144 | - } | |
| 145 | - }) | |
| 156 | + let last_byte = *name.as_bytes().last()?; | |
| 157 | + self.roots_by_last_byte | |
| 158 | + .get(&last_byte)? | |
| 159 | + .iter() | |
| 160 | + .find_map(|&idx| { | |
| 161 | + let existing = &self.roots[idx]; | |
| 162 | + (existing.name.len() >= name.len() && existing.name.ends_with(name)).then(|| { | |
| 163 | + existing.offset + (existing.name.len() - name.len()) as u32 | |
| 164 | + }) | |
| 165 | + }) | |
| 146 | 166 | } |
| 147 | 167 | } |
| 148 | 168 | |
@@ -253,4 +273,17 @@ mod tests { | ||
| 253 | 273 | assert_eq!(array, afs + 4); |
| 254 | 274 | assert_eq!(table.as_bytes().len() % 8, 0); |
| 255 | 275 | } |
| 276 | + | |
| 277 | + #[test] | |
| 278 | + fn builder_ignores_same_last_byte_non_suffix_names() { | |
| 279 | + let mut builder = StringTableBuilder::new(); | |
| 280 | + builder.insert("_alpha"); | |
| 281 | + builder.insert("_beta"); | |
| 282 | + | |
| 283 | + let (bytes, offsets) = builder.finish(); | |
| 284 | + let table = StringTable::from_bytes(bytes); | |
| 285 | + | |
| 286 | + assert_eq!(table.get(offsets["_alpha"]).unwrap(), "_alpha"); | |
| 287 | + assert_eq!(table.get(offsets["_beta"]).unwrap(), "_beta"); | |
| 288 | + } | |
| 256 | 289 | } |
tests/perf_baseline.rsmodified@@ -39,7 +39,7 @@ fn executable_opts(inputs: Vec<PathBuf>, output: PathBuf) -> LinkOptions { | ||
| 39 | 39 | |
| 40 | 40 | fn assert_profile_basics(name: &str, profile: &LinkProfile) { |
| 41 | 41 | eprintln!( |
| 42 | - "{name}: total={:?} parse={:?} resolve={:?} atomize={:?} layout={:?} synth={:?} (linkedit={:?}: symbols={:?} dyld={:?} metadata={:?} codesig={:?}; unwind={:?}) reloc={:?} write={:?}", | |
| 42 | + "{name}: total={:?} parse={:?} resolve={:?} atomize={:?} layout={:?} synth={:?} (linkedit={:?}: symbols={:?} [locals={:?} globals={:?} strtab={:?}] dyld={:?} metadata={:?} codesig={:?}; unwind={:?}) reloc={:?} write={:?}", | |
| 43 | 43 | profile.total_wall, |
| 44 | 44 | profile.phases.input_parsing, |
| 45 | 45 | profile.phases.symbol_resolution, |
@@ -48,6 +48,9 @@ fn assert_profile_basics(name: &str, profile: &LinkProfile) { | ||
| 48 | 48 | profile.phases.synth_sections, |
| 49 | 49 | profile.phases.synth_linkedit_finalize, |
| 50 | 50 | profile.phases.synth_linkedit_symbol_plan, |
| 51 | + profile.phases.synth_linkedit_symbol_plan_locals, | |
| 52 | + profile.phases.synth_linkedit_symbol_plan_globals, | |
| 53 | + profile.phases.synth_linkedit_symbol_plan_strtab, | |
| 51 | 54 | profile.phases.synth_linkedit_dyld_info, |
| 52 | 55 | profile.phases.synth_linkedit_metadata_tables, |
| 53 | 56 | profile.phases.synth_linkedit_code_signature, |
@@ -77,6 +80,13 @@ fn assert_profile_basics(name: &str, profile: &LinkProfile) { | ||
| 77 | 80 | + profile.phases.synth_linkedit_code_signature, |
| 78 | 81 | "{name}: linkedit subphases exceeded linkedit total" |
| 79 | 82 | ); |
| 83 | + assert!( | |
| 84 | + profile.phases.synth_linkedit_symbol_plan | |
| 85 | + >= profile.phases.synth_linkedit_symbol_plan_locals | |
| 86 | + + profile.phases.synth_linkedit_symbol_plan_globals | |
| 87 | + + profile.phases.synth_linkedit_symbol_plan_strtab, | |
| 88 | + "{name}: symbol-plan subphases exceeded symbol-plan total" | |
| 89 | + ); | |
| 80 | 90 | } |
| 81 | 91 | |
| 82 | 92 | #[test] |