Fix #114742: Draw: Buffers never shrink #114857

Miguel Pozo · 2023-11-14T18:49:49+01:00

Miguel Pozo commented

2023-11-14 18:49:49 +01:00

The buffers from the new Draw Manager increase their size as needed, but they never shrink.

This adds a new optimize_size function to StorageArrayBuffer that can downsize the buffer following the same heuristic as get_or_resize. And applies it automatically on StorageVectorBuffer::clear().

Notes:
It may be best to use StorageVectorBuffer where possible, so this can be handled in a more automatic and error-proof way.

The way this is handled may cause a performance loss if the number of objects is always changing in the scene in a way that causes resizes every frame. It may be best to leave more room for max_size even if that means increasing memory use?
For example: size_t max_size = power_of_2_max_u(required_size * 2);

Also, in the case of StorageVectorBuffer::push_update() we should only push up to item_len_. But that may be best left for 4.1.

The buffers from the new Draw Manager increase their size as needed, but they never shrink. This adds a new `optimize_size` function to `StorageArrayBuffer` that can downsize the buffer following the same heuristic as `get_or_resize`. And applies it automatically on `StorageVectorBuffer::clear()`. Notes: It may be best to use `StorageVectorBuffer` where possible, so this can be handled in a more automatic and error-proof way. The way this is handled may cause a performance loss if the number of objects is always changing in the scene in a way that causes resizes every frame. It may be best to leave more room for `max_size` even if that means increasing memory use? For example: `size_t max_size = power_of_2_max_u(required_size * 2);` Also, in the case of `StorageVectorBuffer::push_update()` we should only push up to `item_len_`. But that may be best left for 4.1.

Miguel Pozo added this to the 4.0 milestone 2023-11-14 18:49:49 +01:00

Miguel Pozo added the

Module

EEVEE & Viewport

label 2023-11-14 18:49:49 +01:00

Miguel Pozo requested review from Clément Foucault 2023-11-14 18:50:24 +01:00

Miguel Pozo requested review from Jeroen Bakker 2023-11-14 18:50:34 +01:00

Miguel Pozo force-pushed pull-draw-memory-housekeeping from 2c856692d1 to 4b1defa78d

2023-11-14 18:52:19 +01:00

Compare

Miguel Pozo commented

2023-11-14 19:02:48 +01:00

@blender-bot package

Blender Bot commented

2023-11-14 19:02:51 +01:00

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR114857) when ready.

Miguel Pozo referenced this pull request

2023-11-14 19:04:31 +01:00

Regression: Lag and high memory usage from high amount of instances persists after removing those instances #114742

Clément Foucault requested changes 2023-11-14 20:02:52 +01:00

Clément Foucault left a comment

I think the heuristic would be to use a hysteresis threshold in the draw manager itself since the counts are stored there.

Usually, try following what the BLI Vector API does in terms of convention.

We cannot use StorageVectorBuffer since it would mean all items in it are stored contiguously which might not the case of most of the draw manager buffers (some buffers have holes IIRC).

I think the heuristic would be to use a hysteresis threshold in the draw manager itself since the counts are stored there. Usually, try following what the BLI Vector API does in terms of convention. We cannot use `StorageVectorBuffer` since it would mean all items in it are stored contiguously which might not the case of most of the draw manager buffers (some buffers have holes IIRC).

source/blender/draw/intern/DRW_gpu_wrapper.hh Outdated

						
				@ -367,2 +367,4 @@

				  }

				  /* Downsize the buffer if current size is much larger than the required size. */

				  void optimize_size(int64_t required_size)

Clément Foucault commented

2023-11-14 19:54:21 +01:00

Rename to resize_to_next_power_of_2(int64_t new_size).
Don't forget to remove the check. There is already a check in resize() if size doesn't change.

Rename to `resize_to_next_power_of_2(int64_t new_size)`. Don't forget to remove the check. There is already a check in `resize()` if size doesn't change.

