MapCell (#271)

* Sketch out MapCell type

Starts to address Issue #178 where TakeCell actually moves data out of
the option. This is a performance problem if the data in a TakeCell is
large, but saves both cycles and memory if the data is small (like a
pointer or slice) because the Option can be optimized in many cases and
the code operates on registers as opposed to memory.

To resolve this, just introducing a new Cell type called MapCell which
keeps the data in the MapCell during a `map` operation. Otherwise the
interface is the same so should be a dropin replacement.

* Benchmark for TakeCell vs MapCell

Utility test in the `hail` board that measures memory overhead of
TakeCell and MapCell, as well as cycles for a no-op map operation on
data of increasing sizes:

| Type         |Cycles | Bytes|
|--------------|-------|------|
|`&u32`        | 3     | 4    |
|`[[u8;256];1]`| 1655  | 257  |
|`[[u8;256];2]`| 2795  | 513  |
|`[[u8;256];3]`| 4365  | 769  |
|`[[u8;256];4]`| 4838  | 1025 |
|`[[u8;256];5]`| 6780  | 1281 |
|`[[u8;256];6]`| 7706  | 1537 |
|`[[u8;256];7]`| 10326 | 1793 |

| Type         | Cycles | Bytes|
|--------------|--------|------|
|`&u32`        | 18     | 8    |
|`[[u8;256];1]`| 20     | 257  |
|`[[u8;256];2]`| 22     | 513  |
|`[[u8;256];3]`| 20     | 769  |
|`[[u8;256];4]`| 22     | 1025 |
|`[[u8;256];5]`| 22     | 1281 |
|`[[u8;256];6]`| 22     | 1537 |
|`[[u8;256];7]`| 20     | 1793 |

* Convert TakeCell to MapCell/Cell if appropriate

Changes TakeCell to only admit mutable references. Other types should
simply use Cell or MapCell.

* docs

* rustfmt

* rustfmt

boards/hail/src/main.rs

@@ -23,6 +23,8 @@ use sam4l::usart;
 
 #[macro_use]
 pub mod io;
+#[allow(dead_code)]
+mod test_take_map_cell;
 
 static mut SPI_READ_BUF: [u8; 64] = [0; 64];
 static mut SPI_WRITE_BUF: [u8; 64] = [0; 64];
@@ -275,7 +277,7 @@ pub unsafe fn reset_handler() {
     let syscall_spi_device = static_init!(
         VirtualSpiMasterDevice<'static, sam4l::spi::Spi>,
         VirtualSpiMasterDevice::new(mux_spi, 0),
-        384/8);
+        352/8);
 
     // Create the SPI systemc call capsule, passing the client
     let spi_syscalls = static_init!(
@@ -343,6 +345,8 @@ pub unsafe fn reset_handler() {
         pin.set_client(gpio);
     }
 
+
+
     let hail = Hail {
         console: console,
         gpio: gpio,
@@ -371,5 +375,8 @@ pub unsafe fn reset_handler() {
     let mut chip = sam4l::chip::Sam4l::new();
     chip.mpu().enable_mpu();
 
+    // Uncomment to measure overheads for TakeCell and MapCell:
+    // test_take_map_cell::test_take_map_cell();
+
     kernel::main(&hail, &mut chip, load_processes(), &hail.ipc);
 }

boards/hail/src/test_take_map_cell.rs

@@ -0,0 +1,44 @@
+use kernel::common::take_cell::MapCell;
+
+pub unsafe fn test_take_map_cell() {
+    static FOO: u32 = 1234;
+
+    static mut mc_ref: MapCell<&'static u32> = MapCell::new(&FOO);
+    test_map_cell(&mc_ref);
+
+    static mut mc1: MapCell<[[u8; 256]; 1]> = MapCell::new([[125; 256]; 1]);
+    test_map_cell(&mc1);
+
+    static mut mc2: MapCell<[[u8; 256]; 2]> = MapCell::new([[125; 256]; 2]);
+    test_map_cell(&mc2);
+
+    static mut mc3: MapCell<[[u8; 256]; 3]> = MapCell::new([[125; 256]; 3]);
+    test_map_cell(&mc3);
+
+    static mut mc4: MapCell<[[u8; 256]; 4]> = MapCell::new([[125; 256]; 4]);
+    test_map_cell(&mc4);
+
+    static mut mc5: MapCell<[[u8; 256]; 5]> = MapCell::new([[125; 256]; 5]);
+    test_map_cell(&mc5);
+
+    static mut mc6: MapCell<[[u8; 256]; 6]> = MapCell::new([[125; 256]; 6]);
+    test_map_cell(&mc6);
+
+    static mut mc7: MapCell<[[u8; 256]; 7]> = MapCell::new([[125; 256]; 7]);
+    test_map_cell(&mc7);
+}
+
+#[inline(never)]
+#[allow(unused_unsafe)]
+unsafe fn test_map_cell<'a, A>(tc: &MapCell<A>) {
+    let dwt_ctl: *mut u32 = 0xE0001000 as *mut u32;
+    let dwt_cycles: *mut u32 = 0xE0001004 as *mut u32;
+    let demcr: *mut u32 = 0xE000EDFC as *mut u32;
+
+    ::core::ptr::write_volatile(demcr, 0x01000000);
+    ::core::ptr::write_volatile(dwt_cycles, 0);
+    ::core::ptr::write_volatile(dwt_ctl, ::core::ptr::read_volatile(dwt_ctl) | 1);
+    tc.map(|_| ());
+    let end = ::core::ptr::read_volatile(dwt_cycles);
+    println!("time: {}, size: {}", end, ::core::mem::size_of_val(tc));
+}

boards/imix/src/main.rs

@@ -234,7 +234,9 @@ pub unsafe fn reset_handler() {
     let syscall_spi_device = static_init!(
         VirtualSpiMasterDevice<'static, sam4l::spi::Spi>,
         VirtualSpiMasterDevice::new(mux_spi, 3),
-        48);
+        352/8);
+
+    // Create the SPI systemc call capsule, passing the client
     let spi_syscalls = static_init!(
         capsules::spi::Spi<'static, VirtualSpiMasterDevice<'static, sam4l::spi::Spi>>,
         capsules::spi::Spi::new(syscall_spi_device),
@@ -264,7 +266,7 @@ pub unsafe fn reset_handler() {
     // Create a second virtualized SPI client, for the RF233
     let rf233_spi = static_init!(VirtualSpiMasterDevice<'static, sam4l::spi::Spi>,
                                  VirtualSpiMasterDevice::new(mux_spi, 3),
-                                 48);
+                                 352/8);
     // Create the RF233 driver, passing its pins and SPI client
     let rf233: &RF233<'static, VirtualSpiMasterDevice<'static, sam4l::spi::Spi>> =
         static_init!(RF233<'static, VirtualSpiMasterDevice<'static, sam4l::spi::Spi>>,

boards/storm/src/main.rs

@@ -314,7 +314,7 @@ pub unsafe fn reset_handler() {
     let syscall_spi_device = static_init!(
         VirtualSpiMasterDevice<'static, sam4l::spi::Spi>,
         VirtualSpiMasterDevice::new(mux_spi, 3),
-        384/8);
+        352/8);
 
     // Create the SPI systemc call capsule, passing the client
     let spi_syscalls = static_init!(

capsules/src/console.rs

@@ -3,6 +3,7 @@
 //! Console provides userspace with the ability to print text via a serial
 //! interface.
 
+use core::cell::Cell;
 use kernel::{AppId, AppSlice, Container, Callback, Shared, Driver, ReturnCode};
 use kernel::common::take_cell::TakeCell;
 use kernel::hil::uart::{self, UART, Client};
@@ -37,8 +38,8 @@ pub static mut WRITE_BUF: [u8; 64] = [0; 64];
 pub struct Console<'a, U: UART + 'a> {
     uart: &'a U,
     apps: Container<App>,
-    in_progress: TakeCell<AppId>,
-    tx_buffer: TakeCell<&'static mut [u8]>,
+    in_progress: Cell<Option<AppId>>,
+    tx_buffer: TakeCell<'static, [u8]>,
     baud_rate: u32,
 }
 
@@ -51,7 +52,7 @@ impl<'a, U: UART> Console<'a, U> {
         Console {
             uart: uart,
             apps: container,
-            in_progress: TakeCell::empty(),
+            in_progress: Cell::new(None),
             tx_buffer: TakeCell::new(tx_buffer),
             baud_rate: baud_rate,
         }
@@ -99,8 +100,8 @@ impl<'a, U: UART> Console<'a, U> {
     /// Internal helper function for sending data for an existing transaction.
     /// Cannot fail. If can't send now, it will schedule for sending later.
     fn send(&self, app_id: AppId, app: &mut App, slice: AppSlice<Shared, u8>) {
-        if self.in_progress.is_none() {
-            self.in_progress.replace(app_id);
+        if self.in_progress.get().is_none() {
+            self.in_progress.set(Some(app_id));
             self.tx_buffer.take().map(|buffer| {
                 let mut transaction_len = app.write_remaining;
                 for (i, c) in slice.as_ref()[slice.len() - app.write_remaining..slice.len()]
@@ -204,7 +205,8 @@ impl<'a, U: UART> Client for Console<'a, U> {
         // Either print more from the AppSlice or send a callback to the
         // application.
         self.tx_buffer.replace(buffer);
-        self.in_progress.take().map(|appid| {
+        self.in_progress.get().map(|appid| {
+            self.in_progress.set(None);
             self.apps.enter(appid, |app, _| {
                 match self.send_continue(appid, app) {
                     Ok(more_to_send) => {
@@ -229,7 +231,7 @@ impl<'a, U: UART> Client for Console<'a, U> {
 
         // If we are not printing more from the current AppSlice,
         // see if any other applications have pending messages.
-        if self.in_progress.is_none() {
+        if self.in_progress.get().is_none() {
             for cntr in self.apps.iter() {
                 let started_tx = cntr.enter(|app, _| {
                     if app.pending_write {

capsules/src/fm25cl.rs

@@ -4,7 +4,7 @@ use core::cell::Cell;
 use core::cmp;
 use kernel::{AppId, AppSlice, Callback, Driver, ReturnCode, Shared};
 
-use kernel::common::take_cell::TakeCell;
+use kernel::common::take_cell::{MapCell, TakeCell};
 use kernel::hil;
 
 
@@ -52,10 +52,10 @@ pub trait FM25CLClient {
 pub struct FM25CL<'a, S: hil::spi::SpiMasterDevice + 'a> {
     spi: &'a S,
     state: Cell<State>,
-    txbuffer: TakeCell<&'static mut [u8]>,
-    rxbuffer: TakeCell<&'static mut [u8]>,
-    client: TakeCell<&'static FM25CLClient>,
-    client_buffer: TakeCell<&'static mut [u8]>, // Store buffer and state for passing back to client
+    txbuffer: TakeCell<'static, [u8]>,
+    rxbuffer: TakeCell<'static, [u8]>,
+    client: Cell<Option<&'static FM25CLClient>>,
+    client_buffer: TakeCell<'static, [u8]>, // Store buffer and state for passing back to client
     client_write_address: Cell<u16>,
     client_write_len: Cell<u16>,
 }
@@ -71,15 +71,15 @@ impl<'a, S: hil::spi::SpiMasterDevice + 'a> FM25CL<'a, S> {
             state: Cell::new(State::Idle),
             txbuffer: TakeCell::new(txbuffer),
             rxbuffer: TakeCell::new(rxbuffer),
-            client: TakeCell::empty(),
+            client: Cell::new(None),
             client_buffer: TakeCell::empty(),
             client_write_address: Cell::new(0),
             client_write_len: Cell::new(0),
         }
     }
 
     pub fn set_client<C: FM25CLClient>(&self, client: &'static C) {
-        self.client.replace(client);
+        self.client.set(Some(client));
     }
 
     /// Setup SPI for this chip
@@ -165,7 +165,7 @@ impl<'a, S: hil::spi::SpiMasterDevice + 'a> hil::spi::SpiMasterClient for FM25CL
                     // Also replace this buffer
                     self.rxbuffer.replace(read_buffer);
 
-                    self.client.map(|client| { client.status(status); });
+                    self.client.get().map(|client| { client.status(status); });
                 });
             }
             State::WriteEnable => {
@@ -195,9 +195,7 @@ impl<'a, S: hil::spi::SpiMasterDevice + 'a> hil::spi::SpiMasterClient for FM25CL
 
                 // Call done with the write() buffer
                 self.client_buffer.take().map(move |buffer| {
-                    self.client.map(move |client| {
-                        client.done(buffer);
-                    });
+                    self.client.get().map(move |client| { client.done(buffer); });
                 });
             }
             State::ReadMemory => {
@@ -216,7 +214,7 @@ impl<'a, S: hil::spi::SpiMasterDevice + 'a> hil::spi::SpiMasterClient for FM25CL
 
                         self.rxbuffer.replace(read_buffer);
 
-                        self.client.map(move |client| { client.read(buffer, read_len); });
+                        self.client.get().map(move |client| { client.read(buffer, read_len); });
                     });
                 });
             }
@@ -227,18 +225,18 @@ impl<'a, S: hil::spi::SpiMasterDevice + 'a> hil::spi::SpiMasterClient for FM25CL
 
 /// Holds buffers and whatnot that the application has passed us.
 struct AppState {
-    callback: Cell<Option<Callback>>,
-    read_buffer: TakeCell<AppSlice<Shared, u8>>,
-    write_buffer: TakeCell<AppSlice<Shared, u8>>,
+    callback: Option<Callback>,
+    read_buffer: Option<AppSlice<Shared, u8>>,
+    write_buffer: Option<AppSlice<Shared, u8>>,
 }
 
 /// Default implementation of the FM25CL driver that provides a Driver
 /// interface for providing access to applications.
 pub struct FM25CLDriver<'a, S: hil::spi::SpiMasterDevice + 'a> {
     fm25cl: &'a FM25CL<'a, S>,
-    app_state: TakeCell<AppState>,
-    kernel_read: TakeCell<&'static mut [u8]>,
-    kernel_write: TakeCell<&'static mut [u8]>,
+    app_state: MapCell<AppState>,
+    kernel_read: TakeCell<'static, [u8]>,
+    kernel_write: TakeCell<'static, [u8]>,
 }
 
 impl<'a, S: hil::spi::SpiMasterDevice + 'a> FM25CLDriver<'a, S> {
@@ -248,7 +246,7 @@ impl<'a, S: hil::spi::SpiMasterDevice + 'a> FM25CLDriver<'a, S> {
                -> FM25CLDriver<'a, S> {
         FM25CLDriver {
             fm25cl: fm25,
-            app_state: TakeCell::empty(),
+            app_state: MapCell::empty(),
             kernel_read: TakeCell::new(read_buf),
             kernel_write: TakeCell::new(write_buf),
         }
@@ -258,15 +256,15 @@ impl<'a, S: hil::spi::SpiMasterDevice + 'a> FM25CLDriver<'a, S> {
 impl<'a, S: hil::spi::SpiMasterDevice + 'a> FM25CLClient for FM25CLDriver<'a, S> {
     fn status(&self, status: u8) {
         self.app_state.map(|app_state| {
-            app_state.callback.get().map(|mut cb| { cb.schedule(0, status as usize, 0); });
+            app_state.callback.map(|mut cb| { cb.schedule(0, status as usize, 0); });
         });
     }
 
     fn read(&self, data: &'static mut [u8], len: usize) {
         self.app_state.map(|app_state| {
             let mut read_len: usize = 0;
 
-            app_state.read_buffer.map(move |read_buffer| {
+            app_state.read_buffer.as_mut().map(move |read_buffer| {
                 read_len = cmp::min(read_buffer.len(), len);
 
                 let d = &mut read_buffer.as_mut()[0..(read_len as usize)];
@@ -277,15 +275,15 @@ impl<'a, S: hil::spi::SpiMasterDevice + 'a> FM25CLClient for FM25CLDriver<'a, S>
                 self.kernel_read.replace(data);
             });
 
-            app_state.callback.get().map(|mut cb| { cb.schedule(1, read_len, 0); });
+            app_state.callback.map(|mut cb| { cb.schedule(1, read_len, 0); });
         });
     }
 
     fn done(&self, buffer: &'static mut [u8]) {
         self.kernel_write.replace(buffer);
 
         self.app_state
-            .map(|app_state| { app_state.callback.get().map(|mut cb| { cb.schedule(2, 0, 0); }); });
+            .map(|app_state| { app_state.callback.map(|mut cb| { cb.schedule(2, 0, 0); }); });
     }
 }
 
@@ -297,13 +295,13 @@ impl<'a, S: hil::spi::SpiMasterDevice + 'a> Driver for FM25CLDriver<'a, S> {
                 let appst = match self.app_state.take() {
                     None => {
                         AppState {
-                            callback: Cell::new(None),
-                            read_buffer: TakeCell::new(slice),
-                            write_buffer: TakeCell::empty(),
+                            callback: None,
+                            read_buffer: Some(slice),
+                            write_buffer: None,
                         }
                     }
-                    Some(appst) => {
-                        appst.read_buffer.replace(slice);
+                    Some(mut appst) => {
+                        appst.read_buffer = Some(slice);
                         appst
                     }
                 };
@@ -312,20 +310,15 @@ impl<'a, S: hil::spi::SpiMasterDevice + 'a> Driver for FM25CLDriver<'a, S> {
             }
             // Pass write buffer in from application
             1 => {
-                let appst = match self.app_state.take() {
-                    None => {
-                        AppState {
-                            callback: Cell::new(None),
-                            write_buffer: TakeCell::new(slice),
-                            read_buffer: TakeCell::empty(),
-                        }
-                    }
-                    Some(appst) => {
-                        appst.write_buffer.replace(slice);
-                        appst
-                    }
-                };
-                self.app_state.replace(appst);
+                if self.app_state.is_none() {
+                    self.app_state.put(AppState {
+                        callback: None,
+                        write_buffer: Some(slice),
+                        read_buffer: None,
+                    });
+                } else {
+                    self.app_state.map(|appst| appst.write_buffer = Some(slice));
+                }
                 ReturnCode::SUCCESS
             }
             _ => ReturnCode::ENOSUPPORT,
@@ -335,20 +328,15 @@ impl<'a, S: hil::spi::SpiMasterDevice + 'a> Driver for FM25CLDriver<'a, S> {
     fn subscribe(&self, subscribe_num: usize, callback: Callback) -> ReturnCode {
         match subscribe_num {
             0 => {
-                let appst = match self.app_state.take() {
-                    None => {
-                        AppState {
-                            callback: Cell::new(Some(callback)),
-                            write_buffer: TakeCell::empty(),
-                            read_buffer: TakeCell::empty(),
-                        }
-                    }
-                    Some(appst) => {
-                        appst.callback.set(Some(callback));
-                        appst
-                    }
-                };
-                self.app_state.replace(appst);
+                if self.app_state.is_none() {
+                    self.app_state.put(AppState {
+                        callback: Some(callback),
+                        write_buffer: None,
+                        read_buffer: None,
+                    });
+                } else {
+                    self.app_state.map(|appst| appst.callback = Some(callback));
+                }
                 ReturnCode::SUCCESS
             }
 
@@ -385,7 +373,7 @@ impl<'a, S: hil::spi::SpiMasterDevice + 'a> Driver for FM25CLDriver<'a, S> {
                 let len = ((data >> 16) & 0xFFFF) as usize;
 
                 self.app_state.map(|app_state| {
-                    app_state.write_buffer.map(|write_buffer| {
+                    app_state.write_buffer.as_mut().map(|write_buffer| {
                         self.kernel_write.take().map(|kernel_write| {
                             // Check bounds for write length
                             let buf_len = cmp::min(write_buffer.len(), kernel_write.len());