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

[Matteo Bruni]

On Mon, Aug 16, 2021 at 6:05 PM Henri Verbeet <hverbeet at gmail.com> wrote:
>
> On Fri, 13 Aug 2021 at 16:15, Matteo Bruni <mbruni at codeweavers.com> wrote:
> >  static bool vkd3d_string_buffer_resize(struct vkd3d_string_buffer *buffer, int rc)
> >  {
> > -    unsigned int new_buffer_size = buffer->buffer_size * 2;
> > +    unsigned int new_buffer_size = max(buffer->buffer_size * 2, 32);
> >      char *new_buffer;
> >
> > -    new_buffer_size = max(new_buffer_size, 32);
> > +    if (rc < buffer->buffer_size - buffer->content_size)
> > +        return true;
> > +
> That doesn't quite handle the case where "rc" is -1. Perhaps it's fine
> to assume it isn't these days, but in that case we might as well get
> rid of the rest of the code handling that case as well. The patch also
> seems a little superfluous though; we already have a similar check in
> vkd3d_string_buffer_vprintf().

I entirely forgot about the rc == -1 case... I can fix that.

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.

> 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.