Skip to content

Commit

Permalink
Reduce deserialization allocations/copies (solana-labs#1197)
Browse files Browse the repository at this point in the history
* AccountSharedData::reserve: remove extra alloc + memcpy

Calling data_mut().reserve(additional) used to result in _two_ allocs
and memcpys: the first to unshare the underlying vector, and the second
upon calling reserve since Arc::make_mut clones so it uses capacity ==
len.

With this fix we manually "unshare" allocating with capacity = len +
additional, therefore saving on extra alloc and memcpy.

* 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 solana-labs#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.
  • Loading branch information
alessandrod authored and CriesofCarrots committed May 7, 2024
1 parent 7aac03f commit 629470f
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 10 deletions.
5 changes: 3 additions & 2 deletions programs/bpf_loader/src/syscalls/cpi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion sdk/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,13 @@ impl AccountSharedData {
}

pub fn reserve(&mut self, additional: usize) {
self.data_mut().reserve(additional)
if let Some(data) = Arc::get_mut(&mut self.data) {
data.reserve(additional)
} else {
let mut data = Vec::with_capacity(self.data.len().saturating_add(additional));
data.extend_from_slice(&self.data);
self.data = Arc::new(data);
}
}

pub fn capacity(&self) -> usize {
Expand Down
11 changes: 4 additions & 7 deletions sdk/src/transaction_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down

0 comments on commit 629470f

Please sign in to comment.