Re: [PATCH 2/5] wined3d: Handle CS op emission from CS op handlers.

[Sebastian Lackner]

On 17.02.2017 14:08, Henri Verbeet wrote:
> Signed-off-by: Henri Verbeet
>
> dlls/wined3d/cs.c              | 37 +++++++++++++++++++++++++++++--------
>  dlls/wined3d/wined3d_private.h |  2 +-
>  2 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/dlls/wined3d/cs.c b/dlls/wined3d/cs.c
> index 55706cd..10905c0 100644
> --- a/dlls/wined3d/cs.c
> +++ b/dlls/wined3d/cs.c
> @@ -1576,26 +1576,47 @@ static void (* const wined3d_cs_op_handlers[])(struct wined3d_cs *cs, const void
>
> static void *wined3d_cs_st_require_space(struct wined3d_cs *cs, size_t size)
>  {
> - if (size > cs->data_size)
> - if (size > (cs->data_size - cs->end))
> {
> -
>    size_t new_size;
>    void *new_data;
>
> -
>    size = max( size, cs->data_size * 2 );
> -
>    if (!(new_data = HeapReAlloc(GetProcessHeap(), 0, cs->data, size)))
> -
>    new_size = max(size, cs->data_size * 2);
> -
>    if (!cs->end)
> -
>        new_data = HeapReAlloc(GetProcessHeap(), 0, cs->data, new_size);
> -
>    else
> -
>        new_data = HeapAlloc(GetProcessHeap(), 0, new_size);
> -
>    if (!new_data)
>        return NULL;
>
> -
>    cs->data_size = size;
> -
>    cs->data_size = new_size;
> -
>    cs->start = cs->end = 0;
>    cs->data = new_data;
> }
> - return cs->data;
> - cs->end += size;
> -
> - return (BYTE *)cs->data + cs->start;
>
> }
>
> static void wined3d_cs_st_submit(struct wined3d_cs *cs)
>  {
> - enum wined3d_cs_op opcode = *(const enum wined3d_cs_op *)cs->data;
> -
> - wined3d_cs_op_handlers[opcode](cs, cs->data);
> - enum wined3d_cs_op opcode;
> - size_t start;
> - BYTE *data;
> -
> - data = cs->data;
> - start = cs->start;
> - cs->start = cs->end;
> - opcode = *(const enum wined3d_cs_op *)&data[start];
> - wined3d_cs_op_handlers[opcode](cs, &data[start]);
> - if (!start)
> - {
> -
>    if (cs->data != data)
> -
>        HeapFree(GetProcessHeap(), 0, data);
> -
>    else
> -
>        cs->start = cs->end = 0;
> - }

Is it intentional that you are not updating cs->start and cs->end in any
other situation? This might lead to unnecessary memory allocations because
start / end is not set back before the block is fully empty. Here an example
what happens if wined3d_device_delete_opengl_contexts_cs is called with lots
of resources (with this patch http://ix.io/dP5):

fixme:d3d:wined3d_device_delete_opengl_contexts_cs -> unloading resources
fixme:d3d:wined3d_cs_st_require_space start = 12
fixme:d3d:wined3d_cs_st_require_space start = 20
fixme:d3d:wined3d_cs_st_require_space start = 28
[...]
fixme:d3d:wined3d_cs_st_require_space start = 4068
fixme:d3d:wined3d_cs_st_require_space start = 4076
fixme:d3d:wined3d_cs_st_require_space start = 4084
fixme:d3d:wined3d_cs_st_require_space allocated new block 0x579fdb70
fixme:d3d:wined3d_cs_st_require_space start = 0
fixme:d3d:wined3d_cs_st_require_space start = 0
fixme:d3d:wined3d_cs_st_require_space start = 0
[..]

In this case, with the buffer basically almost empty, there shouldn't be any
need to do memory allocations.
> }
>
> static void wined3d_cs_st_push_constants(struct wined3d_cs *cs, enum wined3d_push_constants p,
> diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
> index 54adf26..49fba06 100644
> --- a/dlls/wined3d/wined3d_private.h
> +++ b/dlls/wined3d/wined3d_private.h
> @@ -3170,7 +3170,7 @@ struct wined3d_cs
>      struct wined3d_fb_state fb;
>      struct wined3d_state state;
> - size_t data_size;
> - size_t data_size, start, end;
> void *data;
>
> };