search

mjhouse / ag-lcd

A rust port of the C++ LiquidCrystal library for arduino systems
GNU General Public License v3.0
17 stars 8 forks source link

Add `#[inline(always)]` to setter/getter methods on LcdDisplay #30

Open mjhouse opened 1 year ago

mjhouse commented 1 year ago

Most of these could probably have an inline declaration. I'm not sure if it would hurt anything to do it for every function, actually, but for now, all of the getters and setters could be marked for inlining- set_blink, blink_off, set_position etc.

vcrn commented 1 year ago

My knowledge regarding this is quite basic, being more or less that inlining tends to decrease runtime cost at the price of compilation time. Is your reasoning that the crate will be compiled way fewer times in comparison with how much the code will be used on running systems (and that the runtime costs could be especially important on the target devices)?

mjhouse commented 1 year ago

It probably wouldn't noticeably impact compile times, but it could improve runtime performance a bit. Technically. However, the compiler is probably already inlining them, and even if it didn't, they probably wouldn't be called often enough for it to be noticeable. I made this issue mostly just because we can do it, it won't hurt anything, and it's conceivable that it could improve performance.

vcrn commented 1 year ago

Having read the code of some HALs now, I agree with you on all your points. The compiler should do this already (but it isn't certain), it won't hurt, and it could improve performance.