openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.86k stars 3.57k forks source link

Improvement of sleeping in loop #4592

Closed o0lwj0o closed 5 years ago

o0lwj0o commented 5 years ago

Recently, I found an interesting case in this project. It has “Thread.sleep()” in a “while(true)” loop. I found some discussion in Stackoverflow and other websites. It can be refactored by using a executor service. https://ejrh.wordpress.com/2012/07/13/sleeping-in-loops-considered-harmful/ https://stackoverflow.com/questions/3535754/netbeans-java-new-hint-thread-sleep-called-in-loop

Detail websites and lines:

383 | https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.miele/src/main/java/org/openhab/binding/miele/internal/handler/MieleBridgeHandler.java 302 | https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.yamahareceiver/src/main/java/org/openhab/binding/yamahareceiver/internal/protocol/xml/InputWithNavigationControlXML.java 108 | https://github.com/openhab/openhab2-addons/blob/master/addons/voice/org.openhab.voice.kaldi/src/main/java/org/openhab/voice/kaldi/internal/STTServiceKaldiRunnable.java 250 347 | https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.lutron/src/main/java/org/openhab/binding/lutron/internal/grxprg/SocketSession.java 122 225 337 | https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.atlona/src/main/java/org/openhab/binding/atlona/internal/net/SocketChannelSession.java 229 | https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.tellstick/src/main/java/org/openhab/binding/tellstick/internal/core/TelldusCoreDeviceController.java 444 461 478 | https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.kodi/src/main/java/org/openhab/binding/kodi/internal/handler/KodiHandler.java 146 | https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/api/FreeboxApiManager.java 516 | https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/api/FreeboxApiManager.java 1344 | https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.tesla/src/main/java/org/openhab/binding/tesla/handler/TeslaHandler.java 207 284 | https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.russound/src/main/java/org/openhab/binding/russound/internal/net/SocketChannelSession.java 214 | https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.miio/src/main/java/org/openhab/binding/miio/internal/transport/MiIoAsyncCommunication.java

Best regards

davidgraeff commented 5 years ago

Nah, I don't fully agree. If a protocol forces you to "wait" for 50ms or 100ms a Thread.sleep() is not harmful. The operating system will leave and schedule the task anyway.

And because java forces you to handle the InterruptException you can also interrupt those receive data threads if the handler gets disposed.

However: Recently I refactored the Milight addon to use the operating systems socket timeout instead of a Thread.sleep() in my receive Threads. That is the more elegant solution, I guess.

o0lwj0o commented 5 years ago

I agree " If a protocol forces you to "wait" for 50ms or 100ms a Thread.sleep() is not harmful. ". However, when i wait for a response, it will get the problem that how long will i wait. I think leave this determined by OS will be better.

davidgraeff commented 5 years ago

We are accepting PRs to replace any unnecessary Sleep with a scheduler call. Closing this generic Issue however as not being helpful for the project.