| Summary: | hardware_concurrency not ideal for thread pools ... | ||
|---|---|---|---|
| Product: | LibreOffice | Reporter: | Michael Meeks <michael.meeks> |
| Component: | LibreOffice | Assignee: | Ashod Nakashian <ashodnakashian> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | drichard, sberg.fun, xiscofauli |
| Priority: | medium | ||
| Version: | 5.1.0.1 rc | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | target:5.3.0 target:5.2.0.1 target:5.1.5 | ||
| Crash report or crash signature: | Regression By: | ||
|
Description
Michael Meeks
2016-03-29 13:35:05 UTC
Stephan - any thoughts ? =) I imagine a sal/ wrapper would make most sense here - I imagine that some OS' have a tweakable for this and others don't =) I imagine we'll need the same thing for 'online' too as we run on larger hardware. So comphelper::ThreadPool spawns a fixed number of threads upfront, regardless of how many are actually ever needed? Sounds odd. Maybe std::async would be a more useful implementation to use. Not sure there's any support on any OS to determine such a concurrency parameter that would be deemed useful in the given scenario. But sure, if there is, sal looks like a logical place to put it. (I just want to avoid adding anything there that's already available anyway, like in the std::thread::hardware_concurrency case.) > So comphelper::ThreadPool spawns a fixed number of threads upfront, > regardless of how many are actually ever needed? Sounds odd. Don't think so - but we use the standard thread-pool <N> wide in where <N> is std::thread::hardware_concurrency() in a number of places: XLSX load, image scaling, and so on which are triggered reasonably commonly. Things like: sc/source/filter/excel/xetable.cxx: const size_t nThreads = nRows < 128 ? 1 : std::max(std::thread::hardware_concurrency(), 1U); Fall a bit foul of the '80 cores' there - which (anyway) are shared with a large number of other users =) > Not sure there's any support on any OS to determine such a concurrency > parameter that would be deemed useful in the given scenario. Fair enough; either way - I think it'd be reasonable for an administrator (or just when debugging) to have an environment variable / tweak-able to affect the maximum parallelism we want. Then again - no real need for it to go into sal/ I guess. Something like: sal_uInt32 getPreferredConcurrency(); somewhere would do the trick I guess; backending onto an env. var and the std::thread method. Ashod Nakashian committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=60e75fb276778459f6055360646d879b8c615d83 tdf#98955 hardware_concurrency not ideal for thread pools It will be available in 5.3.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. Ashod Nakashian committed a patch related to this issue. It has been pushed to "libreoffice-5-2": http://cgit.freedesktop.org/libreoffice/core/commit/?id=c739cc104844893955dc9a4de2bf8050759703f2&h=libreoffice-5-2 tdf#98955 hardware_concurrency not ideal for thread pools It will be available in 5.2.0.1. 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. Ashod Nakashian committed a patch related to this issue. It has been pushed to "libreoffice-5-1": http://cgit.freedesktop.org/libreoffice/core/commit/?id=197c1dacd5725db8ab2faf0aa9b39a2655eb4940&h=libreoffice-5-1 tdf#98955 hardware_concurrency not ideal for thread pools It will be available in 5.1.5. 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, Is this bug fixed? If so, could you please close it as RESOLVED FIXED? Regards Yes fixed; thanks for noticing =) |