Miguel Pozo commented

2023-11-15 15:24:44 +01:00

But that could actually grow the buffer if it was resized manually.
Also, I think we may want to eventually use a different heuristic.
At some point, twice the size can be just too much.

But that could actually grow the buffer if it was resized manually. Also, I think we may want to eventually use a different heuristic. At some point, twice the size can be just too much.

Clément Foucault commented

2023-11-17 16:20:38 +01:00

If the user set the size directly then it there's no incentive to use optimize_size. The resize_to_next_power_of_2 is just to use the same heuristic as get_or_resize.

I think we could make it linear after a certain point, but that would require more testing in order to judge what are the best values.

If the user set the size directly then it there's no incentive to use `optimize_size`. The `resize_to_next_power_of_2` is just to use the same heuristic as `get_or_resize`. I think we could make it linear after a certain point, but that would require more testing in order to judge what are the best values.

source/blender/draw/intern/DRW_gpu_wrapper.hh Outdated

						
				@ -404,0 +412,4 @@

				   * Set item count to zero.

				   * If do_optimize_size is true, it may resize the buffer,

				   * but never to a capacity less than the current count.

				   * Otherwise it wont not free memory or resize the buffer.

Clément Foucault commented

2023-11-14 19:07:50 +01:00

wont not

Double negation.

> wont not Double negation.

pragma37 marked this conversation as resolved

source/blender/draw/intern/DRW_gpu_wrapper.hh Outdated

						
				@ -404,2 +415,3 @@

				   * Otherwise it wont not free memory or resize the buffer.

				   */

				  void clear()

				  void clear(bool do_optimize_size = true)

Clément Foucault commented

2023-11-14 20:02:34 +01:00

Use clear_and_trim() for function name. Split to another function for better semantic and to avoid bugs caused by the change of behavior (but does not free memory or resize the buffer is vastly different from it may resize the buffer).

Change the usage on a per case basis (in separate commits to allow bisecting) and validate it works fine.

Use `clear_and_trim()` for function name. Split to another function for better semantic and to avoid bugs caused by the change of behavior (`but does not free memory or resize the buffer` is vastly different from `it may resize the buffer`). Change the usage on a per case basis (in separate commits to allow bisecting) and validate it works fine.

pragma37 marked this conversation as resolved

source/blender/draw/intern/draw_command.hh Outdated

						
				@ -543,2 +546,4 @@

				  void clear()

				  {

				    group_buf_.optimize_size(group_count_);

				    command_buf_.optimize_size(group_count_ * 2);

Clément Foucault commented

2023-11-14 19:42:08 +01:00

This group_count_ * 2 is not explained and not obvious.

This `group_count_ * 2` is not explained and not obvious.

pragma37 marked this conversation as resolved

Miguel Pozo changed title from ~~Fix #114742: Draw: Buffers never shrink~~ to Fix #114742: Draw: Buffers never shrink

2023-11-17 17:27:11 +01:00

pragma37 changed target branch from blender-v4.0-release to main

2023-11-17 17:27:17 +01:00

Miguel Pozo force-pushed pull-draw-memory-housekeeping from 90b9c45e58 to 1a775040a8

2023-11-17 17:30:18 +01:00

Compare

Clément Foucault approved these changes 2023-11-17 23:05:38 +01:00

Miguel Pozo merged commit 4dc1c23384 into main

2023-11-20 12:23:22 +01:00

Miguel Pozo deleted branch pull-draw-memory-housekeeping

2023-11-20 12:23:24 +01:00

Miguel Pozo referenced this issue from a commit

2023-11-20 12:23:26 +01:00

Fix #114742: Draw: Buffers never shrink

Thomas Dinges referenced this issue from a commit

2023-11-27 17:38:20 +01:00

Fix #114742: Draw: Buffers never shrink

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

Fix #114742: Draw: Buffers never shrink #114857