From d92de1acd2c02e93b0be014837b324b86767dc00 Mon Sep 17 00:00:00 2001 From: Johannes Hostert Date: Wed, 4 Dec 2024 14:42:53 +0100 Subject: [PATCH] TB: optimize accesses on large trees by ignoring subtrees if the access would mostly be a NOP --- src/borrow_tracker/tree_borrows/perms.rs | 4 +++ src/borrow_tracker/tree_borrows/tree.rs | 26 ++++++++++++++++++- ...tree_traversal_skipping_diagnostics.stderr | 12 ++++----- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/borrow_tracker/tree_borrows/perms.rs b/src/borrow_tracker/tree_borrows/perms.rs index 6e157d3fcd..5d7c3d8c21 100644 --- a/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/borrow_tracker/tree_borrows/perms.rs @@ -237,6 +237,10 @@ impl Permission { pub fn is_active(&self) -> bool { self.inner == Active } + /// Check if `self` is the never-allow-writes-again state of a pointer (is `Frozen`). + pub fn is_frozen(&self) -> bool { + self.inner == Frozen + } /// Default initial permission of the root of a new tree at inbounds positions. /// Must *only* be used for the root, this is not in general an "initial" permission! diff --git a/src/borrow_tracker/tree_borrows/tree.rs b/src/borrow_tracker/tree_borrows/tree.rs index 6d4ec36f7b..844a4fdbdb 100644 --- a/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/borrow_tracker/tree_borrows/tree.rs @@ -153,8 +153,32 @@ impl LocationState { ) -> ContinueTraversal { if rel_pos.is_foreign() { let happening_now = IdempotentForeignAccess::from_foreign(access_kind); - let new_access_noop = + let mut new_access_noop = self.idempotent_foreign_access.can_skip_foreign_access(happening_now); + if self.permission.is_disabled() { + // A foreign access to a `Disabled` tag will have almost no observable effect. + // It's a theorem that `Disabled` node have no protected initialized children, + // and so this foreign access will never trigger any protector. + // Further, the children will never be able to read or write again, since they + // have a `Disabled` parents. Even further, all children of `Disabled` are one + // of `ReservedIM`, `Disabled`, or a not-yet-accessed "lazy" permission thing. + // The two former are already invariant under all foreign accesses, and for + // the latter it does not really matter, since they can not be used/initialized + // due to having a protected parent. So this only affects diagnostics, but the + // blocking write will still be identified directly, just at a different tag. + new_access_noop = true; + } + if self.permission.is_frozen() && access_kind == AccessKind::Read { + // A foreign read to a `Frozen` tag will have almost no observable effect. + // It's a theorem that `Frozen` nodes have no active children, so all children + // already survive foreign reads. Foreign reads in general have almost no + // effect, the only further thing they could do is make protected `Reserved` + // nodes become conflicted, i.e. make them reject child writes for the further + // duration of their protector. But such a child write is already rejected + // because this node is frozen. So this only affects diagnostics, but the + // blocking read will still be identified directly, just at a different tag. + new_access_noop = true; + } if new_access_noop { // Abort traversal if the new access is indeed guaranteed // to be noop. diff --git a/tests/fail/tree_borrows/subtree_traversal_skipping_diagnostics.stderr b/tests/fail/tree_borrows/subtree_traversal_skipping_diagnostics.stderr index 4968047d87..d3ad2a39f2 100644 --- a/tests/fail/tree_borrows/subtree_traversal_skipping_diagnostics.stderr +++ b/tests/fail/tree_borrows/subtree_traversal_skipping_diagnostics.stderr @@ -5,18 +5,18 @@ LL | *m = 42; | ^^^^^^^ write access through at ALLOC[0x0] is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag has state Reserved (conflicted) which forbids this child write access -help: the accessed tag was created here, in the initial state Reserved + = help: the accessed tag is a child of the conflicting tag + = help: the conflicting tag has state Frozen which forbids this child write access +help: the accessed tag was created here --> tests/fail/tree_borrows/subtree_traversal_skipping_diagnostics.rs:LL:CC | LL | fn write_to_mut(m: &mut u8, other_ptr: *const u8) { | ^ -help: the accessed tag later transitioned to Reserved (conflicted) due to a foreign read access at offsets [0x0..0x1] +help: the conflicting tag was created here, in the initial state Frozen --> tests/fail/tree_borrows/subtree_traversal_skipping_diagnostics.rs:LL:CC | -LL | std::hint::black_box(*other_ptr); - | ^^^^^^^^^^ - = help: this transition corresponds to a temporary loss of write permissions until function exit +LL | let intermediary = &root; + | ^^^^^ = note: BACKTRACE (of the first span): = note: inside `write_to_mut` at tests/fail/tree_borrows/subtree_traversal_skipping_diagnostics.rs:LL:CC note: inside `main`