therealprof / display-interface

Rust crates providing a generic interface for display drivers and some default implementations (GPIO, SPI and I2C)
Apache License 2.0
73 stars 27 forks source link

lib: add #[inline] hint for most of the functions #47

Closed luojia65 closed 3 months ago

luojia65 commented 8 months ago

This pull request adds #[inline] to most of the functions; it will perform better on bare-metal programs.

luojia65 commented 8 months ago

r? @bugadani

almindor commented 8 months ago

I don't think this should be the default. Could we feature-gate it? I suspect it might be a net negative in some cases to inline.

bugadani commented 8 months ago

I might be wrong but as far as I know generic code is inline by default anyway. Do you have some example where it is not?

In general I'm not against adding #[inline] to public API, and I believe it is also encouraged generally (clippy has a lint for it, for example). But this is generic code so I'm not sure what the results are, so it would be nice to know some specific cases where it helps (and where it hurts, too!)

For new and release this is a no brainer. For others, like send_command and send_data those functions may just be large enough anyway for the compiler to not consider inlining them.

it will perform better

Do you have numbers that support this claim? How much better, in what cases?

Could we feature-gate it?

Please don't. #[inline] is a hint and not a guarantee, so that feature gate may or may not do anything and it would be very confusing.