Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ The generated loop is now written from the program's source alone, and the manif

### Fixed

- **Generated Rust hands a record to functions that tail-call each other without copying it.** The public function of such a group borrowed a record or collection argument and cloned it into the loop that runs the group, so while the caller still held its copy, the first `Map.set` on a Map inside it copied the whole Map, once per call. It now takes the argument by value, and a caller at its last use moves it in; a caller that keeps it clones at the call, as the function did before. An argument every function of the group passes on unchanged is still borrowed. A loop handing a pool with a million-entry Map to such a pair 100 times ran in 0.64 s and now runs in 0.07 s.
- **`perf-shared-update` reports the copies generated code makes, and only those.** The check now reads the same field moves the Rust backend makes, from the module lowered as it compiles. `done = toppedUp(s.pool, s.book, s.nextKey)?` followed by `S.update(s, pool = done.pool, book = done.book, nextKey = done.nextKey)` moves the Book out of `s` and no longer warns. `match (s.window.created, s.height)` with `(created, _) -> f(s, absorbed(created, k))` copies the Map, since `s` still holds it, and now warns: "`absorbed` updates `created`, read from `s.window.created`, a Map that is still held by `s`". A field of a record a loop hands on unchanged is reported too.
- **Generated Rust moves the rest of a nested record into its update.** `Setting.update(setting, window = Window.update(setting.window, created = Map.set(setting.window.created, k, v)), height = setting.height + 1)` used to clone `setting.window` and `setting.window.created`, so every `Map.set` copied the Map. When nothing reads that part of `setting` again, the Map now moves into `Map.set` and the other fields of `setting.window` move into the new `Window`.
- **`check` no longer asks a verify block of a function taking a `Tcp.Socket` or a `Wait.Item`.** Every constructor of either carries a capability resource, so no verify case can write one, but the exemption only looked into the types of the function's own module. It now looks into a capability's own records and sums by the same rule.
Expand Down
5 changes: 4 additions & 1 deletion src/codegen/rust/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,10 @@ pub(super) fn callee_borrow_mask(name: &str, arg_count: usize, ctx: &CodegenCont
.params
.iter()
.take(arg_count)
.map(|(_, ty)| should_borrow_param(ty))
.enumerate()
.map(|(i, (_, ty))| {
should_borrow_param(ty) && !super::from_mir::mutual_param_by_value(ctx, fn_id, i)
})
.collect()
};

