Bug 98955 - hardware_concurrency not ideal for thread pools ...
Summary: hardware_concurrency not ideal for thread pools ...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
5.1.0.1 rc
Hardware: All All
: medium normal
Assignee: Ashod Nakashian
QA Contact:
URL:
Whiteboard: target:5.3.0 target:5.2.0.1 target:5.1.5
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-29 13:35 UTC by Michael Meeks
Modified: 2016-09-16 04:02 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2016-03-29 13:35:05 UTC
Currently we use:

std::thread::hardware_concurrency()

in several places to see what parallelism we can use to optimize performance.

This leads to some amusing situations; eg. in Largo Florida they have terminal server machines with ~80 CPUs - and so things like XLSX loading, and/or image scaling end up creating huge scads of threads that look like:

Thread 65 (Thread 0x7f3d858b6700 (LWP 56434)):
#0  0x00007f3daa0f238c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
#1  0x00007f3dab311c4b in osl_waitCondition () from /opt/libreofficedev5.0/program/libuno_sal.so.3
#2  0x00007f3da63d5739 in comphelper::ThreadPool::ThreadWorker::execute() () from /opt/libreofficedev5.0/program/libcomphelper.so
#3  0x00007f3da5542516 in salhelper::Thread::run() () from /opt/libreofficedev5.0/program/libuno_salhelpergcc3.so.3
#4  0x00007f3da554271a in threadFunc () from /opt/libreofficedev5.0/program/libuno_salhelpergcc3.so.3
#5  0x00007f3dab3206e7 in osl_thread_start_Impl(void*) () from /opt/libreofficedev5.0/program/libuno_sal.so.3
#6  0x00007f3daa0eda3f in start_thread () from /lib64/libpthread.so.0
#7  0x00007f3dab04871d in clone () from /lib64/libc.so.6
#8  0x0000000000000000 in ?? ()
 
Thread 64 (Thread 0x7f3d8dff5700 (LWP 56435)):
#0  0x00007f3daa0f238c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
#1  0x00007f3dab311c4b in osl_waitCondition () from /opt/libreofficedev5.0/program/libuno_sal.so.3
#2  0x00007f3da63d5739 in comphelper::ThreadPool::ThreadWorker::execute() () from /opt/libreofficedev5.0/program/libcomphelper.so
#3  0x00007f3da5542516 in salhelper::Thread::run() () from /opt/libreofficedev5.0/program/libuno_salhelpergcc3.so.3
#4  0x00007f3da554271a in threadFunc () from /opt/libreofficedev5.0/program/libuno_salhelpergcc3.so.3
#5  0x00007f3dab3206e7 in osl_thread_start_Impl(void*) () from /opt/libreofficedev5.0/program/libuno_sal.so.3
#6  0x00007f3daa0eda3f in start_thread () from /lib64/libpthread.so.0
#7  0x00007f3dab04871d in clone () from /lib64/libc.so.6
#8  0x0000000000000000 in ?? ()
...

Which is less than ideal.

Most likely we should have a tweak-able, to affect the maximum parallelism we want to exploit as a single user on the machine.
Comment 1 Michael Meeks 2016-03-29 13:36:03 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.
Comment 2 Stephan Bergmann 2016-03-29 14:20:10 UTC
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.)
Comment 3 Michael Meeks 2016-03-29 15:25:02 UTC
> 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.
Comment 4 Commit Notification 2016-06-15 21:29:56 UTC
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.
Comment 5 Commit Notification 2016-06-17 09:21:01 UTC
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.
Comment 6 Commit Notification 2016-06-20 16:18:46 UTC
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.
Comment 7 Xisco Faulí 2016-09-15 20:40:45 UTC
Hi,
Is this bug fixed?
If so, could you please close it as RESOLVED FIXED?
Regards
Comment 8 Michael Meeks 2016-09-16 04:02:48 UTC
Yes fixed; thanks for noticing =)