[PATCH 6/6] d3dx9: Merge constant setting for child parameters when possible.

[Paul Gofman]

On 05/29/2017 09:24 PM, Matteo Bruni wrote:
> 2017-05-24 11:46 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
>
>> +            if (!(const_set->table == current_table && current_start_offset == start_offset
>> +                    && const_set->direct_copy == first_const->direct_copy
>> +                    && current_data == const_set->param->data
>> +                    && (const_set->direct_copy || (first_const->param->type == const_set->param->type
>> +                    && first_const->param->class == const_set->param->class
>> +                    && first_const->param->columns == const_set->param->columns
>> +                    && first_const->param->rows == const_set->param->rows
>> +                    && first_const->register_count == const_set->register_count
>> +                    && (i == const_tab->const_set_count - 1
>> +                    || first_const->param->element_count == const_set->param->element_count)))))
>> +            {
>> +                TRACE("direct_copy %u, i %u, index %u, param %s.%s, current_data %p, const_set->param->data %p, "\
>> +                        "current_start_offset %u, start_offset %u, const_set->table %u, current_table %u.\n",
>> +                        const_set->direct_copy, i, index, debugstr_a(param->name),
>> +                        debugstr_a(const_set->param->name), current_data, const_set->param->data,
>> +                        current_start_offset, start_offset, const_set->table, current_table);
>> +                break;
>> +            }
> This looks like a debug trace.
>
>> +        TRACE("Merging %u child parameters for %s, not merging %u, direct_copy %u.\n", i - index,
>> +                debugstr_a(param->name), const_tab->const_set_count - i, first_const->direct_copy);
> I guess it's intentional that you're printing this even when not
> merging anything, in that case the TRACE sounds a bit awkward though.
> Maybe have a separate TRACE for that case?
Both traces are to have a possibility to check from trace log what is 
merged and not "merged" and why, for performance analysis. If you think 
it is more of debug I can remove it, though since it is in 
initialization only I was thinking it does not add too much noise.
>
> BTW, please format the BOOL with %#x.
>
>> +        if (i > index + 1)
>> +        {
>> +            first_const->element_count = element_count;
>> +            if (first_const->direct_copy)
>> +            {
>> +                first_const->element_count = 1;
>> +                if (index == start_index
>> +                        && !(param->type == D3DXPT_VOID && param->class == D3DXPC_STRUCT))
>> +                {
>> +                    if (param_type_to_table_type(param->type) == PRES_VT_MAX)
>> +                        return D3DERR_INVALIDCALL;
>> +                    first_const->param = param;
>> +                }
>> +                first_const->register_count = get_reg_offset(current_table, current_start_offset)
>> +                        - first_const->register_index;
>> +            }
>> +            memmove(&const_tab->const_set[index + 1], &const_tab->const_set[i],
>> +                    sizeof(*const_tab->const_set) * (const_tab->const_set_count - i));
>> +            const_tab->const_set_count -= i - index - 1;
>> +        }
> Just for my own curiosity, do you think it's significant to merge
> non-direct_copy entries? Not that it hurts anything to have it.
>
>
Yes, it is crucial together with the next patches I did not send yet. 
Even in this patchset removing extra const_set entries is beneficial: it 
removes a lot of dirty checks when skipping const_set's from the same 
parameter on CommitChanges, and also initializing the data for inner 
loops less times. In the next patches which is not there yet I:

- Factor out type conversion which can be used for an array of values, 
removing switching for each value being set;

- Introduce a separate path for setting scalar and vectors (which is 
simpler than general matrix case an quite often may be grouped 
effectively even when it is not 'direct_copy');

- Optimize matrix settings loops.

     Copying ~100 element array with transpose is quite a common case. 
Merging them in a one const_set and doing that things further is a huge 
optimization actually.