Compositor: Enable lock-free GPU context activation on macOS #118920

Sergey Sharybin · 2024-02-29T18:25:49+01:00

Sergey Sharybin commented

2024-02-29 18:25:49 +01:00

This required to apply a small fix in the Metal texture uploader.

Without synchronization pixels of a wrong pass can be uploaded to
a wrong texture. This is because this code path is heavily reusing
temporary allocations, and at some point the allocation is not
considered as still in use, unless the command buffer used by the
texture uploader is submitted.

Ref #118919

This required to apply a small fix in the Metal texture uploader. Without synchronization pixels of a wrong pass can be uploaded to a wrong texture. This is because this code path is heavily reusing temporary allocations, and at some point the allocation is not considered as still in use, unless the command buffer used by the texture uploader is submitted. Ref #118919

Sergey Sharybin added 1 commit 2024-02-29 18:25:58 +01:00

Compositor: Enable lock-free GPU context activation on macOS fcbfba9515

This required to apply a small fix in the Metal texture uploader.

Without synchronization pixels of a wrong pass can be uploaded to
a wrong texture. This is because this code path is heavily reusing
temporary allocations, and at some point the allocation is not
considered as still in use, unless the command buffer used by the
texture uploader is submitted.

Ref #118919

Sergey Sharybin requested review from Clément Foucault 2024-02-29 18:26:10 +01:00

Aras Pranckevicius reviewed 2024-03-01 08:18:57 +01:00

source/blender/gpu/metal/mtl_texture.mm

						
				@ -1125,2 +1125,4 @@

				    ctx->main_command_buffer.submit(false);

				    /* Release temporary staging buffer allocation.

				     * NOTE: Allocation will be tracked with command submission and released once no longer in use.

Aras Pranckevicius commented

2024-03-01 08:18:57 +01:00

The comment wrt command buffer submission is probably outdated now, given that the cmdbuf was just submitted right above?

Sergey Sharybin commented

2024-03-01 09:48:59 +01:00

I do not think it is outdated. You submit command, and it tracks allocation, so it is safe to free memory here. So you need to submit in order to make the allocation tracked.

Michael Parkin-White commented

2024-03-03 04:19:44 +01:00

First-time contributor

FYI on this, texture::update_sub is called quite a lot, and having a command buffer flush here may negatively impact performance. It will also prevent multiple subsequent texture updates being batched together and optimized by the driver/GPU workload scheduler.

However, given the cause of the original issue, if this is fixing a missing GPU_flush, then we should ideally be able to either insert that at the high level in the problem code, or look into why this was happening in the first place. i.e. perhaps a later function requires a flush which is not happening.

Regarding the memory, this should not matter either way as if the buffer is used by any command buffer, the command buffer instance will retain a reference to any used memory. Freeing the buffer reference on host can be done safely in any case.

I can have a look into this however, if this does indeed resolve the issue.

FYI on this, texture::update_sub is called quite a lot, and having a command buffer flush here may negatively impact performance. It will also prevent multiple subsequent texture updates being batched together and optimized by the driver/GPU workload scheduler. However, given the cause of the original issue, if this is fixing a missing GPU_flush, then we should ideally be able to either insert that at the high level in the problem code, or look into why this was happening in the first place. i.e. perhaps a later function requires a flush which is not happening. Regarding the memory, this should not matter either way as if the buffer is used by any command buffer, the command buffer instance will retain a reference to any used memory. Freeing the buffer reference on host can be done safely in any case. I can have a look into this however, if this does indeed resolve the issue.

Clément Foucault approved these changes 2024-03-01 12:38:22 +01:00

Clément Foucault left a comment

I approve but this might backfire at some point. Just tagging @Michael-Parkin-White-Apple

Sergey Sharybin merged commit 3fcd7ccbc0 into main

2024-03-01 14:38:21 +01:00

Sergey Sharybin referenced this issue from a commit

2024-03-01 14:38:22 +01:00

Compositor: Enable lock-free GPU context activation on macOS

Sergey Sharybin deleted branch compositor_gpu_metal

2024-03-01 14:38:24 +01:00

Sergey Sharybin commented

2024-03-01 14:41:23 +01:00

@Michael-Parkin-White-Apple In case you'd looking into deeper details, the file to reproduce the original issue can be found in #118919. You don't need to modify code anymore to make the GPU compositor lock-free. Just Ctrl-Click on the image input node to plug the viewer and pan the editor to force some ascynhronous GPU use.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Compositor: Enable lock-free GPU context activation on macOS #118920