Summary: | sal - add sane sleep interface: cleanup osl_waitThread | ||
---|---|---|---|
Product: | LibreOffice | Reporter: | Michael Meeks <michael.meeks> |
Component: | LibreOffice | Assignee: | Kevin Dubrulle <kevin.dubrulle> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | h3734236, mentoring, sberg.fun |
Priority: | medium | Keywords: | difficultyBeginner, easyHack, skillCpp, topicCleanup |
Version: | 4.3.0.2 rc | ||
Hardware: | Other | ||
OS: | All | ||
See Also: | https://bugs.documentfoundation.org/show_bug.cgi?id=100085 | ||
Whiteboard: | target:5.2.0 target:5.4.0 target:6.2.0 | ||
Crash report or crash signature: | Regression By: |
Description
Michael Meeks
2014-09-25 10:05:57 UTC
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. A polite ping, still working on this bug? 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. A polite ping, still working on this bug? Hi, In my opinion, this issue should be closed. Sounds sensible - thanks for the nice fixes everyone ! =) |