WIP:Work sets for better work distrubtion with a Multi-GPU setup #108147

William Leeson · 2023-05-22T14:28:00+02:00

William Leeson commented

2023-05-22 14:28:00 +02:00

Why

To improve the distribution of work between GPUs by giving them a set of work that should take roughly the same amount of time

What

This patch adds an option to the cycles performance category called "device scale factor" that determines how many vertical splits occur in a render tile or image if tiling is not used. This results in the tiles or images being broken into n parts each of which is given to a different device. The image tile are split by giving every 1/s*w_{device} scan lines in the tile to a each GPU. Where s is the "device scale factor" an integer value and w_{device} the proportion of work to assign to the device.. This way they all render a similar image which results in each work set taking a similar amount of time. So for instance at the beginning for 2 GPU the weight would be equal at 0.5 each. Thus for a 256x256 tile each would get 128 scan lines.

Added some performance profiles here. I also exported some of the graphs below note I need to redo "attic gpu map" as it was done with NVTX markers which slowed it down.

## Why To improve the distribution of work between GPUs by giving them a set of work that should take roughly the same amount of time ## What This patch adds an option to the cycles performance category called "device scale factor" that determines how many vertical splits occur in a render tile or image if tiling is not used. This results in the tiles or images being broken into $n$ parts each of which is given to a different device. The image tile are split by giving every $1/s*w_{device}$ scan lines in the tile to a each GPU. Where s is the "device scale factor" an integer value and $w_{device}$ the proportion of work to assign to the device.. This way they all render a similar image which results in each work set taking a similar amount of time. So for instance at the beginning for 2 GPU the weight would be equal at 0.5 each. Thus for a 256x256 tile each would get 128 scan lines. Added some performance profiles [here](https://docs.google.com/spreadsheets/d/1Hqg7vJ2C2aIdSvizWxymoeqZTvRZGWMV1LIdDMFyUBE/edit?usp=sharing). I also exported some of the graphs below note I need to redo "attic gpu map" as it was done with NVTX markers which slowed it down.

Rebalance Times.png

29 KiB

Path Trace Times.png

26 KiB

👀 2 🎉 2

William Leeson added 12 commits 2023-05-22 14:28:06 +02:00

127066dd99 Add work_sets for better multi-gpu scaling

4e4d5f3ea0 Only realloc the buffer if it is bigger

c5da8e6429 Request transfers for all slices then sync only once

84fa6cfc51 Merge branch 'upstream_main' into work_sets

d21cd0bc2e Merge branch 'upstream_main' into work_sets

edfb257495 Add the ability to allocate pinned memory

b8fbf9dfcb Allocate path trace counters in pinned memory to allow bg transfer

5f5595caa7 Allocate render buffers as pinned so as to be able to bg transfer

790e12a444 FIX: Adds missing device import for MacOS bvh.mm

9c7f5266a6 Merge branch 'upstream_main' into work_sets

4c1d196af8 Merge branch 'upstream_main' into work_sets

d8bd1c1eb8 Merge branch 'upstream_main' into work_sets

William Leeson added 9 commits 2023-05-25 10:25:29 +02:00

8c4db51deb FIX: Don't read kernel data for the KernelLightTreeEmitter if the index is -1

cbdb1379e2 Use a single master buffer to hold all the slices

This replaces n slice buffers with a single master buffer and n
slices which reference into the master buffer. This allows a
single copy to upload or download the data.

9565aaa6fe FIX: Denoise buffer when more than one work_set

Previouly when there were more than one worker (aka device) and
multiple work_sets the denoising was skipped.

6050b49ee4 FIX: Use remaining rows in the last work item

The remain rows were not added to the last work item as it was
detected incorrectly.

47e1cd75cb Merge branch 'upstream_main' into work_sets

7a3135ec98 FIX: Set pinned to false when not allocating pinned memory

f0185ed234 FIX: Don't copy 0 height slices to display

