WIP:Interleaved slices for better work distriubtion with a Multi-GPU setup #110348
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#110348
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "leesonw/blender-cluster:work_sets_similar"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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 "interleaved slices". This splits the work load by giving each device a set of scan lines such that the one with the smallest weight
w_{smallest}
gets 1 scan line and the others getn_i = w_i/w_{smallest}
. The scaliness for each device are interleaved such that the first device takes the firstn_0
and the second gets the nextn_1
+ }$ and so on. The set of scan lines are reassigned each time the weights change.This pull request is based on that in 108147 I could not find a way to change the branch being merged from so I started a new one.
I put some performance statistics here
WIP:Work sets for better work distrubtion with a Multi-GPU setupto WIP:Interleaved slices for better work distriubtion with a Multi-GPU setup@blender-bot package
Package build started. Download here when ready.
Did an initial pass of review. Nothing big yet, but I am still trying to figure out what would be a good way to see how close to the ideal scaling we are. It is a bit hard to measure speedup on dual GP100 setup we currently have here. Need to think a bit of what is the worst case scenario for the current algorithm to get a good performance comparison.
From the testing it seems that if you start viewport render, then enable OptiX denoiser the Blender stalls. Can reproduce reliably with this PR, but not with the current buildbot build of the main branch.
What I am not sure about is how this patch deals with the adaptive sampling. Does the every slice have overscan around it? Or are there "gaps" (information-wise) in the scanlines rendered on a particular device? The former would mean quite high memory usage, the latter would mean the result is not converging to the same result as when rendering on a single device. Or something completely different going on and the result converges to the same result no matter how many devices you're rendering on?
For some reason I can not get Samples Count pass to work on the test machine and pabellon file, I'll dig into it later. But think comparing this pass will be a good measure to see whether the patch converges the same way on multi-device as it does on a single device.
@ -978,6 +978,12 @@ class CyclesRenderSettings(bpy.types.PropertyGroup):
min=8, max=8192,
)
interleaved_slices: BoolProperty(
Outside of the development purposes I don't think this should be an option.
If we plan to have it for the future development/investigation better move to the debug panel.
Otherwise perhaps just remove the option.
@ -46,3 +46,3 @@
destination.num_components;
parallel_for(0, buffer_params.window_height, [&](int64_t y) {
/* Calculate how many full plus partial slices there are */
Fullstops in the comments.
@ -49,0 +65,4 @@
/* Copy over each slice to the destination */
parallel_for(0, slices, [&](int slice) {
//for(int slice = 0;slice < slices;++slice) {
Remove the dead code. Also seems that the clang-format is not properly configured in your setup.
@ -254,0 +298,4 @@
slice_sizes[largest_weight] += leftover_scanlines;
slice_stride++;
} else if(leftover_scanlines < 0) {
VLOG_WARNING << "#######Used to many scanlines";
Not sure about details, but seems that
VLOG_WARNING << "Used to many scanlines"
will be much better suit here.If this is something that must get a developer attention can use
DCHECK_GE(leftover_scanlines, 0)
above.@ -254,0 +301,4 @@
VLOG_WARNING << "#######Used to many scanlines";
}
VLOG_INFO << "===================SLICE allocatable:" << allocatable_slices << " fixed:"<< fixed_slices << "================";
Not sure I'd keep such explicit separation. Surely, it is important for this PR, but outside of this PR it feels it'll draw too much attention.
@ -412,2 +463,4 @@
});
const double work_time = time_dt() - start_time;
VLOG(3) << "render time total for frame: "
I don't think it is a total time. It is a path tracing time for the current work. There might be more path tracing needing to be done, and there are other types of work as well.
Perhaps it will be much better to log "bare" time from the
RenderScheduler::report_*_time()
functions.@ -78,2 +78,4 @@
kg, state, render_buffer, scheduled_sample, tile->sample_offset);
/* Map the buffer coordinates to the image coordinates */
int tile_y = y - tile->slice_start_y;
Should the same mapping be done for the
init_from_bake
?Still not sure what's up with the Samples Count pass in the Pabellon file, but it works fine in the Junkshop, so that is what I've used for verification.
Unfortunately, the convergence is indeed different with this patch.
Attached the Samples Count pass with the single device render and dual-GPU render. There are noticeable stripe artifacts in the samples distribution.
To reproduce the issue get the Junkshop benchmark scene, enable adaptive samples, Samples Count pass, and render.
In the current main branch both single and multi-GPU render converges to the same result, and their Sample Count pass matches. This is done by applying some overscan around the slices, so that the adaptive sampling filter has enough data to propagate to/from.
Doing so for inetrleaved case would be rather very costly. Not sure what the best solution would be. The quick idea is to perform filtering on a single device. This would require copying pixels from/to multiple devices, but we can limit it to a single required pass only, and doing so every N samples is probably not so bad. Maybe can also utilize half-float, but that could make it harder to deal with the over-exposed areas.
Thanks for taking a look at this.
This would kind of make sense as the interleaved scan lines would have difference noise and therefore convergence. The same issue would probably appear in the previous division method but only along the single line of division.
Copying between would probably cause a large slow down. I'll take a look at that code and see what I can come up with.
It shouldn't. This is what the overscan for the render result is meant to solve. While it is theoretically possible that propagation will affect more than the current overscan size, but we did not see such practical scene, yet alone big difference from the single device rendering.
I think by default filtering is only happening every 16 samples, which isn't that often. Also, on the one hand we'll be copying a single pass across devices to do filtering, but on the other hand we'll be able to disable overscan.
So to be it is not that obvious that there will be a slowdown.
One thing I've realized is that the render buffers are storing passes as interleaved pixels, which seems it'll make it harder to copy a single pass.
I kind of curious if it will make sense to store them non-interleaved. Could help running denoising tasks, and maybe will help path tracing performance as well?
@leesonw One more thing. The change to switch from oevrscan to filtering on a single device would need to be submitted in a separate PR.
I was giving the change a deeper check and look into the code.
For the performance checks I used a slightly modified Victor scene where I've hit Victor and Frank, to force the worst scenario for the current algorithm where the top half of the scene is trivial. I've also increased resolution to 150% to ensure both GPUs have enough work to do.
The scalability of the current algorithm is somewhere between 3% to 10% behind the ideal scaling (as in, rendering on 2 GPUs is not really 2x speedup, but more like 1.9-ish). The proposed algorithm was always within a couple of percent from the ideal scaling. And I guess the benefit only gets better with more GPUs.
The code I found a bit tricky to follow. I think the biggest issue is that I am now not really sure about semantic meaning of fields in the
RenderBuffer
. Think once we make those clear the rest of the code will be easier to understand, modify if needed, and maintain in the future.For the
width
andheight
I think it is fair to state that they are/* Width/height of the allocated buffer. NOTE: It does not necessarily translate to a continuous region within a camera space. */
. Something like this, but point being: make it clear that it is not really a "continuous region" within a camera space.The window parameters I am not sure if we'll need in the longer term. Currently it only seems to be used for adaptive sampling, and to get adaptive sampling to wrk with such interleaved scanlines a different solution is needed, as mentioned above. If we can have different algorithm for adaptive sampling filtering which does not require overscan and works with internleaved scanlines that will simplify this part of the render buffer and related scheduler logic.
The
full_{x,y}
is where things becomes quite fuzzy to me.If i remember things correctly, currently for multi GPU rendering the
full_y
is offset for every device. So in a way,. RenderBuffers is configured similarly to a border render. Think this is an easy way to think about it.But with interleaving scanlines I am not sure what those offsets intuitively mean. Are all devices have render buffers with the same offset to the full buffer, and the way to think of it is that it is an offset within the full buffer calculated prior slicing the result to be passed to individual devices (which will make it simple like: typically, just a border render information)?
This needs to be clarified in the comment of those fields.
The
slice_stride
andslice_height
is also something I don't fully understand:slice
exactly is? How slice is different from a "smaller" RenderBuffer (smaller width, height, offset using full_x, full_y) ? In a way it feels that the extra information we need to store is more about "interleaving" rather than "slicing".slice_height
different fromheight
?slice_stride
is measured in? When talking about stride it typically helps defining it in a terms of "number of between two consequitive scanlines".Not sure I understood the meaning of them correctly, so the following suggestion might be a bit wrong. Anyway, but I'd try to define this part of the
RenderBuffers
I'd do something like the following:Could elaborate further to make it more explicit how the scanlines are stored in the buffer itself. Also, I didn't go into explanation of
full_x
/full_y
as I am not yet sure what they mean for interleaved scheduling yet.In any case, I hope it gives a good starting point for clarifying the
RenderBuffers
structure.Checkout
From your project repository, check out a new branch and test the changes.