Skip to content

Commit 684fb37

Browse files
committed
Auto merge of #55871 - ljedrz:llvm_back_allocations, r=<try>
codegen_llvm_back: improve allocations This commit was split out from #54864. Last time it was causing an LLVM OOM, presumably due to aggressive preallocation strategy in `thin_lto`. This time preallocations are more cautious and there are a few additional memory-related improvements (last 3 points from the list below). - _gently_ preallocate vectors of known length - `extend` instead of `append` where the argument is consumable - turn 2 `push` loops into `extend`s - create a vector from a function producing one instead of using `extend_from_slice` on it - consume `modules` when no longer needed - return an `impl Iterator` from `generate_lto_work` - don't `collect` `globals`, as they are iterated over and consumed right afterwards While I'm hoping it won't cause an OOM anymore, I would still consider this a "high-risk" PR and not roll it up.
2 parents b76ee83 + dc1b2c7 commit 684fb37

File tree

5 files changed

+33
-27
lines changed

5 files changed

+33
-27
lines changed

src/librustc_codegen_llvm/back/link.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -134,12 +134,12 @@ pub(crate) fn link_binary(sess: &Session,
134134
bug!("invalid output type `{:?}` for target os `{}`",
135135
crate_type, sess.opts.target_triple);
136136
}
137-
let mut out_files = link_binary_output(sess,
138-
codegen_results,
139-
crate_type,
140-
outputs,
141-
crate_name);
142-
out_filenames.append(&mut out_files);
137+
let out_files = link_binary_output(sess,
138+
codegen_results,
139+
crate_type,
140+
outputs,
141+
crate_name);
142+
out_filenames.extend(out_files);
143143
}
144144

145145
// Remove the temporary object file and metadata if we aren't saving temps

src/librustc_codegen_llvm/back/lto.rs

