Skip to content

Commit

Permalink
Reduce code bloat from generics monomorphization
Browse files Browse the repository at this point in the history 
For large functions with generic parameters that are called with
many different generic parameters, move as much code as possible within those
functions into separate, non-generic functions. This ensures that when
Rust monomorphizes those functions, only the portions of code which
actually need to be duplicated are duplicated. The approach here
is similar to that described in
https://www.possiblerust.com/pattern/non-generic-inner-functions,
except that it is applied to methods rather than functions.

This change reduces code size by 4016 bytes, most of which comes from
the changes to Grant::enter().

A corresponding PR has been sent to upstream Tock:
tock#2648 . That PR is for the Tock 2.0
version of grant.rs, and thus makes some different decisions based on
the updated grant design.

BUG=None,

TEST=make build;

Change-Id: I5fc0a360c9a37852746ebb32e83de2a03dc1e957
Reviewed-on: https://chrome-internal-review.googlesource.com/c/ti50/third_party/tock/tock/+/3947379
Reviewed-by: Andrey Pronin <[email protected]>
Reviewed-by: Jett Rink <[email protected]>
Commit-Queue: Hudson Ayers <[email protected]>
Tested-by: Hudson Ayers <[email protected]>
  • Loading branch information
@hudson-ayers
hudson-ayers authored and Commit Bot committed Jul 2, 2021
1 parent 1e299d0 commit 7c96a73
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 128 deletions.
238 changes: 142 additions & 96 deletions kernel/src/grant.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,27 +82,35 @@ impl Allocator {
}
}

// Like `alloc`, but the caller is responsible for free-ing the allocated
// memory, as it is not wrapped in a type that implements `Drop`
unsafe fn alloc_unowned<T: Default>(&mut self) -> Result<NonNull<T>, Error> {
// Move non-generic code into a function with no generic parameters to
// prevent monomorphization of this code. Saves ~200 bytes on Ti50.
#[inline(never)]
unsafe fn alloc_unowned_inner(
&mut self,
size: usize,
align: usize,
) -> Result<NonNull<u8>, Error> {
self.appid
.kernel
.process_map_or(Err(Error::NoSuchApp), self.appid, |process| {
process.alloc(size_of::<T>(), align_of::<T>()).map_or(
Err(Error::OutOfMemory),
|buf| {
// Convert untyped `*mut u8` allocation to allocated type
let ptr = NonNull::cast::<T>(buf);

// We use `ptr::write` to avoid `Drop`ping the uninitialized memory in
// case `T` implements the `Drop` trait.
write(ptr.as_ptr(), T::default());

Ok(ptr)
},
)
process
.alloc(size, align)
.map_or(Err(Error::OutOfMemory), |buf| Ok(buf))
})
}

// Like `alloc`, but the caller is responsible for free-ing the allocated
// memory, as it is not wrapped in a type that implements `Drop`
unsafe fn alloc_unowned<T: Default>(&mut self) -> Result<NonNull<T>, Error> {
let buf = self.alloc_unowned_inner(size_of::<T>(), align_of::<T>())?;
let ptr = NonNull::cast::<T>(buf);

// We use `ptr::write` to avoid `Drop`ping the uninitialized memory in
// case `T` implements the `Drop` trait.
write(ptr.as_ptr(), T::default());

Ok(ptr)
}
}

pub struct Borrowed<'a, T: 'a + ?Sized> {
Expand Down Expand Up @@ -149,62 +157,46 @@ impl<T: Default> Grant<T> {
}
}