Expand Down
77 changes: 70 additions & 7 deletions src/codegen/rust/from_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,7 @@ pub(super) fn compute_owned_record_params(
let Some(program) = ctx.mir_program.as_ref() else {
return owned;
};
owned.extend(mutual_tco_value_params(ctx));
let mut candidates: Vec<(crate::ir::FnId, Vec<usize>)> = Vec::new();
for (id, mir_fn) in program.iter() {
let Some(resolved) = ctx.resolved_program.fn_by_id(*id) else {
Expand Down Expand Up @@ -774,6 +775,59 @@ pub(super) fn compute_owned_record_params(
owned
}

/// Which params of each mutual tail-call member its wrapper takes by value.
///
/// Every arm of the trampoline holds its params by value, so a wrapper that
/// borrows a record or collection has to clone it into the trampoline's
/// state, and while the caller's copy lives every in-place update of a Map
/// in it copies the Map. Taken by value, a caller at its last use moves the
/// value in and nothing is cloned; a caller that keeps it clones at the call
/// instead of in the wrapper, which costs the same. The group's invariants
/// (params every member hands on unchanged) stay borrowed: the trampoline
/// reads them through `&T` for the whole run and never needs its own copy.
fn mutual_tco_value_params(ctx: &CodegenContext) -> HashMap<crate::ir::FnId, Vec<bool>> {
let members: Vec<&crate::ir::hir::ResolvedFnDef> = ctx
.mutual_tco_members
.iter()
.filter_map(|id| ctx.resolved_program.fn_by_id(*id))
.collect();
let mut out = HashMap::new();
for group in crate::call_graph::tailcall_scc_components_resolved(&members) {
let invariants = super::toplevel::compute_resolved_rc_params(&group);
let invariant_names: HashSet<&str> = invariants
.iter()
.filter_map(|&i| group[0].params.get(i).map(|(name, _)| name.as_str()))
.collect();
for fd in &group {
let by_value = fd
.params
.iter()
.map(|(name, ty)| {
should_borrow_param(ty) && !invariant_names.contains(name.as_str())
})
.collect();
out.insert(fd.fn_id, by_value);
}
}
out
}

/// Whether mutual tail-call member `fn_id` takes param `index`, which would
/// otherwise be borrowed, by value (see [`mutual_tco_value_params`]).
pub(super) fn mutual_param_by_value(
ctx: &CodegenContext,
fn_id: crate::ir::FnId,
index: usize,
) -> bool {
ctx.mutual_tco_members.contains(&fn_id)
&& ctx
.rust_owned_record_params
.get(&fn_id)
.and_then(|params| params.get(index))
.copied()
.unwrap_or(false)
}

/// What [`consumes_local`] reads besides the expression: the by-value
/// callee positions so far, the body's movable projections, the Map/Vector
/// params updated in place and the builtin names.
Expand Down Expand Up @@ -2350,7 +2404,8 @@ fn adapt_first_class_fn_ref(name: &str, static_ref: String, ctx: &MirEmitCtx<'_>
let borrow_mask: Vec<bool> = resolved
.params
.iter()
.map(|(_, ty)| should_borrow_param(ty))
.enumerate()
.map(|(i, (_, ty))| should_borrow_param(ty) && !mutual_param_by_value(cg, fn_id, i))
.collect();
if !borrow_mask.iter().any(|borrowed| *borrowed) {
return static_ref;
Expand Down Expand Up @@ -4473,14 +4528,18 @@ pub(super) fn emit_mir_mutual_tco_block(
for fd in group_fns {
let fn_name = aver_name_to_rust(&fd.name);
let variant = fn_name_to_variant(&fd.name);
let params = emit_resolved_fn_params(&fd.params, ctx, scope);
let by_value: Vec<bool> = (0..fd.params.len())
.map(|i| mutual_param_by_value(ctx, fd.fn_id, i))
.collect();
let params = emit_resolved_fn_params(&fd.params, &by_value, ctx, scope);
let variant_arg_names: Vec<String> = fd
.params
.iter()
.filter(|(name, _)| !rc_names.contains(name))
.map(|(name, ty)| {
.enumerate()
.filter(|(_, (name, _))| !rc_names.contains(name))
.map(|(i, (name, ty))| {
let rust_name = aver_name_to_rust(name);
if should_borrow_param(ty) {
if should_borrow_param(ty) && !by_value[i] {
format!("{}.clone()", rust_name)
} else {
rust_name
Expand Down Expand Up @@ -4559,16 +4618,20 @@ fn mutual_rc_param_sig(
}
}

/// A mutual tail-call wrapper's params: borrowed by default, by value where
/// `by_value` says so.
fn emit_resolved_fn_params(
params: &[(String, crate::types::Type)],
by_value: &[bool],
ctx: &CodegenContext,
scope: Option<&str>,
) -> String {
params
.iter()
.map(|(name, ty)| {
.enumerate()
.map(|(i, (name, ty))| {
let rust_type = super::types::type_to_rust_scoped(ty, ctx, scope);
if should_borrow_param(ty) {
if should_borrow_param(ty) && !by_value.get(i).copied().unwrap_or(false) {
format!("{}: &{rust_type}", explicit_parameter_pattern(name, false))
} else {
format!("{}: {rust_type}", explicit_parameter_pattern(name, false))
Expand Down
20 changes: 8 additions & 12 deletions src/self_host/aver_generated/domain/builtins/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,19 @@ fn __mutual_tco_trampoline_1(

/// Convert list of (key, value) tuples to a Map.
pub fn tuplesToMap(
items @ _: &aver_rt::AverList<crate::aver_generated::domain::value::Val>,
acc @ _: &aver_rt::AverMap<AverStr, crate::aver_generated::domain::value::Val>,
items @ _: aver_rt::AverList<crate::aver_generated::domain::value::Val>,
acc @ _: aver_rt::AverMap<AverStr, crate::aver_generated::domain::value::Val>,
) -> aver_rt::AverMap<AverStr, crate::aver_generated::domain::value::Val> {
__mutual_tco_trampoline_1(__MutualTco1::TuplesToMap(items.clone(), acc.clone()))
__mutual_tco_trampoline_1(__MutualTco1::TuplesToMap(items, acc))
}

/// Extract key-value from tuple parts.
pub fn tuplesToMapOne(
parts @ _: &aver_rt::AverList<crate::aver_generated::domain::value::Val>,
rest @ _: &aver_rt::AverList<crate::aver_generated::domain::value::Val>,
acc @ _: &aver_rt::AverMap<AverStr, crate::aver_generated::domain::value::Val>,
parts @ _: aver_rt::AverList<crate::aver_generated::domain::value::Val>,
rest @ _: aver_rt::AverList<crate::aver_generated::domain::value::Val>,
acc @ _: aver_rt::AverMap<AverStr, crate::aver_generated::domain::value::Val>,
) -> aver_rt::AverMap<AverStr, crate::aver_generated::domain::value::Val> {
__mutual_tco_trampoline_1(__MutualTco1::TuplesToMapOne(
parts.clone(),
rest.clone(),
acc.clone(),
))
__mutual_tco_trampoline_1(__MutualTco1::TuplesToMapOne(parts, rest, acc))
}

/// Dispatch qualified builtin calls to sub-module implementations.
Expand Down Expand Up @@ -604,7 +600,7 @@ pub fn builtinMapFromList(
let v @ _ = crate::aver_generated::domain::builtins::helpers::oneArg(args)?;
let items @ _ = crate::aver_generated::domain::builtins::helpers::expectList(&v)?;
Ok(crate::aver_generated::domain::value::Val::ValMap(
crate::aver_generated::domain::builtins::tuplesToMap(&items, &HashMap::new()),
crate::aver_generated::domain::builtins::tuplesToMap(items, HashMap::new()),
))
}

Expand Down
Loading
Loading