[PATCH vkd3d 1/3] vkd3d-shader: Don't resize the buffer when there is enough free space.

[Henri Verbeet]

On Mon, 16 Aug 2021 at 18:39, Matteo Bruni <matteo.mystral at gmail.com> wrote:
> Otherwise, I know that I only noticed this issue and started writing
> these patches because I was in fact getting out of memory errors :)
> IIRC vkd3d_string_buffer_resize() would at least double the size of
> the buffer every time it's called and vkd3d_string_buffer_vprintf() in
> practice ended up writing 0 bytes to the buffer relatively often (I
> haven't investigated that part, but it seems legit, if weird). Also
> apparently the code often doubled the size one too many times. I'll
> check that again but I don't think I saw that after this patch.
>
My guess would be that it's caused by the "if (!buffer->content_size
&& !vkd3d_string_buffer_resize(buffer, 32))" line. As mentioned, that
will cause the buffer size to double when calling
vkd3d_string_buffer_vprintf() after a vkd3d_string_buffer_clear(),
until "content_size" actually becomes non-zero. That could be
mitigated by doing something like this in vkd3d_string_buffer_resize()

    if (rc >= 0)
    {
        new_buffer_size = max(buffer->buffer_size, 32);
        while ((unsigned int)rc >= new_buffer_size - buffer->content_size)
            new_buffer_size *= 2;
    }
    else
    {
        new_buffer_size = max(buffer->buffer_size * 2, 32);
    }

but in the end there's just no need for that particular
vkd3d_string_buffer_resize() call.

> > We would do an unnecessary resize when calling
> > vkd3d_string_buffer_vprintf() the first time after a
> > vkd3d_string_buffer_clear(), although the best way to fix that would
> > seem to be simply getting rid of the vkd3d_string_buffer_resize() call
> > responsible for that, like patch 2/3 in this series does. It was added
> > by commit 5fb9bcdd148ed728d90a420f209cdfcc03af24ac, but that same
> > commit also adds the max() in vkd3d_string_buffer_resize() that should
> > make it superfluous.
>
> I guess one possible mitigation would be to rename
> vkd3d_string_buffer_resize() to vkd3d_string_buffer_reserve() or
> similar, to clarify that it won't reallocate the buffer when
> unnecessary.

We could also simply use vkd3d_array_reserve() in
vkd3d_string_buffer_vprintf(); it would essentially look something
like this:

    if (rc >= 0)
        vkd3d_array_reserve(..., rc, ...);
    else
        vkd3d_array_reserve(..., buffer->buffer_size * 2, ...);