LibreOffice has a nice 'usleep' style function - but its docs suck. the method is osl_waitThread - and it is sufficiently opaque that I couldn't find it: include/osl/thread.h /** Blocks the calling thread at least for the given number of time. */ SAL_DLLPUBLIC void SAL_CALL osl_waitThread(const TimeValue* pDelay); That comment needs to be longer, describe the parameter, and mention the string 'sleep' and also 'usleep' ideally =) It might be nice to have a usleep version that sets up the TimeValue for you in the hxx header too: static void SAL_CALL wait(const TimeValue& Delay) { osl_waitThread(&Delay); } And then we should fix the call-sites that cooked their own version; even inside sal/ this happens: cd sal ; git grep -5 Sleep to find the several instances that need to be cleaned up here =) Thanks !
Easy-hackify ...
Hi I would like to know what I suppose to do in this part: And then we should fix the call-sites that cooked their own version; even inside sal/ this happens: cd sal ; git grep -5 Sleep to find the several instances that need to be cleaned up here =) Should I replaces the places where Sleep appears to wait or usleep or should I just remove Sleep. Thanks
We should do a functionally identical replacement of Sleep with our nice new, static / inline (?) usleep thing I guess: re-factoring & cleanup should provide something functionally identical in that commit. Perhaps later we'll discover that we can do better and/or drop some sleeps too =)
Workin' on that.
Stephan Bergmann committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=b190574b6a0de3e26c6338324f74440ec0505bfb tdf#84323: Make osl::Thread::wait more readable It will be available in 5.1.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Stephan Bergmann committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=26821b7e2eff4a7db22606f70daa3f3d442f7525 tdf#84323: Make osl::Condition::wait more readable It will be available in 5.1.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
See the commits mentioned in comment 5 and comment 6 for ways to improve the readability of calls to osl::Thread::wait and osl::Condition::wait, respectively. Improve further calls of these two functions across the code base.
Migrating Whiteboard tags to Keywords: (easyHack, difficultyBeginner, skillCpp, topicCleanup) [NinjaEdit]
JanI is default CC for Easy Hacks (Add Jan; remove LibreOffice Dev List from CC) [NinjaEdit]
Gurkaran committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=9e7447d39e356857ef5786b513e99cee79385247 tdf#84323: Make osl::Thread::wait more readable It will be available in 5.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Gurkaran committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=d27e3eca2cca1249a0bdd9e6385ca693d471ef9b tdf#84323: Make osl::Condition::wait more readable It will be available in 5.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
(asked at <https://gerrit.libreoffice.org/#/c/23417/2> to clarify whether comment 10 and comment 11 cover all calls that remained to be cleaned up, in which case this issue should be closed now)
Some good improvements here; I hate to re-open bugs; but a quick grep shows eg. helpcompiler/source/HelpCompiler.cxx-static void impl_sleep( sal_uInt32 nSec ) helpcompiler/source/HelpCompiler.cxx-{ helpcompiler/source/HelpCompiler.cxx- TimeValue aTime; helpcompiler/source/HelpCompiler.cxx- aTime.Seconds = nSec; helpcompiler/source/HelpCompiler.cxx- aTime.Nanosec = 0; helpcompiler/source/HelpCompiler.cxx- helpcompiler/source/HelpCompiler.cxx: osl::Thread::wait( aTime ); helpcompiler/source/HelpCompiler.cxx-} Which is another victim of this API ;-) I'd love to cleanup and clarify things like: sal/qa/osl/thread/test_thread.cxx- // Give the spawned threads enough time to terminate: sal/qa/osl/thread/test_thread.cxx- TimeValue const twentySeconds = { 20, 0 }; sal/qa/osl/thread/test_thread.cxx: osl::Thread::wait(twentySeconds); or: sd/qa/unit/misc-tests.cxx- TimeValue aSleep(0, 100 * 1000000); // 100 msec sd/qa/unit/misc-tests.cxx: osl::Thread::wait(aSleep); Which could be much clearer with the chrono thing =) Also I would want to see a nice example in include/sal if possible - in a verbose comment - that includes the word 'sleep' ;-) cd include/sal git grep -i sleep still no hits =) Hope that makes sense. Any chance of some more cleanup there ? =)
A polite ping, still working on this issue ?
Would be good to turn comment 13 into a new easy-hack perhaps =)
(In reply to Michael Meeks from comment #15) > Would be good to turn comment 13 into a new easy-hack perhaps =) As suggested so done :-) https://bugs.documentfoundation.org/show_bug.cgi?id=100085
Unassign due to lack of work. Remark, if you want to continue working on this patch, please assign it again.
Fakabbir Amin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=12fc1d5399a688a80eec2565a4b552377e428ab7 tdf#84323: Make osl::Condition::wait more readable It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
dilekuzulmez committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=8468945322b72ddadbe42584b9bba395c1e5bb7e tdf#84323: Make osl::Condition::wait more readable It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
dilekuzulmez committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=cbda84e5422c066b93d8c06b49c6bc86eca7c71f tdf#84323: Make osl::Condition::wait more readable It will be available in 5.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
A polite ping, still working on this bug?
Hi, I would like to work on that. First, I am cleanning calls to osl_waitThread with a hand made TimeValue with std::chrono. After, I will replace osl_waitThread to osl::Thread::wait, where it is possible.
Hi, Didn't have time recently, but I will !
Kevin Dubrulle committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=d0f44d8ba7e87aa263008d3cfc4e68294d783162 tdf#84323 - sal - add sane sleep interface: cleanup osl_waitThread It will be available in 6.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Hi, In my opinion, this issue should be closed.
Sounds sensible - thanks for the nice fixes everyone ! =)