pub fn grant(&self, appid: AppId) -> Option<AppliedGrant<T>> {
// Move non-generic code from grant() into non-generic function.
// Saves 16 bytes on Ti50 as of 06/30/21
#[inline(never)]
fn grant_inner(&self, appid: AppId) -> Option<NonNull<u8>> {
appid.kernel.process_map_or(None, appid, |process| {
if let Some(grant_ptr) = process.get_grant_ptr(self.grant_num) {
NonNull::new(grant_ptr).map(|grant| AppliedGrant {
appid: appid,
grant: grant.cast::<T>(),
_phantom: PhantomData,
})
NonNull::new(grant_ptr)
} else {
None
}
})
}

pub fn enter<F, R>(&self, appid: AppId, fun: F) -> Result<R, Error>
where
F: FnOnce(&mut Borrowed<T>) -> R,
R: Copy,
{
pub fn grant(&self, appid: AppId) -> Option<AppliedGrant<T>> {
match self.grant_inner(appid) {
Some(grant) => Some(AppliedGrant {
appid: appid,
grant: grant.cast::<T>(),
_phantom: PhantomData,
}),
None => None,
}
}

// Moves non-generic code from enter() into non-generic function to reduce code bloat,
// as it is common to have over 50 copies of Grant::enter() in a Tock kernel.
// This saves over 3kB.
// The returned bool indicates if allocation happened -- if allocation happened,
// the generic portion of enter() needs to write the default value into the allocated
// space.
#[inline(never)]
fn enter_inner(
&self,
appid: AppId,
size: usize,
align: usize,
) -> Result<(*mut u8, bool), Error> {
appid
.kernel
.process_map_or(Err(Error::NoSuchApp), appid, |process| {
// Here is an example of how the grants are laid out in a
// process's memory:
//
// Mem. Addr.
// 0x0040000 ┌────────────────────
// │ GrantPointer0 [0x003FFC8]
// │ GrantPointer1 [0x003FFC0]
// │ ...
// │ GrantPointerN [0x0000000 (NULL)]
// ├────────────────────
// │ Process Control Block
// 0x003FFE0 ├────────────────────
// │ GrantRegion0
// 0x003FFC8 ├────────────────────
// │ GrantRegion1
// 0x003FFC0 ├────────────────────
// │
// │ --unallocated--
// │
// └────────────────────
//
// An array of pointers (one per possible grant region)
// point to where the actual grant memory is allocated
// inside of the process. The grant memory is not allocated
// until the actual grant region is actually used.
//
// This function provides the app access to the specific
// grant memory, and allocates the grant region in the
// process memory if needed.

// Get the GrantPointer to start. Since process.rs does not know
// anything about the datatype of the grant, and the grant
// memory may not yet be allocated, it can only return a `*mut
// u8` here. We will eventually convert this to a `*mut T`.
if let Some(untyped_grant_ptr) = process.get_grant_ptr(self.grant_num) {
// This is the allocator for this process when needed
let mut allocator = Allocator { appid: appid };
Expand All @@ -213,15 +205,15 @@ impl<T: Default> Grant<T> {
// GrantRegion needs to be allocated. Otherwise, we can
// convert the pointer to a `*mut T` because we know we
// previously allocated enough memory for type T.
let typed_grant_pointer = if untyped_grant_ptr.is_null() {
let ret = if untyped_grant_ptr.is_null() {
unsafe {
// Allocate space in the process's memory for
// something of type `T` for the grant.
//
// Note: This allocation is intentionally never
// freed. A grant region is valid once allocated
// for the lifetime of the process.
let new_region = allocator.alloc_unowned()?;
let new_region = allocator.alloc_unowned_inner(size, align)?;

// Update the grant pointer in the process. Again,
// since the process struct does not know about the
Expand All @@ -230,31 +222,78 @@ impl<T: Default> Grant<T> {

// The allocator returns a `NonNull`, we just want
// the raw pointer.
new_region.as_ptr()
(new_region.as_ptr(), true)
}
} else {
// Grant region previously allocated, just convert the
// pointer.
untyped_grant_ptr as *mut T
(untyped_grant_ptr, false)
};

// Dereference the typed GrantPointer to make a GrantRegion
// reference.
let region = unsafe { &mut *typed_grant_pointer };

// Wrap the grant reference in something that knows
// what app its a part of.
let mut borrowed_region = Borrowed::new(region, appid);

// Call the passed in closure with the borrowed grant region.
let res = fun(&mut borrowed_region);
Ok(res)
Ok(ret)
} else {
Err(Error::InactiveApp)
}
})
}

pub fn enter<F, R>(&self, appid: AppId, fun: F) -> Result<R, Error>
where
F: FnOnce(&mut Borrowed<T>) -> R,
R: Copy,
{
// Here is an example of how the grants are laid out in a
// process's memory:
//
// Mem. Addr.
// 0x0040000 ┌────────────────────
// │ GrantPointer0 [0x003FFC8]
// │ GrantPointer1 [0x003FFC0]
// │ ...
// │ GrantPointerN [0x0000000 (NULL)]
// ├────────────────────
// │ Process Control Block
// 0x003FFE0 ├────────────────────
// │ GrantRegion0
// 0x003FFC8 ├────────────────────
// │ GrantRegion1
// 0x003FFC0 ├────────────────────
// │
// │ --unallocated--
// │
// └────────────────────
//
// An array of pointers (one per possible grant region)
// point to where the actual grant memory is allocated
// inside of the process. The grant memory is not allocated
// until the actual grant region is actually used.
//
// This function provides the app access to the specific
// grant memory, and allocates the grant region in the
// process memory if needed.

// Get the GrantPointer to start. Since process.rs does not know
// anything about the datatype of the grant, and the grant
// memory may not yet be allocated, it can only return a `*mut
// u8` here. We then convert this to a `*mut T`.

let (still_untyped_grant_pointer, need_to_init) =
self.enter_inner(appid, size_of::<T>(), align_of::<T>())?;
let typed_grant_pointer = still_untyped_grant_pointer as *mut T;
if need_to_init {
unsafe { write(typed_grant_pointer, T::default()) }
}

// Dereference the typed GrantPointer to make a GrantRegion
// reference.
let region = unsafe { &mut *typed_grant_pointer };

// Wrap the grant reference in something that knows
// what app its a part of.
let mut borrowed_region = Borrowed::new(region, appid);

// Call the passed in closure with the borrowed grant region.
let res = fun(&mut borrowed_region);
Ok(res)
}

pub fn each<F>(&self, fun: F)
where
F: Fn(&mut Owned<T>),
Expand Down Expand Up @@ -286,33 +325,40 @@ pub struct Iter<'a, T: 'a + Default> {
fn(&Option<&'static dyn ProcessType>) -> Option<&'static dyn ProcessType>,
>,
}
impl<T: Default> Iter<'_, T> {
#[inline(never)]
fn next_internal(&mut self, grant_num: usize) -> Option<AppId> {
// Get the next `AppId` from the kernel processes array that is setup to use this grant.
// Since the iterator itself is saved calling this function
// again will start where we left off.
self.subiter
.find(|process| {
// We have found a candidate process that exists in the
// processes array. Now we have to check if this grant is setup
// for this process. If not, we have to skip it and keep
// looking.
if let Some(grant_ptr) = process.get_grant_ptr(grant_num) {
!grant_ptr.is_null()
} else {
false
}
})
.map(|process| process.appid())
}
}

impl<T: Default> Iterator for Iter<'_, T> {
type Item = AppliedGrant<T>;

fn next(&mut self) -> Option<Self::Item> {
// Save a local copy of grant_num so we don't have to access `self`
// in the closure below.
// in the closure in next_internal
let grant_num = self.grant.grant_num;

// Get the next `AppId` from the kernel processes array that is setup to use this grant.
// Since the iterator itself is saved calling this function
// again will start where we left off.
let res = self.subiter.find(|process| {
// We have found a candidate process that exists in the
// processes array. Now we have to check if this grant is setup
// for this process. If not, we have to skip it and keep
// looking.
if let Some(grant_ptr) = process.get_grant_ptr(grant_num) {
!grant_ptr.is_null()
} else {
false
}
});

let res = self.next_internal(grant_num);
// Check if our find above returned another `AppId`, or if we hit the
// end of the iterator. If we found another app, try to access its grant
// region.
res.map_or(None, |process| self.grant.grant(process.appid()))
res.map_or(None, |appid| self.grant.grant(appid))
}
}
59 changes: 27 additions & 32 deletions kernel/src/sched.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,28 @@ impl Kernel {
self.work.get() == 0
}

/// Helper function that moves all non-generic portions of process_map_or
/// into a non-generic function to reduce code bloat from monomorphization.
#[inline(never)]
fn get_process(&self, processid: AppId) -> Option<&dyn process::ProcessType> {
// We use the index in the `appid` so we can do a direct lookup.
// However, we are not guaranteed that the app still exists at that
// index in the processes array. To avoid additional overhead, we do the
// lookup and check here, rather than calling `.index()`.
match self.processes.get(processid.index) {
Some(Some(process)) => {
// Check that the process stored here matches the identifier
// in the `appid`.
if process.appid() == processid {
Some(*process)
} else {
None
}
}
_ => None,
}
}

/// Run a closure on a specific process if it exists. If the process with a
/// matching `AppId` does not exist at the index specified within the
/// `AppId`, then `default` will be returned.
Expand All @@ -118,30 +140,10 @@ impl Kernel {
where
F: FnOnce(&dyn process::ProcessType) -> R,
{
// We use the index in the `appid` so we can do a direct lookup.
// However, we are not guaranteed that the app still exists at that
// index in the processes array. To avoid additional overhead, we do the
// lookup and check here, rather than calling `.index()`.
let tentative_index = appid.index;

// Get the process at that index, and if it matches, run the closure
// on it.
self.processes
.get(tentative_index)
.map_or(None, |process_entry| {
// Check if there is any process state here, or if the entry is
// `None`.
process_entry.map_or(None, |process| {
// Check that the process stored here matches the identifier
// in the `appid`.
if process.appid() == appid {
Some(closure(process))
} else {
None
}
})
})
.unwrap_or(default)
match self.get_process(appid) {
Some(process) => closure(process),
None => default,
}
}

/// Run a closure on every valid process. This will iterate the array of
Expand Down Expand Up @@ -188,14 +190,7 @@ impl Kernel {
) where
F: Fn(&dyn process::ProcessType),
{
for process in self.processes.iter() {
match process {
Some(p) => {
closure(*p);
}
None => {}
}
}
self.process_each(closure)
}

/// Run a closure on every process, but only continue if the closure returns
Expand Down

0 comments on commit 7c96a73

Please sign in to comment.