Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experiment: remove Instance generics. #980

Closed
wants to merge 1 commit into from
Closed

Conversation

Dirbaio
Copy link
Member

@Dirbaio Dirbaio commented Sep 27, 2022

This is an experiment of removing "peripheral instance" generics on driver structs. For example, uart is now Uart<'d, Blocking> instead of Uart<'d, UART1, Blocking>.

Everything is still checked at compile time as before (such as pin functions). The T generic is moved from the struct to the new methods, not removed entirely.

The main advantage is you can now write code that works with, e.g, both UART0 and UART1 without generics or macros.

#[embassy_executor::main]
async fn main(spawner: Spawner) {
    let p = embassy_rp::init(Default::default());
    let config = uart::Config::default();
    let uart0 = uart::Uart::new_with_rtscts_blocking(p.UART0, p.PIN_0, p.PIN_1, p.PIN_3, p.PIN_2, config);
    let uart1 = uart::Uart::new_with_rtscts_blocking(p.UART1, p.PIN_4, p.PIN_5, p.PIN_7, p.PIN_6, config);
    spawner.spawn(hello_task(uart0)).unwrap();
    spawner.spawn(hello_task(uart1)).unwrap();
}

#[embassy_executor::task(pool_size = 2)]
async fn hello_task(mut uart: Uart<'static, Blocking>) {
    uart.blocking_write("Hello World!\r\n".as_bytes()).unwrap();

    loop {
        uart.blocking_write("hello there!\r\n".as_bytes()).unwrap();
        Timer::after(Duration::from_secs(1)).await;
    }
}

@Dirbaio
Copy link
Member Author

Dirbaio commented Sep 27, 2022

code size comparison with default build settings:

   text    data     bss     dec     hex filename
  14656      48    1552   16256    3f80 old, uart0
  14720      48    1568   16336    3fd0 new, uart0: +64 bytes
  16312      48    1616   17976    4638 old, uart0+uart1 (copypaste)
  15032      48    1656   16736    4160 new, uart0+uart1: -1280 bytes

As expected, it's a big win when using multiple instances because the code doesn't get duplicated

With build-std, panic-immediate-abort, lto=fat, opt-level=z:

   text    data     bss     dec     hex filename
   4356      48    1544    5948    173c old, uart0
   4376      48    1568    5992    1768 new, uart0: +20 bytes
   4652      48    1608    6308    18a4 old, uart0+uart1 (copypaste)
   4792      48    1656    6496    1960 new, uart0+uart1: +140 bytes

new loses, not sure why. Probably the inliner just changing its mind. It doesn't reproduce with a more complex example:

   text    data     bss     dec     hex filename
   5532      48    2592    8172    1fec uart_old_1
   5520      48    2632    8200    2008 uart_new_1: -12 bytes
   6644      48    3704   10396    289c uart_old_2
   6016      48    3784    9848    2678 uart_new_2: -628 bytes

so i'm pretty confident that in general new/old is a ~tie when using just 1 instance, and new wins when using more than 1.

@lulf lulf mentioned this pull request Nov 10, 2022
55 tasks
bors bot added a commit that referenced this pull request Nov 25, 2022
1044: rp/uart: use lockfree ringbuffer. r=Dirbaio a=Dirbaio

This gets rid of another PeripheralMutex usage.

Extracted out of #980 

Co-authored-by: Dario Nieuwenhuis <[email protected]>
@Dirbaio
Copy link
Member Author

Dirbaio commented Dec 3, 2024

this is hopelessly outdated. Needs redoing, probably copying the final design we ended up using for stm32.

@Dirbaio Dirbaio closed this Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant