Aircoookie / WLED

Control WS2812B and many more types of digital RGB LEDs with an ESP8266 or ESP32 over WiFi!
https://kno.wled.ge
MIT License
15.04k stars 3.25k forks source link

Complete transition of UsermodManager and PinManager away from classes #4213

Open willmmiles opened 1 month ago

willmmiles commented 1 month ago

In C++, the standard collection for a group of global functions is a namespace. Complete the transition of UsermodManager and PinManager away from classes, as they're no longer meaningful data structures.

This is purely a syntactic change and should have no functional effects.

netmindz commented 1 month ago

Is this because we only ever have a single instance of each so no need for a class?

Are there any advantages of just being a namespace?

willmmiles commented 1 month ago

Is this because we only ever have a single instance of each so no need for a class?

Are there any advantages of just being a namespace?

It's because there are no instances of each - they are no longer objects that should be instantiated at all. PinManager and UsermodManager are now collections of global functions and variables -- the standard C++ language tool for representing that concept is a namespace. There's no technical advantage to the change; it's more about being clear to a reader about what the nature of these collections is.

blazoncek commented 1 month ago

You could expand on the idea IMO. Even though we have strip global variable that is an instance of WS2812FX class, there is no need for it technically as there is always only one strip and most of its members are public. Or did I get the concept wrong?

willmmiles commented 1 month ago

Broadly, yes: any case where we have a singleton (a class that must be instantiated exactly once), it can be broken down in to collection of global variables and functions. There are two caveats:

Here is a quick compiler explorer example: https://godbolt.org/z/GxWrYs5nK

Note the first function ("my_cool_function") does a lot of ".LCx" lookups -- those are 32-bit pointers, stored in code memory nearby to the function body, that are filled in by the linker with the actual memory locations for the global variables. This makes it longer than the struct version, which can do relative address calculation (the struct ptr is held in a8 for the whole function.) The struct version will use more RAM, though, as it's got to add padding between the struct elements.

That said: link-time optimization has the power to turn the first version in to the second -- since the linker knows the exact layout, it can replace the global lookups with relative calculations. This is one of the ways LTO can improve code size.


For collections like UsermodManager and PinManager where there are few to no public member variables, and they're simple data types, this is a clear win.

For WS2812FX, I think it would be an interesting experiment, but I'm a lot less certain where the chips would fall. It would definitely cause a lot of code churn (eg. strip.x becoming FX::x or somesuch). I can give it a try, though!

netmindz commented 4 weeks ago

Ah yes. They went from singleton to static in 0.15. makes sense @willmmiles

Java is my primary language so everything has to be a class, even if every method is static, but if it's best practice with C++ to just use a namespace then we should follow that