Attempting to copy a zero height slice resulted in CUDA errors.
Also switches back to using the master buffer when zeroing all
the slices.

bd669026e3 Skip over 0 height slices

76b0775361 Clean up code

William Leeson added 4 commits 2023-05-30 13:47:39 +02:00

0a716b2f68 Put wavefront tracing counters in pinned memory.

6a73c7d345 Merge branch 'upstream_main' into work_sets

ef5d66e1bf FIX: Buffers must always be cleared

2ad2d46aeb Removes performance penalty due to rebalance for having slices.

William Leeson added 2 commits 2023-05-31 13:32:28 +02:00

f283a9beb4 Merge branch 'upstream_main' into work_sets

0e0056707e FIX: Master buffer is now copied directly using it's buffer

CPU data was not being copied properly due to the buffer not
having the correct parameters.

Brecht Van Lommel commented

2023-06-02 14:03:47 +02:00

I think this is not implemented at the right level (but maybe that is planned as a further step already?). In this PR a device seems to fully complete rendering a slice before moving on to the next slice. But then as you increase the number of slice there is more overhead from synchronizing each time. And probably worse, GPU occupancy and coherence will drop as there is less work to keep all cores busy, especially near the end when fewer and fewer paths remain.

For example imagine a shot with a character where it's hair is the most expensive to render, but takes up only a small part of the entire image. Getting the slices small enough to distribute that works as more GPUs are added is going to require a high device scale factor, probably with too much overhead? So you'd want to be able to make that really granular, maybe even just one scanline per device.

It should be rendering pixels from all slices simultaneously, all in the same path state array. I would expect kernel/device/gpu/work_stealing.h to have the logic to map a global work index to a slice and coordinate in the slice.

I think every device should have just one render buffer to work with that, not multiple. The device_memory.slice mechanism I'm not sure will work well for this, if you do path tracing for multiple slices at once, you would pass it just a single render buffer device pointer and this abstraction can't really be used to hide that.

I think this is not implemented at the right level (but maybe that is planned as a further step already?). In this PR a device seems to fully complete rendering a slice before moving on to the next slice. But then as you increase the number of slice there is more overhead from synchronizing each time. And probably worse, GPU occupancy and coherence will drop as there is less work to keep all cores busy, especially near the end when fewer and fewer paths remain. For example imagine a shot with a character where it's hair is the most expensive to render, but takes up only a small part of the entire image. Getting the slices small enough to distribute that works as more GPUs are added is going to require a high device scale factor, probably with too much overhead? So you'd want to be able to make that really granular, maybe even just one scanline per device. It should be rendering pixels from all slices simultaneously, all in the same path state array. I would expect `kernel/device/gpu/work_stealing.h` to have the logic to map a global work index to a slice and coordinate in the slice. I think every device should have just one render buffer to work with that, not multiple. The `device_memory.slice` mechanism I'm not sure will work well for this, if you do path tracing for multiple slices at once, you would pass it just a single render buffer device pointer and this abstraction can't really be used to hide that.

Brecht Van Lommel requested changes 2023-06-02 14:03:58 +02:00

