[Spice-devel] [PATCH 10/12] qxl-wddm-dod: Optimize allocation of memory chunks

[Yuri Benditovich]

On Tue, Mar 28, 2017 at 7:19 PM, Frediano Ziglio <fziglio at redhat.com> wrote:

>
> On Tue, Mar 28, 2017 at 4:23 PM, Frediano Ziglio <fziglio at redhat.com>
> wrote:
>
>> >
>> > Increased size of allocation to reduce number of allocation per
>> > bitmap. Before change the procedure ignored 'alloc_size' parameter
>> > and always allocated memory chunk according to 'size' parameter.
>> > As a result, first chunk could be up to 64K and all following are
>> > limited by line size. For example, for bitmap 1280x1024 there was
>> > more than 1008 chunks allocated, for bitmap 128x1024 - 897 chunks.
>> > We change the procedure to use chunk size up to 64K (similar to first
>> > chunk). This reduces in described examples number of allocation from
>> > 1008 to 64 and 897 to 8 respectively.
>> >
>> > Signed-off-by: Yuri Benditovich <yuri.benditovich at daynix.com>
>> > ---
>> >  qxldod/QxlDod.cpp | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
>> > index 56a4cf2..94426dd 100755
>> > --- a/qxldod/QxlDod.cpp
>> > +++ b/qxldod/QxlDod.cpp
>> > @@ -4586,7 +4586,7 @@ BOOLEAN QxlDevice::PutBytesAlign(QXLDataChunk
>> > **chunk_ptr, UINT8 **now_ptr,
>> >      UINT8 *now = *now_ptr;
>> >      UINT8 *end = *end_ptr;
>> >      size_t maxAllocSize = BITS_BUF_MAX - BITS_BUF_MAX % size;
>> > -    alloc_size = MIN(size, maxAllocSize);
>> > +    alloc_size = MIN(alloc_size, maxAllocSize);
>> >      DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
>> >
>> >      while (size) {
>>
>> As far as I can see this fix a regression introduced in 9/12 so
>> should be squashed together with 9/12.
>>
>
> There was no regression in 9/12, see commit description
> This patch forces the driver to make bigger allocations and in result we
> will have smaller amount of allocations per operation.
>
> Yes, you are right, there is this computation
>
>   aligned_size = (int)MIN(alloc_size + alignment - 1, BITS_BUF_MAX);
>   aligned_size -=  aligned_size % alignment;
>
> but then the value is discarded and size is used instead.
>
>
>> On the same subject. Why we need to split the allocations?
>> Can't we try to allocate the whole image in one chunk and
>> only if fails fall back to multiple chunks ?
>>
>
> No, we can't do that, as maximal size of allocation from device memory in
> driver's code was always limited by ~64K.
> So allocation were always splitted, see numbers in commit description.
> I suppose there is some reason for that.
>
> Honestly knowing the server part I don't see any reasons. Maybe an history
> one...
>
> If any version of Spice server is able to work with bigger chunks, this
optimization can be evaluated later.
In any case it requires non-forced allocations to be fully implemented and
used ( as we can 'try' to allocate
full size chunk only if we do not block during such allocation).



> Frediano
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170329/0a1c7eb7/attachment.html>