Re: Review Request 121930: [OS X] improvements to KSharedData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/ --- (Updated Jan. 9, 2015, 3:25 p.m.) Status -- This change has been marked as submitted. Review request for KDE Software on Mac OS X, kdelibs and Allen Winter. Repository: kdelibs Description --- This patch improves KSharedData on 2 points: - It enables `KSDC_THREAD_PROCESS_SHARED_SUPPORTED` on OS X because even if the OS cannot do timeouts on mutex locking, it does have Posix mutexes (pthreads). I don't know why this was deactivated explicitly on OS X (do you remember, Allan?), but haven't seen issues with KSDC_THREAD_PROCESS_SHARED_SUPPORTED - for now. - OS X doesn't have `posix_fallocate()`, but an emulation of this function is available in the Mozilla code (reference found on StackOverflow). The code seems to be license-compatible, so I removed the code for non-OS X platforms, and include it in `kshareddatacache_p.h`. Again, this seems to work. Diffs - kdecore/util/kshareddatacache_p.h 931de4d kdecore/util/posix_fallocate_mac.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/121930/diff/ Testing --- On OS X 10.9.5 with kdelibs 4.14.4 and KDE PIM 4.13.3 (I use KMail as my default MUA). Thanks, René J.V. Bertin
Re: Review Request 121930: [OS X] improvements to KSharedData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/ --- (Updated Jan. 9, 2015, 3:19 p.m.) Review request for KDE Software on Mac OS X, kdelibs and Allen Winter. Repository: kdelibs Description --- This patch improves KSharedData on 2 points: - It enables `KSDC_THREAD_PROCESS_SHARED_SUPPORTED` on OS X because even if the OS cannot do timeouts on mutex locking, it does have Posix mutexes (pthreads). I don't know why this was deactivated explicitly on OS X (do you remember, Allan?), but haven't seen issues with KSDC_THREAD_PROCESS_SHARED_SUPPORTED - for now. - OS X doesn't have `posix_fallocate()`, but an emulation of this function is available in the Mozilla code (reference found on StackOverflow). The code seems to be license-compatible, so I removed the code for non-OS X platforms, and include it in `kshareddatacache_p.h`. Again, this seems to work. Diffs (updated) - kdecore/util/kshareddatacache_p.h 931de4d kdecore/util/posix_fallocate_mac.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/121930/diff/ Testing --- On OS X 10.9.5 with kdelibs 4.14.4 and KDE PIM 4.13.3 (I use KMail as my default MUA). Thanks, René J.V. Bertin
Re: Review Request 121930: [OS X] improvements to KSharedData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/#review73597 --- Ship it! kdecore/util/kshareddatacache_p.h https://git.reviewboard.kde.org/r/121930/#comment51245 just add that to the conditional above, no need to duplicate it here. sorry that I didn't spot this sooner - Milian Wolff On Jan. 9, 2015, 2:19 p.m., René J.V. Bertin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/ --- (Updated Jan. 9, 2015, 2:19 p.m.) Review request for KDE Software on Mac OS X, kdelibs and Allen Winter. Repository: kdelibs Description --- This patch improves KSharedData on 2 points: - It enables `KSDC_THREAD_PROCESS_SHARED_SUPPORTED` on OS X because even if the OS cannot do timeouts on mutex locking, it does have Posix mutexes (pthreads). I don't know why this was deactivated explicitly on OS X (do you remember, Allan?), but haven't seen issues with KSDC_THREAD_PROCESS_SHARED_SUPPORTED - for now. - OS X doesn't have `posix_fallocate()`, but an emulation of this function is available in the Mozilla code (reference found on StackOverflow). The code seems to be license-compatible, so I removed the code for non-OS X platforms, and include it in `kshareddatacache_p.h`. Again, this seems to work. Diffs - kdecore/util/kshareddatacache_p.h 931de4d kdecore/util/posix_fallocate_mac.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/121930/diff/ Testing --- On OS X 10.9.5 with kdelibs 4.14.4 and KDE PIM 4.13.3 (I use KMail as my default MUA). Thanks, René J.V. Bertin
Re: Review Request 121930: [OS X] improvements to KSharedData
On Jan. 8, 2015, 6:12 p.m., Milian Wolff wrote: personally, I also think that if you tested and it works, and Allan has no objections, that you can go ahead and push this. but please don't comment out code, just remove it. René J.V. Bertin wrote: OK, will do. I'll give it a bit more testing, though. Mutex locking without timeouts could lead to deadlocks, and maybe that's why the feature was disabled on OS X (maybe someone even ran into such a deadlock). I just did a build-and-test using KGoldrunner, a highly-animated game that requires KSharedDataCache for SVG graphics pixmaps, rendered at various sizes. Everything worked perfectly. There was plenty of kDebug log output from KSharedDataCache, but nowhere did I see any error messages, such as those which used to occur in https://bugs.kde.org/show_bug.cgi?id=307652. I also tested with no cache-files present initially and with theme-changes and fast resizes of the main window while the demo was running. Resizes cause pixmaps of new sizes to be added to the cache, or retrieved if the required size is already there. This is quite a tough test of KSharedDataCache performance, but I do not think it would be prone to deadlocks. I tried two KGr demo windows running at once, but there is a feature in KGr that automatically pauses whichever window is not in focus. Real-time performance was very good throughout all tests. So ship away, René! - Ian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/#review73521 --- On Jan. 8, 2015, 5:09 p.m., René J.V. Bertin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/ --- (Updated Jan. 8, 2015, 5:09 p.m.) Review request for KDE Software on Mac OS X, kdelibs and Allen Winter. Repository: kdelibs Description --- This patch improves KSharedData on 2 points: - It enables `KSDC_THREAD_PROCESS_SHARED_SUPPORTED` on OS X because even if the OS cannot do timeouts on mutex locking, it does have Posix mutexes (pthreads). I don't know why this was deactivated explicitly on OS X (do you remember, Allan?), but haven't seen issues with KSDC_THREAD_PROCESS_SHARED_SUPPORTED - for now. - OS X doesn't have `posix_fallocate()`, but an emulation of this function is available in the Mozilla code (reference found on StackOverflow). The code seems to be license-compatible, so I removed the code for non-OS X platforms, and include it in `kshareddatacache_p.h`. Again, this seems to work. Diffs - kdecore/util/kshareddatacache_p.h 931de4d kdecore/util/posix_fallocate_mac.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/121930/diff/ Testing --- On OS X 10.9.5 with kdelibs 4.14.4 and KDE PIM 4.13.3 (I use KMail as my default MUA). Thanks, René J.V. Bertin
Re: Review Request 121930: [OS X] improvements to KSharedData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/#review73519 --- No, I can't recall why i did that. You know a lot more about OSX than I do. - Allen Winter On Jan. 8, 2015, 5:09 p.m., René J.V. Bertin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/ --- (Updated Jan. 8, 2015, 5:09 p.m.) Review request for KDE Software on Mac OS X, kdelibs and Allen Winter. Repository: kdelibs Description --- This patch improves KSharedData on 2 points: - It enables `KSDC_THREAD_PROCESS_SHARED_SUPPORTED` on OS X because even if the OS cannot do timeouts on mutex locking, it does have Posix mutexes (pthreads). I don't know why this was deactivated explicitly on OS X (do you remember, Allan?), but haven't seen issues with KSDC_THREAD_PROCESS_SHARED_SUPPORTED - for now. - OS X doesn't have `posix_fallocate()`, but an emulation of this function is available in the Mozilla code (reference found on StackOverflow). The code seems to be license-compatible, so I removed the code for non-OS X platforms, and include it in `kshareddatacache_p.h`. Again, this seems to work. Diffs - kdecore/util/kshareddatacache_p.h 931de4d kdecore/util/posix_fallocate_mac.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/121930/diff/ Testing --- On OS X 10.9.5 with kdelibs 4.14.4 and KDE PIM 4.13.3 (I use KMail as my default MUA). Thanks, René J.V. Bertin
Re: Review Request 121930: [OS X] improvements to KSharedData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/#review73521 --- Ship it! personally, I also think that if you tested and it works, and Allan has no objections, that you can go ahead and push this. but please don't comment out code, just remove it. kdecore/util/kshareddatacache_p.h https://git.reviewboard.kde.org/r/121930/#comment51185 remove dead code - Milian Wolff On Jan. 8, 2015, 5:09 p.m., René J.V. Bertin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/ --- (Updated Jan. 8, 2015, 5:09 p.m.) Review request for KDE Software on Mac OS X, kdelibs and Allen Winter. Repository: kdelibs Description --- This patch improves KSharedData on 2 points: - It enables `KSDC_THREAD_PROCESS_SHARED_SUPPORTED` on OS X because even if the OS cannot do timeouts on mutex locking, it does have Posix mutexes (pthreads). I don't know why this was deactivated explicitly on OS X (do you remember, Allan?), but haven't seen issues with KSDC_THREAD_PROCESS_SHARED_SUPPORTED - for now. - OS X doesn't have `posix_fallocate()`, but an emulation of this function is available in the Mozilla code (reference found on StackOverflow). The code seems to be license-compatible, so I removed the code for non-OS X platforms, and include it in `kshareddatacache_p.h`. Again, this seems to work. Diffs - kdecore/util/kshareddatacache_p.h 931de4d kdecore/util/posix_fallocate_mac.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/121930/diff/ Testing --- On OS X 10.9.5 with kdelibs 4.14.4 and KDE PIM 4.13.3 (I use KMail as my default MUA). Thanks, René J.V. Bertin
Re: Review Request 121930: [OS X] improvements to KSharedData
On Jan. 8, 2015, 7:12 p.m., Milian Wolff wrote: personally, I also think that if you tested and it works, and Allan has no objections, that you can go ahead and push this. but please don't comment out code, just remove it. OK, will do. I'll give it a bit more testing, though. Mutex locking without timeouts could lead to deadlocks, and maybe that's why the feature was disabled on OS X (maybe someone even ran into such a deadlock). - René J.V. --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/#review73521 --- On Jan. 8, 2015, 6:09 p.m., René J.V. Bertin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/ --- (Updated Jan. 8, 2015, 6:09 p.m.) Review request for KDE Software on Mac OS X, kdelibs and Allen Winter. Repository: kdelibs Description --- This patch improves KSharedData on 2 points: - It enables `KSDC_THREAD_PROCESS_SHARED_SUPPORTED` on OS X because even if the OS cannot do timeouts on mutex locking, it does have Posix mutexes (pthreads). I don't know why this was deactivated explicitly on OS X (do you remember, Allan?), but haven't seen issues with KSDC_THREAD_PROCESS_SHARED_SUPPORTED - for now. - OS X doesn't have `posix_fallocate()`, but an emulation of this function is available in the Mozilla code (reference found on StackOverflow). The code seems to be license-compatible, so I removed the code for non-OS X platforms, and include it in `kshareddatacache_p.h`. Again, this seems to work. Diffs - kdecore/util/kshareddatacache_p.h 931de4d kdecore/util/posix_fallocate_mac.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/121930/diff/ Testing --- On OS X 10.9.5 with kdelibs 4.14.4 and KDE PIM 4.13.3 (I use KMail as my default MUA). Thanks, René J.V. Bertin
Review Request 121930: [OS X] improvements to KSharedData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121930/ --- Review request for KDE Software on Mac OS X, kdelibs and Allen Winter. Repository: kdelibs Description --- This patch improves KSharedData on 2 points: - It enables `KSDC_THREAD_PROCESS_SHARED_SUPPORTED` on OS X because even if the OS cannot do timeouts on mutex locking, it does have Posix mutexes (pthreads). I don't know why this was deactivated explicitly on OS X (do you remember, Allan?), but haven't seen issues with KSDC_THREAD_PROCESS_SHARED_SUPPORTED - for now. - OS X doesn't have `posix_fallocate()`, but an emulation of this function is available in the Mozilla code (reference found on StackOverflow). The code seems to be license-compatible, so I removed the code for non-OS X platforms, and include it in `kshareddatacache_p.h`. Again, this seems to work. Diffs - kdecore/util/kshareddatacache_p.h 931de4d kdecore/util/posix_fallocate_mac.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/121930/diff/ Testing --- On OS X 10.9.5 with kdelibs 4.14.4 and KDE PIM 4.13.3 (I use KMail as my default MUA). Thanks, René J.V. Bertin
