From b03ce11e256d58b1f9e6b4bbff4fcc2789ffa255 Mon Sep 17 00:00:00 2001 From: Alessandro Decina Date: Mon, 6 May 2024 08:34:46 +0000 Subject: [PATCH] AccountSharedData::set_data_from_slice: skip extra alloc + memcpy We used call make_data_mut() from set_data_from_slice() from the days when direct mapping couldn't deal with accounts getting shrunk. That changed in https://github.com/solana-labs/solana/pull/32649 (see the if callee_account.capacity() < min_capacity check in cpi.rs:update_caller_account()). With this change we don't call make_data_mut() anymore before set_data_from_slice(), saving the cost of a memcpy since set_data_from_slice() overrides the whole account content anyway. --- programs/bpf_loader/src/syscalls/cpi.rs | 5 +++-- sdk/src/transaction_context.rs | 11 ++++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs index 4c8a0e45e41b53..22eb2f97bf3f8d 100644 --- a/programs/bpf_loader/src/syscalls/cpi.rs +++ b/programs/bpf_loader/src/syscalls/cpi.rs @@ -1308,8 +1308,9 @@ fn update_caller_account( // never points to an invalid address. // // Note that the capacity can be smaller than the original length only if the account is - // reallocated using the AccountSharedData API directly (deprecated). BorrowedAccount - // and CoW don't trigger this, see BorrowedAccount::make_data_mut. + // reallocated using the AccountSharedData API directly (deprecated) or using + // BorrowedAccount::set_data_from_slice(), which implements an optimization to avoid an + // extra allocation. let min_capacity = caller_account.original_data_len; if callee_account.capacity() < min_capacity { callee_account diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index 7df7fc96d67933..8ec6902415a73c 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -869,13 +869,10 @@ impl<'a> BorrowedAccount<'a> { self.can_data_be_changed()?; self.touch()?; self.update_accounts_resize_delta(data.len())?; - // Calling make_data_mut() here guarantees that set_data_from_slice() - // copies in places, extending the account capacity if necessary but - // never reducing it. This is required as the account migh be directly - // mapped into a MemoryRegion, and therefore reducing capacity would - // leave a hole in the vm address space. After CPI or upon program - // termination, the runtime will zero the extra capacity. - self.make_data_mut(); + // Note that we intentionally don't call self.make_data_mut() here. make_data_mut() will + // allocate + memcpy the current data if self.account is shared. We don't need the memcpy + // here tho because account.set_data_from_slice(data) is going to replace the content + // anyway. self.account.set_data_from_slice(data); Ok(())