+16-10
Original file line numberDiff line numberDiff line change
@@ -279,17 +279,19 @@ fn fat_lto(cgcx: &CodegenContext,
279279
// and we want to move everything to the same LLVM context. Currently the
280280
// way we know of to do that is to serialize them to a string and them parse
281281
// them later. Not great but hey, that's why it's "fat" LTO, right?
282-
for module in modules {
282+
serialized_modules.extend(modules.into_iter().map(|module| {
283283
let buffer = ModuleBuffer::new(module.module_llvm.llmod());
284284
let llmod_id = CString::new(&module.name[..]).unwrap();
285-
serialized_modules.push((SerializedModule::Local(buffer), llmod_id));
286-
}
285+
286+
(SerializedModule::Local(buffer), llmod_id)
287+
}));
287288

288289
// For all serialized bitcode files we parse them and link them in as we did
289290
// above, this is all mostly handled in C++. Like above, though, we don't
290291
// know much about the memory management here so we err on the side of being
291292
// save and persist everything with the original module.
292293
let mut linker = Linker::new(llmod);
294+
serialized_bitcode.reserve(serialized_modules.len());
293295
for (bc_decoded, name) in serialized_modules {
294296
info!("linking {:?}", name);
295297
time_ext(cgcx.time_passes, None, &format!("ll link {:?}", name), || {
@@ -403,9 +405,10 @@ fn thin_lto(cgcx: &CodegenContext,
403405
.map(|&(_, ref wp)| (wp.cgu_name.clone(), wp.clone()))
404406
.collect();
405407

406-
let mut thin_buffers = Vec::new();
407-
let mut module_names = Vec::new();
408-
let mut thin_modules = Vec::new();
408+
// Reserve memory only partially in order to avoid OOM
409+
let mut thin_buffers = Vec::with_capacity(modules.len());
410+
let mut module_names = Vec::with_capacity(modules.len());
411+
let mut thin_modules = Vec::with_capacity(modules.len());
409412

410413
// FIXME: right now, like with fat LTO, we serialize all in-memory
411414
// modules before working with them and ThinLTO. We really
@@ -414,7 +417,7 @@ fn thin_lto(cgcx: &CodegenContext,
414417
// into the global index. It turns out that this loop is by far
415418
// the most expensive portion of this small bit of global
416419
// analysis!
417-
for (i, module) in modules.iter().enumerate() {
420+
for (i, module) in modules.into_iter().enumerate() {
418421
info!("local module: {} - {}", i, module.name);
419422
let name = CString::new(module.name.clone()).unwrap();
420423
let buffer = ThinBuffer::new(module.module_llvm.llmod());
@@ -460,12 +463,15 @@ fn thin_lto(cgcx: &CodegenContext,
460463
// incremental ThinLTO first where we could actually avoid
461464
// looking at upstream modules entirely sometimes (the contents,
462465
// we must always unconditionally look at the index).
463-
let mut serialized = Vec::new();
464-
465466
let cached_modules = cached_modules.into_iter().map(|(sm, wp)| {
466467
(sm, CString::new(wp.cgu_name).unwrap())
467468
});
468469

470+
let upstream_cached_len = serialized_modules.len() + cached_modules.len();
471+
let mut serialized = Vec::with_capacity(upstream_cached_len);
472+
thin_modules.reserve(upstream_cached_len);
473+
module_names.reserve(upstream_cached_len);
474+
469475
for (module, name) in serialized_modules.into_iter().chain(cached_modules) {
470476
info!("upstream or cached module {:?}", name);
471477
thin_modules.push(llvm::ThinLTOModule {
@@ -521,7 +527,7 @@ fn thin_lto(cgcx: &CodegenContext,
521527
});
522528

523529
let mut copy_jobs = vec![];
524-
let mut opt_jobs = vec![];
530+
let mut opt_jobs = Vec::with_capacity(shared.module_names.len());
525531

526532
info!("checking which modules can be-reused and which have to be re-optimized.");
527533
for (module_index, module_name) in shared.module_names.iter().enumerate() {

src/librustc_codegen_llvm/back/rpath.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,12 @@ pub fn get_rpath_flags(config: &mut RPathConfig) -> Vec<String> {
3131
return Vec::new();
3232
}
3333

34-
let mut flags = Vec::new();
35-
3634
debug!("preparing the RPATH!");
3735

3836
let libs = config.used_crates.clone();
3937
let libs = libs.iter().filter_map(|&(_, ref l)| l.option()).collect::<Vec<_>>();
4038
let rpaths = get_rpaths(config, &libs);
41-
flags.extend_from_slice(&rpaths_to_flags(&rpaths));
39+
let mut flags = rpaths_to_flags(&rpaths);
4240

4341
// Use DT_RUNPATH instead of DT_RPATH if available
4442
if config.linker_is_gnu {
@@ -49,7 +47,8 @@ pub fn get_rpath_flags(config: &mut RPathConfig) -> Vec<String> {
4947
}
5048

5149
fn rpaths_to_flags(rpaths: &[String]) -> Vec<String> {
52-
let mut ret = Vec::new();
50+
let mut ret = Vec::with_capacity(rpaths.len()); // the minimum needed capacity
51+
5352
for rpath in rpaths {
5453
if rpath.contains(',') {
5554
ret.push("-Wl,-rpath".into());

src/librustc_codegen_llvm/back/write.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ unsafe fn optimize(cgcx: &CodegenContext,
648648
fn generate_lto_work(cgcx: &CodegenContext,
649649
modules: Vec<ModuleCodegen>,
650650
import_only_modules: Vec<(SerializedModule, WorkProduct)>)
651-
-> Vec<(WorkItem, u64)>
651+
-> impl Iterator<Item = (WorkItem, u64)>
652652
{
653653
let mut timeline = cgcx.time_graph.as_ref().map(|tg| {
654654
tg.start(CODEGEN_WORKER_TIMELINE,
@@ -670,7 +670,7 @@ fn generate_lto_work(cgcx: &CodegenContext,
670670
}), 0)
671671
});
672672

673-
lto_modules.chain(copy_jobs).collect()
673+
lto_modules.chain(copy_jobs)
674674
}
675675

676676
unsafe fn codegen(cgcx: &CodegenContext,
@@ -2586,8 +2586,8 @@ fn create_msvc_imps(cgcx: &CodegenContext, llcx: &llvm::Context, llmod: &llvm::M
25862586
imp_name.extend(name.to_bytes());
25872587
let imp_name = CString::new(imp_name).unwrap();
25882588
(imp_name, val)
2589-
})
2590-
.collect::<Vec<_>>();
2589+
});
2590+
25912591
for (imp_name, val) in globals {
25922592
let imp = llvm::LLVMAddGlobal(llmod,
25932593
i8p_ty,

src/librustc_codegen_utils/symbol_export.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,11 @@ fn exported_symbols_provider_local<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
229229
"__llvm_profile_raw_version",
230230
"__llvm_profile_filename",
231231
];
232-
for sym in &PROFILER_WEAK_SYMBOLS {
232+
233+
symbols.extend(PROFILER_WEAK_SYMBOLS.iter().map(|sym| {
233234
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(sym));
234-
symbols.push((exported_symbol, SymbolExportLevel::C));
235-
}
235+
(exported_symbol, SymbolExportLevel::C)
236+
}));
236237
}
237238

238239
if tcx.sess.crate_types.borrow().contains(&config::CrateType::Dylib) {

0 commit comments

Comments
 (0)