intern/cycles/blender/addon/properties.py Outdated

						
				@ -973,6 +973,13 @@ class CyclesRenderSettings(bpy.types.PropertyGroup):

				        min=8, max=8192,

				    )

				    device_scale_factor: IntProperty(

Brecht Van Lommel commented

2023-06-02 13:00:51 +02:00

This may be ok for debugging, but I don't think the final implementation should require tuning a parameter like this.

intern/cycles/device/device.cpp Outdated

						
				@ -435,6 +435,17 @@ void Device::free_memory()

				  metal_devices.free_memory();

				}

				bool Device::alloc_host(void *&shared_pointer, size_t size, bool /* pinned */) {

Brecht Van Lommel commented

2023-06-02 13:02:31 +02:00

Please use clang-format.

intern/cycles/device/memory.cpp Outdated

						
				@ -45,2 +46,3 @@

				  void *ptr = util_aligned_malloc(size, MIN_ALIGNMENT_CPU_DATA_TYPES);

				  void *ptr = NULL;

				  if(!pinned_mem) {

Brecht Van Lommel commented

2023-06-02 13:19:50 +02:00

I'm confused by what non-pinned memory means now, since here it's just a regular memory allocation, but in the CUDA device it allocates with the CUDA API.

intern/cycles/kernel/light/tree.h Outdated

						
				@ -630,3 +620,1 @@

				    if (knode->type == LIGHT_TREE_INSTANCE) {

				      /* Switch to the node with the subtree. */

				      *node_index = knode->instance.reference;

				  if (selected_index != -1) {

Brecht Van Lommel commented

2023-06-02 12:59:48 +02:00

Unrelated change, leave out.

William Leeson commented

2023-06-02 14:55:24 +02:00

I think this is not implemented at the right level (but maybe that is planned as a further step already?). In this PR a device seems to fully complete rendering a slice before moving on to the next slice. But then as you increase the number of slice there is more overhead from synchronizing each time. And probably worse, GPU occupancy and coherence will drop as there is less work to keep all cores busy, especially near the end when fewer and fewer paths remain.

For example imagine a shot with a character where it's hair is the most expensive to render, but takes up only a small part of the entire image. Getting the slices small enough to distribute that works as more GPUs are added is going to require a high device scale factor, probably with too much overhead? So you'd want to be able to make that really granular, maybe even just one scanline per device.

It should be rendering pixels from all slices simultaneously, all in the same path state array. I would expect kernel/device/gpu/work_stealing.h to have the logic to map a global work index to a slice and coordinate in the slice.

I think every device should have just one render buffer to work with that, not multiple. The device_memory.slice mechanism I'm not sure will work well for this, if you do path tracing for multiple slices at once, you would pass it just a single render buffer device pointer and this abstraction can't really be used to hide that.

Yes, I totally agree. I am working on a change that will just use one render buffer and render all the slices into it. That way like you say then you don't need to restart the path tracing for each slice. I was doing this another way but now that you point it out it's probably better to use kernel/device/gpu/work_stealing.h to do the mapping instead of the slices and work_sets. I have noticed that many of the wins from this are a result of the rebalance happening less often and are quicker as the slices are smaller so the copying to and from the GPUs is way less expensive. This is particularly important with setups that have many GPUs (10 in our case where it really made a difference)

> I think this is not implemented at the right level (but maybe that is planned as a further step already?). In this PR a device seems to fully complete rendering a slice before moving on to the next slice. But then as you increase the number of slice there is more overhead from synchronizing each time. And probably worse, GPU occupancy and coherence will drop as there is less work to keep all cores busy, especially near the end when fewer and fewer paths remain. > > For example imagine a shot with a character where it's hair is the most expensive to render, but takes up only a small part of the entire image. Getting the slices small enough to distribute that works as more GPUs are added is going to require a high device scale factor, probably with too much overhead? So you'd want to be able to make that really granular, maybe even just one scanline per device. > > It should be rendering pixels from all slices simultaneously, all in the same path state array. I would expect `kernel/device/gpu/work_stealing.h` to have the logic to map a global work index to a slice and coordinate in the slice. > > I think every device should have just one render buffer to work with that, not multiple. The `device_memory.slice` mechanism I'm not sure will work well for this, if you do path tracing for multiple slices at once, you would pass it just a single render buffer device pointer and this abstraction can't really be used to hide that. Yes, I totally agree. I am working on a change that will just use one render buffer and render all the slices into it. That way like you say then you don't need to restart the path tracing for each slice. I was doing this another way but now that you point it out it's probably better to use `kernel/device/gpu/work_stealing.h` to do the mapping instead of the slices and `work_sets`. I have noticed that many of the wins from this are a result of the `rebalance` happening less often and are quicker as the slices are smaller so the copying to and from the GPUs is way less expensive. This is particularly important with setups that have many GPUs (10 in our case where it really made a difference)

William Leeson added 7 commits 2023-06-09 10:42:33 +02:00

69e04ddeb3 Merge branch 'upstream_main' into work_sets

35335e4932 Adds NVTX markers for viewing program flow and execution

098353947d Add some markers to visualise the render_pipeline

c836ee5e99 Merge branch 'upstream_main' into work_sets

2bbf552f09 Render all slices in one go

Previously the render_samples iterated over all the WorkSets.
However, this was not ideal due to overheads and was not good at
keeping the GPU busy. Now info is passed in the WorkTile to enable
the GPU to render all the slices in one pass.

7d1379f95d Path rng_hash uses image pixel coordinates

Previously it was using the slice coordinates

cf298a1d7e FIX: Update slice buffer offest into the master buffer on change

The slice buffers offsets into the master buffer were only updated
when the master buffer was reallocated. This ignored that fact that
the resolution scaler could resize the buffer and the slices even
though the master buffer was not reallocated.

William Leeson added 12 commits 2023-06-13 16:15:39 +02:00

7460500a81 FIX: RenderBuffers state now setup correctly for NODE

The parameters for slices added to the RenderBuffers was not setup
correctly for use with Nodes. This adds the necessary setup code.

Also, switched the render buffer to not use pinned memory.

ebfddd1c1a Merge branch 'upstream_main' into work_sets

67116b0844 FIX: Fixes debug build

The debug build was failing because dna_type_offsets.h was not
always generated when building. This also sometimes more rarely
affected the release build.

d463f4d194 Pre-calculate master buffer size

7d0cb956d4 Use slices buffer params to copy data to render buffers

240cad9fb0 Make Copy from render buffers use slice buffer params

4b4fbf6ec0 Clean up code

7c4f3aa0d4 Remove the need for using the WorkSet size()

Adds device_scale_factor_ member variable to PathTraceWork for
iterating over the slices.

7809f5e275 Use a single kernel launch call to perform the adaptive sampling with slices

7f11e5f38d Cryptomatte uses only 1 kernel launch on GPU

d9442b3969 Adaptive sampling uses only one parallel_for on CPU

Also fixes an issue where the render_samples_impl was getting the
incorrect height.

9ba33c795d Remove workset from denoise update

William Leeson added 1 commit 2023-06-26 12:20:53 +02:00

295652a1b9 Merge branch 'upstream_main' into work_sets

William Leeson added 1 commit 2023-06-27 14:25:34 +02:00

481d1c4423 FIX: MacOS compiler cannot initialise dynamic arrays

Replace initializer with for loop.

William Leeson added 1 commit 2023-06-29 18:35:03 +02:00

4e424d384f Make PassAssessor and master_buffers_ aware of slice structure

Adds the correct BufferParams to the master_buffers and also
changes the code to the PathAccessors for bot CPU and GPU to copy
the images according to the slice structure.

William Leeson added 1 commit 2023-06-30 14:32:06 +02:00

dcb9476e9c FIX: Stop (get/set)_render_tile_pixels using work_sets

Sets the master_buffers as the effective_buffer_params so they
only iterate once instead of per buffer. As the device_pointers
cannot be used as regular pointers.

William Leeson added 1 commit 2023-07-04 16:30:55 +02:00

3203ca5c19 Merge branch 'upstream_main' into work_sets

William Leeson referenced this pull request from leesonw/blender-cluster

2023-07-21 15:39:16 +02:00

WIP:Interleaved slices for better work distriubtion with a Multi-GPU/CPU setup #4

William Leeson referenced this pull request

2023-07-21 15:43:05 +02:00

WIP:Interleaved slices for better work distriubtion with a Multi-GPU setup #110348

YimingWu referenced this pull request

2023-07-27 16:47:56 +02:00

Blender not taking advantage of all gpus #110531

This pull request has changes conflicting with the target branch.