Page MenuHome
Differential D1343

Bump maximum threads number to 1024
ClosedPublic
Actions

Authored by Sergey Sharybin (sergey) on Jun 8 2015, 9:15 AM.

Details

Reviewers
Bastien Montagne (mont29)
Campbell Barton (campbellbarton)
Maniphest Tasks
T43306: Feature request - remove or increase to 1024 the maximum number of threads
Commits
rBS12cdc67d8331: Bump maximum threads number to 1024
rB12cdc67d8331: Bump maximum threads number to 1024
Summary

This commit contains all the changes required for most optimal maximum threads
number bump. This is needed to avoid possibly unneeded initialization or data
allocation on systems with lower threads count.

TODO: Still need to review arrays in render data structures from render_types.h,

P.S. We might remove actual bump of max threads from this patch, so when we'll
be applying the patch we can do all the preparation work and then do actual
bump of max threads.

Diff Detail

Repository
rB Blender
Branch
max_threads_bump

Event Timeline

Sergey Sharybin (sergey) updated this revision to Diff 4358.Jun 8 2015, 9:16 AM
Sergey Sharybin (sergey) retitled this revision from to Bump maximum threads number to 1024.
Sergey Sharybin (sergey) added a reviewer: Campbell Barton (campbellbarton).
Sergey Sharybin (sergey) updated this object.
Sergey Sharybin (sergey) planned changes to this revision.Jun 8 2015, 1:54 PM

I screwed up and cases when scene->r.threads is set to higher than the system threads are not handled correct.

Aaron Carlisle (Blendify) added a task: T43306: Feature request - remove or increase to 1024 the maximum number of threads.Oct 6 2015, 7:42 PM
Sergey Sharybin (sergey) updated this revision to Diff 7050.Jul 13 2016, 4:21 PM

Just update against latest master.

Can't remember what was the issue i was having, nor can reproduce any.

Thing is, BLI_system_thread_count is always clamped to an actual limit
of threads and it's not possible to set user threads higher than that
value as well.

Sergey Sharybin (sergey) added a reviewer: Bastien Montagne (mont29).Jul 13 2016, 4:23 PM
Sergey Sharybin (sergey) planned changes to this revision.Jul 13 2016, 4:35 PM

Ahhh, oki. It's all fine when you use -t 1024 but fails when one sets 1024 threads in the interface. Oki-doki, hacking time!

Sergey Sharybin (sergey) updated this revision to Diff 7051.Jul 13 2016, 5:22 PM

Fixed the bug i've been experiencing.

Was playing more but couldn't find anything bad, so recon
the patch fully testable/reviewable.

Bastien Montagne (mont29) edited edge metadata.Jul 13 2016, 6:19 PM

Besides point noted in comment, lgtm.

source/blender/blenlib/intern/rand.c
297–301

Not sure about that change… Unless I’m mistaken:

  • rngarr is allocated based on BLENDER_MAX_THREADS anyway, so we are only skipping some initialization here.
  • There is no guarantee we won't have more threads running at some point than number reported by BLI_system_thread_count(), which would make some random generators use uninitialized memory.
Sergey Sharybin (sergey) added inline comments.Jul 13 2016, 6:30 PM
source/blender/blenlib/intern/rand.c
297–301

The idea is not only save memory, but save calculations.

Can you elaborate a bit more why we'll have more threads running than BLI_system_thread_count() in Blender side?

Bastien Montagne (mont29) added inline comments.Jul 13 2016, 7:44 PM
source/blender/blenlib/intern/rand.c
297–301

It’s actually pretty simple - Add a Noise texture to your default cube, and render with custom number of threads higher than BLI_system_thread_count() returned value, and BLI_rng_thread_rand() will be called with higher values of threadid than reported system count…

Anyway, I really doubt initializing 1k random numbers is anything like a serious load of calculation. ;)

Sergey Sharybin (sergey) planned changes to this revision.Jul 13 2016, 9:35 PM
Sergey Sharybin (sergey) added inline comments.
source/blender/blenlib/intern/rand.c
297–301

Argh, was yet again confused by num_threads_override.

Note for myself: num_threads_override is ONLY set when using command line argument.

Also now i see this function is only called once from creator.c, where we cant' know anything about amount of threads renderer will use later.

Will change this function to always use BLENDER_MAX_THREADS tomorrow.

Sergey Sharybin (sergey) updated this revision to Diff 7053.Jul 14 2016, 11:45 AM
Sergey Sharybin (sergey) edited edge metadata.

Fix for RNG

Bastien Montagne (mont29) accepted this revision.Jul 14 2016, 4:05 PM
Bastien Montagne (mont29) edited edge metadata.
This revision is now accepted and ready to land.Jul 14 2016, 4:05 PM
Campbell Barton (campbellbarton) accepted this revision.Jul 14 2016, 4:26 PM
Campbell Barton (campbellbarton) edited edge metadata.

LGTM, after applying there are a few cases where we use BLENDER_MAX_THREADS for stack memory that could be replaced with allocated memory, but not urgent.

Closed by commit rB12cdc67d8331: Bump maximum threads number to 1024 (authored by Sergey Sharybin (sergey)). · Explain WhyJul 15 2016, 4:54 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
source/
blender/
blenlib/
2 lines
intern/
6 lines
render/
intern/
include/
1 line
source/
11 lines
12 lines
3 lines

Diff 4358

View Options

source/blender/blenlib/BLI_threads.h

Loading...
View Options

source/blender/blenlib/intern/rand.c

Loading...
View Options

source/blender/render/intern/include/render_types.h

Loading...
View Options

source/blender/render/intern/source/occlusion.c

Loading...
View Options

source/blender/render/intern/source/rayshade.c

Loading...
View Options

source/blender/render/intern/source/render_texture.c

Loading...