Alternative Upload geometry data in parallel to multiple GPUs using the "Multi-Device" #107552

William Leeson · 2023-05-02T17:43:20+02:00

William Leeson commented

2023-05-02 17:43:20 +02:00

Why

To improve the upload of data and BVH generation for GPUs especially in the case of having multiple GPUs. As currenly all uploads happen serially which can cause a slow down.

What

This patch is an alternative to #105403 to update device memory buffers using a parallel copy in the multi-device instead of inside a parallel_for in the device_update. To do this it passes a list of buffers to the MultiDevice to copy to the devices to reduce the number of parallel_for's. This method leaves the MultiDevice to implement the parallel division of work. Doing this results in:

(a) 1 parallel_for over devices for copy mesh and attribute data to the devices
(b) n parallel_for's over devices for each object BVH to be built
(c) 1 parallel_for over devices for the scene BVH to be built

A spreadsheet here shows the profile differences between this approach and the one in #105403 alongside the original Blender timings. Some of the results are shown in the graphs below. This patch is "parallel_copy" the other is "geom_update" in the graphs.

## Why To improve the upload of data and BVH generation for GPUs especially in the case of having multiple GPUs. As currenly all uploads happen serially which can cause a slow down. ## What This patch is an alternative to #105403 to update device memory buffers using a parallel copy in the multi-device instead of inside a parallel_for in the device_update. To do this it passes a list of buffers to the MultiDevice to copy to the devices to reduce the number of parallel_for's. This method leaves the MultiDevice to implement the parallel division of work. Doing this results in: (a) 1 parallel_for over devices for copy mesh and attribute data to the devices (b) n parallel_for's over devices for each object BVH to be built (c) 1 parallel_for over devices for the scene BVH to be built A spreadsheet [here](https://docs.google.com/spreadsheets/d/1ywQym3LTLgGIGMedExxsS4RpPKwpP60BjeLtXf1BaF4/edit?usp=sharing) shows the profile differences between this approach and the one in #105403 alongside the original Blender timings. Some of the results are shown in the graphs below. This patch is "parallel_copy" the other is "geom_update" in the graphs.

Scene Update Times (original vs new vs alternative).png

18 KiB

Object BVH Update Times(original vs new vs alternative).png

19 KiB

% Improvement for Object BVH Update (original vs new vs alternative).png

22 KiB

% Improvement for Scene Update (original vs new vs alternative).png

22 KiB

Scene Update Times (original vs new vs alternative).png

Object BVH Update Times(original vs new vs alternative).png

% Improvement for Object BVH Update (original vs new vs alternative).png

% Improvement for Scene Update (original vs new vs alternative).png

William Leeson added 108 commits 2023-05-02 17:43:25 +02:00

598c7c151d Remove some locks from progress update

Removes mutex locks when updating pixel counts etc. Also, when
in background mode text status updates are removed.

31bda3d613 Create function to update progress values locklessly

a5efd8b582 Update the update lock with a lockless update

bbce8a0aae Add macro to switch from std::mutex to SpinLock

Simplified the code to be more similar to the original by
implementing it as a spin lock. Then added a macro to be able to
switch back and forth between std::mutex and the SpinLock.

72b918a9e2 Adds scoped event markers

Add scoped event markers to highlight code regions of interest.

e45b7f3a00 Merge branch 'upstream_main' into progress_update

0f5e6ed93e Refactor geometry update allow for easier parallel updates

fdd1c856e4 Add file that was forgotten

858195e683 Merge branch 'upstream_main' into geometry_update

8f0b239036 Merge branch 'upstream_main' into geometry_update

59ddc1be9e Merge branch 'upstream_main' into geometry_update

2183d70e9d Move scene BVH update into parallel_for

The scen BVH update was outside the parallel for this moves it
inside the parallel_for so it can be performed in parallel also.

5c56f518eb FIX: Preallocate timer arrays in scene

The Windows build failed as the timers were using
a dynamic array. This has been changed to use an
heap array which is allocated with the scene
instead.

cba4d732e2 Remove stats params from deviceDataXferAndBVHUpdate

The stats parameters are now contained in the Scene class and
no longer need to be passed as a parameter.

665a0e84b2 Move scne BVH upload into deviceDataXferAndBVHUpdate

To simplify the code and make it easier to read this moves the
scene BVH upload into the deviceDataXferAndBVHUpdate method.

