[PATCH 3/3] wined3d: Reduce the number of independent heap allocations.

[Jan Sikorski]

> On 5 Oct 2021, at 15:33, Henri Verbeet <hverbeet at gmail.com> wrote:
> 
> On Wed, 29 Sept 2021 at 17:40, Jan Sikorski <jsikorski at codeweavers.com> wrote:
>> ---
>> dlls/wined3d/cs.c | 56 +++++++++++++++++++++--------------------------
>> 1 file changed, 25 insertions(+), 31 deletions(-)
>> 
> Conceptually I think this is fine, although traditionally we'd use
> something like "wined3d: Use a single allocation for command list
> data." for the subject line.
> 
>> @@ -3735,45 +3730,56 @@ HRESULT CDECL wined3d_deferred_context_record_command_list(struct wined3d_device
>> {
>>     struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context);
>>     struct wined3d_command_list *object;
>> +    size_t offset;
>> +    BYTE *memory;
>> 
> We'd prefer uint8_t over BYTE here. (More broadly, we prefer standard
> types over Windows types inside wined3d.)
> You probably need neither "memory" nor "offset" though.
> 
>> +    memory = heap_alloc(sizeof(*object) + deferred->resource_count * sizeof(*object->resources) +
>> +            deferred->upload_count * sizeof(*object->uploads) +
>> +            deferred->command_list_count * sizeof(*object->command_lists) +
>> +            deferred->query_count * sizeof(*object->queries) +
>> +            deferred->data_size);
>> +
>> +    if (!memory)
>>         return E_OUTOFMEMORY;
>> 
>> +    object = (void *)memory;
>> +    offset = sizeof(*object);
>> +    memset(object, 0, sizeof(*object));
>> +
>>     object->refcount = 1;
>>     object->device = deferred->c.device;
>> 
>> -    if (!(object->data = heap_alloc(deferred->data_size)))
>> -        goto out_free_list;
>> -    object->data_size = deferred->data_size;
>> -    memcpy(object->data, deferred->data, deferred->data_size);
>> -
>> -    if (!(object->resources = heap_alloc(deferred->resource_count * sizeof(*object->resources))))
>> -        goto out_free_data;
>> +    object->resources = (void *)(memory + offset);
>> +    offset += deferred->resource_count * sizeof(*object->resources);
>>     object->resource_count = deferred->resource_count;
>>     memcpy(object->resources, deferred->resources, deferred->resource_count * sizeof(*object->resources));
>>     /* Transfer our references to the resources to the command list. */
>> 
>> -    if (!(object->uploads = heap_alloc(deferred->upload_count * sizeof(*object->uploads))))
>> -        goto out_free_resources;
>> +    object->uploads = (void *)(memory + offset);
>> +    offset += deferred->upload_count * sizeof(*object->uploads);
>>     object->upload_count = deferred->upload_count;
>>     memcpy(object->uploads, deferred->uploads, deferred->upload_count * sizeof(*object->uploads));
>>     /* Transfer our references to the resources to the command list. */
>> 
> We'd typically write that like this:
> 
>    object = heap_alloc(...);
>    ...
>    object->resources = (struct wined3d_resource *)&object[1];
>    ...
>    object->uploads = (struct wined3d_deferred_upload
> *)&object->resources[object->resource_count];
> 
> etc.

I’ve seen that elsewhere in the codebase, but I think it has some downsides. As is, each block of code only deals with a single field, and you can reorder these blocks around freely and add/remove without changing the ones next to it. Not that it’s a huge problem to do it, but are there reasons for this?