SmartThingsCommunity / st-device-sdk-c

SmartThings SDK for Direct Connected Devices for C
Other
118 stars 126 forks source link

Spin loop in example.c seems an inefficient solution #111

Open kubycsolutions opened 1 year ago

kubycsolutions commented 1 year ago

Apologies if I'm preaching to the choir and you've already considered this, but unless there's an optimizer in modern C compilers that I'm not aware of, a raw spin loop on a volatile isn't a great design choice.

Barring optimization, a spin loop will always consume as many cycles as the scheduler will permit before transferring control to other tasks. That means those tasks (including your communications code) are being robbed of cycles they could be taking advantage of. I don't think simply making is_exit volatile changes that behavior (unless there's an optimizer I'm not aware of).

To hand-optimize this, it would be more efficient if each iteration of that loop called the operating system's yield(),via whatever name the C libraries give it these days. That way, if the test of is_exit fails, event_loop() would immediately return control to the scheduler, permitting other tasks to progress before testing again.

Even better efficiency might be obtained by waiting on a semaphore rather than a spin loop, and having signal_handler() set that semaphore. That's essentially the same yield-between-tests process, but might be handled entirely at the OS level where it may run more efficiently.

Admittedly, the spin loop works fine for purposes of a simple demo. But if you want this to be an example people will build on when implementing their own devices, it would be better to make it as clean as possible.

junyoun-kim commented 1 year ago

Thank you for your opinion about example spin loop.

Actually, we didn't much pay attention about efficiency when making this example. We wanted to show SDK usage on the example. Anyway we will consider your opinion on next patches.

Thank you.