532479e569 Move BVH2 device data update inside parallel_for

3fb101d2ba Clean up code comments

74a7a2df77 Merge branch 'upstream_main' into geometry_update

20bd8ad907 Merge branch 'upstream_main' into geometry_update

b5e12fa1cc Remove some more comments

a875bc9e45 Remove comments from cuew

0bd30b175a Merge branch 'upstream_main' into geometry_update

717b7a9b6f Remove profiling interval markers from geometry update

buildbot/vexp-code-patch-coordinator Build done. Details

ef84fe9e5d Merge branch 'upstream_main' into geometry_update

ccf7384458 Merge branch 'upstream_main' into geometry_update

042bf24409 Remove cuew changes.

fcb3bd5854 Adds comment to describe the find_matching_mem method

7bf80f2b77 Clean up code and simplify

1. Switch to use auto for DeviceScene
2. use parallel_for instead of tbb::parallel_for
3. rename n_scenes num_scenes

01e96490f1 Change params for host_mem_alloc to host_mem_alloc(size_t size, int aligment)

The parameters for host_mem_alloc were changed to
`host_mem_alloc(size_t size, int aligment) from
host_mem_alloc(size_t size, int aligment, void **p_mem)

4e1469b8bb Only build the BVH2 BVH once

Ensures that only 1 BVH2 BVH is built for each object or scene.
This is done using lock to aquire the BVH2 and other threads
can skip the building if it is for an object. However, for the
top level BVH all threads wait on the TLAS to be finished.

6795ee2f71 Only wait on condition_variable if mutex is locked.

6e0875b729 Only build a BVH2 BVH once

Basically checks if the BVH2 BVH has been built or is being built.
Then when building the top level BVH2 BVH all devices that use it
wait for the one that is building it to finish before continuing.

6618d31e12 Merge branch 'upstream_main' into geometry_update

9f149ee682 Merge branch 'upstream_main' into geometry_update

92a3b720bf Merge branch 'upstream_main' into geometry_update

c952e5d159 FIX: Fix memory leak when using more than 1 device

The mult-device set the device pointer for the device_memory as it
iterated through the devices. However, it did not restore the
original pointer to the device if the device_ptr (pointer to the
memory buffer) was 0 which it always is after a mem_free.

3a566ccee4 FIX: Using the Optix BVH for the NVidia device

Optix device was using the multi-bvh as the Optix BVH which was
causing the device to have the wrong handles for the objects.

b0345701a0 FIX:Use the headless flag to switch on/off progress updates

Don't do progress updates when in headless mode. Previously
the background flag was used but this is also used for F12 render.
Which needs the updates.

d59a30b4a0 FIX: Lock mutex before using condition variable waiting on BVH build

Mutex needs to be locked before using the condition variable can
be used. This adds a scoped lock to ensure it is locked before
waiting on the condition_variable.

b75f6ad788 FIX: Make sure top level BVH is built after all other BVHs

The top level BVH can only be built once all the other (bottom
level) BVHs are completed.

04475876d3 Merge branch 'upstream_main' into geometry_update

b9840e7640 Merge branch 'upstream_main' into geometry_update

fc30231f88 Merge branch 'upstream_main' into geometry_update

a27247e28f Refactor geometry.cpp and geometry_additions.cpp

This breaks up the geometry.cpp and geometry_additions.cpp into
the following files:
- geometry.cpp
- geometry_mesh.cpp
- geometry_attributes.cpp
- geometry_bvh.cpp
so as to organise the code into more manageable pieces.

5409cc2011 Refactor memory copy methods to remove duplication

9b3b180d23 FIX: Enable OneAPI build and fix compile issues

Needed to add the memory copy methods the have offset and size.

3186d8d725 Merge branch 'upstream_main' into geometry_update

57d7deec2a Merge branch 'geometry_update' into geometry_update_refactor

ac67371fa4 Makes sure the CPU is the last device just once at the end.

Previously after every device was added the CPU device was swopped
to the end of the list. Now the position it was added at is saved
and it is swopped with the last device at the end.

674721194e FIX: Adds case where no CPU is selected in MultiDevice

Forgot to push the code which skips swopping if the is no CPU in
the MultiDevice set.

3c81e479a6 Switch off using pinned memory on CUDA devices

Pinned memory was used for all device_memory. However, due to
that it may cause memory pressure this was removed and can be
enabled by passing USE_DEVICE_PINNED_MEMORY.

e3068b23b7 Remove only allocating memory if it needs to be enlarged.

bd08b280a4 Replace include with forward declaration of DeviceScene.

0f1e29912c Remove unused constructor

32c44f9f43 Remove only grow memory allocation.

511d878921 Use unique_ptr to handle the SubDevice and Device pointers

8731ee9761 Change swop to swap.

6e904ef6b2 Rename progressErrorCheck check_cancel_update

06b43213b7 Don't use background to disable progress update.

506106b3a8 Remove custom spin lock and replace with that from utils/thread.h

cddd2bfdf0 Replae upload and building times arrays with a vector struct

Arrays of dougle values are replaced by a vector<> of a struct
scene_times.

66a6a7a0af Clean up code

1. remove camelCase method names.
2. fix up white space changes.
3. remove some unnecessary changes.

bb2e9cc75f FIX: Size needs to be explicit as default params have been removed.

fe548c872d Merge branch 'upstream_main' into geometry_update

4920afae04 Merge branch 'geometry_update' of projects.blender.org:leesonw/blender-cluster into geometry_update

d8dcfcb89b Merge branch 'upstream_main' into geometry_update

5c9b27b374 Switch to use Cycles CUDA error checking.

b1fa18f6f8 Restore const methods and clean up code.

4ba4f7bf65 Remove commented out code

63bb671388 Restore cancel checks

29396f6227 Remove unused data elements.

99f6183d84 Merge branch 'upstream_main' into geometry_update

bbbf76c4db GeometrySizes and AttributeSizes are stored in Scene

Moved GeometrySizes and AttributeSizes to be stored in scene. This
resulted in them being removed as parameters where a Scene was
already passed in.

b1dd204d42 Moves DeviceScene related methods to the DeviceScene class

Refactored the GeometryManager methods so that those relating to
the DeviceScene are now methods in the DeviceScene.

f63c508b40 Move BVH2 device update to DeviceScene

As the device_update_bvh2 method just involves transferring data
to the device it was moved to DeviceScene.

d7cd4a4951 FIX: Add device type definitions

Moving DeviceScene from scene.h caused the device definitions to be
no longer present in various files. This add them back in by
including device.h in the required files.

buildbot/vexp-code-patch-coordinator Build done. Details

064724d14e Merge branch 'geometry_update' of projects.blender.org:leesonw/blender-cluster into geometry_update

b407dba398 FIX: Use new memory copy method that replaced the old one

Forgot to add the file for the previous change to the memory copy
methods, which resulted in the OneAPI build failing.

e69abe1659 Merge branch 'upstream_main' into geometry_update

7ffb958444 Remove only grow allocation for HIP devices

f89abb02c2 Initialise built variable in class definition.

5f0678a81b Restore get_estimated_remaining_time methods constness

5d0c87c1e4 Use info in device_memory to check to determine what data to upload

dea1a464ef Merge branch 'unbroken_main' into geometry_update

215f1267ca Allow BVH2 building to handle multiple sessions correctly

ce91384a2a Rearrange methods to match the main_reorg PR

b850a5d4b1 Merge branch 'upstream_main' into geometry_update

168366b89d Merge branch 'upstream_main' into geometry_update

d96fd9b08f Handle the bvh root index correctly for the device specific DeviceScene

Uses the unique_ptr to decallocate the BVH2 structure in the
BVH postprocess and assignes the bvh root indices in the device
specific areas also.

89f11dedda Manage sub-BVHs life cycles using unique_ptr

a126bf3b53 Revert progress related changes

0669acc5c5 Remove geometry_additions.cpp

ee8d450adc Merge branch 'upstream_main' into geometry_update

4ca1060431 Rename DeviceScene method device_free to device_free_geometry

d949741a33 FIX: Clear the device buffers after hair transparency calculations

1560ec66c9 FIX: Create CUDA context earlier and remove lock

Some CUDA commands were used before the context was setup. Also
since the BVHs are build in order there is not need for the lock
to prevent 2 being build at tehe same time to save memory.

8ba98b8138 Move displacement image upload out of geometry loop

aa24f3dd06 Merge branch 'upstream_main' into geometry_update

0af6a3efbe FIX:Add correct parameters to the OneAPI build_bvh method

58cddaeefe FIX: Upload textures to all devices

For the displacement and shadow texture generation the textures
were originally only uploaded to 1 device. However, this resulted
in some textures that were needed across all devices not being
uploaded. This fixes that by uploading them to all devices.

a916e34033 Move scene DeviceScene to use unique_ptr

To reduce the amount of code and manage the life cycles of the
DeviceScenes they are now stored as unique_ptrs

70f531fc2e Merge branch 'upstream_main' into geometry_update

e5250ab494 FIX: Remove this from lambda capture for parallel_for

df62806c94 Use task pool for object BVH build also fixes stats recording

Re-instates the task pool used to build the BVHs for the objects.
Also the displacement timing was being reported twice and this is
fixed.

2485460fb6 Merge branch 'upstream_main' into geometry_update

buildbot/vexp-code-patch-coordinator Build done. Details

429f953d6c Move CUDA context into a smaller scope

buildbot/vexp-code-patch-coordinator Build done. Details

de58c2ab8e Merge branch 'upstream_main' into geometry_update

d756933965 Create upload_changed to upload all changed buffers

William Leeson requested review from Brecht Van Lommel 2023-05-02 17:43:51 +02:00

William Leeson changed title from ~~WIP:Upload geometry data in parallel to multiple GPUs using the "Multi-Device"~~ to WIP:Alternative Upload geometry data in parallel to multiple GPUs using the "Multi-Device"

2023-05-05 16:19:37 +02:00

William Leeson added 20 commits 2023-05-09 11:15:56 +02:00

6f458619a4 Change upload_changed to take a vector of buffers to upload

66080891f2 FIX: CPU device needs to register globals.

d01c68f4b0 FIX: program_name should be name only broken on window

c9b8ee956d FIX: Don't upload buffers with 0 size

a87d9843d7 FIX: Reinstate lock and context for build_optix_bvh

df0fba0d7d FIX: Upload buffer if data_size is not zero

This was incorrectly set to be device_size which is not set until
it is allocated on the device.

d536ad206a Merge branch 'upstream_main' into wip_branch

3cec5c199e Move buffer list to Scene

9f146557f6 Move copy to device outside parallel_for

8f50b28fa8 Move device_memory list into DeviceScene

e04384f20d FIX: get_device_bvh returns the MultiBVH if the device is the MultiDevice

If the device was the MultiDevice it previously returned the first
BVH. This was incorrect it should return the MultiBVH. Also, if
it cannot find the device then it should return NULL.

b1be09d449 Move the BVH layout determination into a method for reuse

This code was duplicated in MultiDevice and geometry_bvh.cpp so
this removed that duplication.

ab21849d86 Remove the parallel_for over devices

The parallel_for for is replaced by many parallel_for's one for
the upload, one for each Object BVH and one for the scene BVH.

a4b3b8e1fe Add missed changes to select the correct layout.

565bc75769 FIX: get_device_bvh returns the MultiBVH if the device is the MultiDevice

If the device was the MultiDevice it previously returned the first
BVH. This was incorrect it should return the MultiBVH. Also, if
it cannot find the device then it should return NULL.

df7d92a33e FIX: Size used should be the data_size

ed1b971948 FIX: Use memory_size and upload_changed in the correct places

72be875f3c FIX: BVH layout determination is the same across all devices.

b7a80ba6ef FIX: Enable local intersection testing on BVH

The scen bvh layout was set to NONE which disabled the local
intersection testing used for he bevel effect. This removes that
line and also refactors the code to make it cleaner.

buildbot/vexp-code-patch-coordinator Build done. Details

f80ef452a1 Merge branch 'remove_parallel_for' into upload_changed

William Leeson changed title from ~~WIP:Alternative Upload geometry data in parallel to multiple GPUs using the "Multi-Device"~~ to Alternative Upload geometry data in parallel to multiple GPUs using the "Multi-Device"

2023-05-09 11:35:02 +02:00

William Leeson referenced this pull request

2023-05-09 11:39:17 +02:00

Refactor geometry update for better parallelization and upload performance #105403

William Leeson commented

2023-05-09 11:41:48 +02:00

@blender-bot package

William Leeson closed this pull request

2023-05-09 11:41:49 +02:00

Blender Bot commented

2023-05-09 11:41:51 +02:00

Package build started. Download here when ready.