<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On 21 Mar 2017, at 13:59, Yuri Benditovich <<a href="mailto:yuri.benditovich@daynix.com" class="">yuri.benditovich@daynix.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_extra"><br class="Apple-interchange-newline"><br class=""><div class="gmail_quote">On Mon, Mar 20, 2017 at 2:08 PM, Frediano Ziglio<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:fziglio@redhat.com" target="_blank" class="">fziglio@redhat.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><span class="">><br class="">> Instead of sending drawable commands down from presentation<br class="">> callback, collect drawables objects and pass them to dedicated<br class="">> thread for processing. This reduce peak load of presentation<br class="">> callback.<br class="">><br class="">> Signed-off-by: Javier Celaya <<a href="mailto:javier.celaya@flexvdi.com" class="">javier.celaya@flexvdi.com</a>><br class="">> Signed-off-by: Yuri Benditovich <<a href="mailto:yuri.benditovich@daynix.com" class="">yuri.benditovich@daynix.com</a>><br class="">> ---<br class="">>  qxldod/QxlDod.cpp | 43 ++++++++++++++++++++++++++++++<wbr class="">+++----------<br class="">>  qxldod/QxlDod.h   |  4 ++--<br class="">>  2 files changed, 35 insertions(+), 12 deletions(-)<br class="">><br class="">> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp<br class="">> index b952bf9..01de9b3 100755<br class="">> --- a/qxldod/QxlDod.cpp<br class="">> +++ b/qxldod/QxlDod.cpp<br class="">> @@ -3794,11 +3794,20 @@ QxlDevice::<wbr class="">ExecutePresentDisplayOnly(<br class="">>      SIZE_T sizeRects = NumDirtyRects*sizeof(RECT);<br class="">>      SIZE_T size = sizeof(DoPresentMemory) + sizeMoves + sizeRects;<br class="">><br class="">> +    QXLDrawable **pDrawables = reinterpret_cast<QXLDrawable **>(new<br class="">> (NonPagedPoolNx) BYTE[sizeof(QXLDrawable *)*(NumDirtyRects + NumMoves +<br class="">> 1)]);<br class=""><br class=""></span>here would be<br class=""><br class=""> <span class="Apple-converted-space"> </span>QXLDrawable **pDrawables = new (NonPagedPoolNx) QXLDrawable *[NumDirtyRects + NumMoves + 1];<br class=""><br class="">is non paged memory needed? Both functions (producer and consumer) are in paged areas.<br class=""><span class=""><br class="">> +    UINT nIndex = 0;<br class="">> +<br class="">> +    if (!pDrawables)<br class="">> +    {<br class="">> +        return STATUS_NO_MEMORY;<br class="">> +    }<br class="">> +<br class="">>      DoPresentMemory* ctx = reinterpret_cast<<wbr class="">DoPresentMemory*><br class="">>                                  (new (NonPagedPoolNx) BYTE[size]);<br class="">><br class=""><br class=""></span>   DoPresentMemory* ctx = new (NonPagedPoolNx) DoPresentMemory;<br class=""></blockquote><div class=""><br class=""></div><div class="">Current commit intentionally does not change this, to make clear what is changed.</div></div></div></div></div></blockquote><div><br class=""></div><div>Good.</div><div><br class=""></div><div>If we use a non-paged placement new, shouldn’t we use a corresponding placement delete?</div><div><br class=""></div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_extra"><div class="gmail_quote"><div class="">Without changes in the code we can't just allocate less memory, as trailing RECT structures are used for</div><div class="">unneeded copy from parameters.</div><div class="">Removal of this completely unneeded 'ctx' is planned for further commits after all the HCK-related work is done.</div></div></div></div></div></blockquote><div><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><br class="">same comments as above<br class=""><span class=""><br class="">>      if (!ctx)<br class="">>      {<br class="">> +        delete[] reinterpret_cast<BYTE*>(<wbr class="">pDrawables);<br class=""><br class=""></span>delete[] pDrawables;<br class=""><span class=""><br class="">>          return STATUS_NO_MEMORY;<br class="">>      }<br class="">><br class="">> @@ -3828,6 +3837,8 @@ QxlDevice::<wbr class="">ExecutePresentDisplayOnly(<br class="">>          PMDL mdl = IoAllocateMdl((PVOID)SrcAddr, sizeToMap,  FALSE, FALSE,<br class="">>          NULL);<br class="">>          if(!mdl)<br class="">>          {<br class="">> +            delete[] reinterpret_cast<BYTE*>(ctx);<br class="">> +            delete[] reinterpret_cast<BYTE*>(<wbr class="">pDrawables);<br class=""><br class=""></span>similar to above, in this case "delete ctx;"<br class=""><span class=""><br class="">>              return STATUS_INSUFFICIENT_RESOURCES;<br class="">>          }<br class="">><br class="">> @@ -3844,6 +3855,8 @@ QxlDevice::<wbr class="">ExecutePresentDisplayOnly(<br class="">>          {<br class="">>              Status = GetExceptionCode();<br class="">>              IoFreeMdl(mdl);<br class="">> +            delete[] reinterpret_cast<BYTE*>(ctx);<br class="">> +            delete[] reinterpret_cast<BYTE*>(<wbr class="">pDrawables);<br class=""><br class=""></span>ditto<br class=""><span class=""><br class="">>              return Status;<br class="">>          }<br class="">><br class="">> @@ -3857,6 +3870,8 @@ QxlDevice::<wbr class="">ExecutePresentDisplayOnly(<br class="">>              Status = STATUS_INSUFFICIENT_RESOURCES;<br class="">>              MmUnlockPages(mdl);<br class="">>              IoFreeMdl(mdl);<br class="">> +            delete[] reinterpret_cast<BYTE*>(ctx);<br class="">> +            delete[] reinterpret_cast<BYTE*>(<wbr class="">pDrawables);<br class=""><br class=""></span>ditto<br class=""><div class=""><div class="h5"><br class="">>              return Status;<br class="">>          }<br class="">><br class="">> @@ -3922,7 +3937,9 @@ QxlDevice::<wbr class="">ExecutePresentDisplayOnly(<br class="">>          DbgPrint(TRACE_LEVEL_<wbr class="">INFORMATION, ("--- %d SourcePoint.x = %ld,<br class="">>          SourcePoint.y = %ld, DestRect.bottom = %ld, DestRect.left = %ld,<br class="">>          DestRect.right = %ld, DestRect.top = %ld\n",<br class="">>              i , pSourcePoint->x, pSourcePoint->y, pDestRect->bottom,<br class="">>              pDestRect->left, pDestRect->right, pDestRect->top));<br class="">><br class="">> -        CopyBits(*pDestRect, *pSourcePoint);<br class="">> +        pDrawables[nIndex] = CopyBits(*pDestRect, *pSourcePoint);<br class="">> +<br class="">> +        if (pDrawables[nIndex]) nIndex++;<br class="">>      }<br class="">><br class="">>      // Copy all the dirty rects from source image to video frame buffer.<br class="">> @@ -3936,11 +3953,13 @@ QxlDevice::<wbr class="">ExecutePresentDisplayOnly(<br class="">>          DbgPrint(TRACE_LEVEL_<wbr class="">INFORMATION, ("--- %d pDirtyRect->bottom = %ld,<br class="">>          pDirtyRect->left = %ld, pDirtyRect->right = %ld, pDirtyRect->top =<br class="">>          %ld\n",<br class="">>              i, pDirtyRect->bottom, pDirtyRect->left, pDirtyRect->right,<br class="">>              pDirtyRect->top));<br class="">><br class="">> -        BltBits(&DstBltInfo,<br class="">> +        pDrawables[nIndex] = BltBits(&DstBltInfo,<br class="">>          &SrcBltInfo,<br class="">>          1,<br class="">>          pDirtyRect,<br class="">>          &sourcePoint);<br class="">> +<br class="">> +        if (pDrawables[nIndex]) nIndex++;<br class="">>      }<br class="">><br class="">>      // Unmap unmap and unlock the pages.<br class="">> @@ -3951,6 +3970,10 @@ QxlDevice::<wbr class="">ExecutePresentDisplayOnly(<br class="">>      }<br class="">>      delete [] reinterpret_cast<BYTE*>(ctx);<br class="">><br class="">> +    pDrawables[nIndex] = NULL;<br class="">> +<br class="">> +    PostToWorkerThread(pDrawables)<wbr class="">;<br class="">> +<br class="">>      return STATUS_SUCCESS;<br class="">>  }<br class="">><br class="">> @@ -4364,7 +4387,7 @@ VOID QxlDevice::SetImageId(<wbr class="">InternalImage *internal,<br class="">>      }<br class="">>  }<br class="">><br class="">> -void QxlDevice::CopyBits(const RECT& rect, const POINT& sourcePoint)<br class="">> +QXLDrawable *QxlDevice::CopyBits(const RECT& rect, const POINT& sourcePoint)<br class="">>  {<br class=""><br class=""></div></div>This CopyBits and BltBits are not doing anymore the operation, should<br class="">be renamed to something like PrepareCopyBits (better names welcome)<br class=""></blockquote><div class=""><br class=""></div><div class="">No problem, prefer to do it on later commits.</div><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div class="HOEnZb"><div class="h5"><br class="">>      PAGED_CODE();<br class="">>      QXLDrawable *drawable;<br class="">> @@ -4373,18 +4396,18 @@ void QxlDevice::CopyBits(const RECT& rect, const<br class="">> POINT& sourcePoint)<br class="">><br class="">>      if (!(drawable = Drawable(QXL_COPY_BITS, &rect, NULL, 0))) {<br class="">>          DbgPrint(TRACE_LEVEL_ERROR, ("Cannot get Drawable.\n"));<br class="">> -        return;<br class="">> +        return NULL;<br class="">>      }<br class="">><br class="">>      drawable->u.copy_bits.src_pos.<wbr class="">x = sourcePoint.x;<br class="">>      drawable->u.copy_bits.src_pos.<wbr class="">y = sourcePoint.y;<br class="">><br class="">> -    PushDrawable(drawable);<br class="">> -<br class="">>      DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));<br class="">> +<br class="">> +    return drawable;<br class="">>  }<br class="">><br class="">> -VOID QxlDevice::BltBits (<br class="">> +QXLDrawable *QxlDevice::BltBits (<br class="">>      BLT_INFO* pDst,<br class="">>      CONST BLT_INFO* pSrc,<br class="">>      UINT  NumRects,<br class="">> @@ -4407,7 +4430,7 @@ VOID QxlDevice::BltBits (<br class="">><br class="">>      if (!(drawable = Drawable(QXL_DRAW_COPY, pRects, NULL, 0))) {<br class="">>          DbgPrint(TRACE_LEVEL_ERROR, ("Cannot get Drawable.\n"));<br class="">> -        return;<br class="">> +        return NULL;<br class="">>      }<br class="">><br class="">>      CONST RECT* pRect = &pRects[0];<br class="">> @@ -4480,9 +4503,9 @@ VOID QxlDevice::BltBits (<br class="">>           drawable->surfaces_rects[0].<wbr class="">top,<br class="">>           drawable->surfaces_rects[0].<wbr class="">bottom,<br class="">>           drawable->u.copy.src_bitmap));<br class="">><br class="">> -    PushDrawable(drawable);<br class="">> -<br class="">>      DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));<br class="">> +<br class="">> +    return drawable;<br class="">>  }<br class="">><br class="">>  VOID QxlDevice::PutBytesAlign(<wbr class="">QXLDataChunk **chunk_ptr, UINT8 **now_ptr,<br class="">> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h<br class="">> index 4a62680..f441f4b 100755<br class="">> --- a/qxldod/QxlDod.h<br class="">> +++ b/qxldod/QxlDod.h<br class="">> @@ -495,12 +495,12 @@ public:<br class="">>      BOOLEAN IsBIOSCompatible() { return FALSE; }<br class="">>  protected:<br class="">>      NTSTATUS GetModeList(DXGK_DISPLAY_<wbr class="">INFORMATION* pDispInfo);<br class="">> -    VOID BltBits (BLT_INFO* pDst,<br class="">> +    QXLDrawable *BltBits (BLT_INFO* pDst,<br class="">>                      CONST BLT_INFO* pSrc,<br class="">>                      UINT  NumRects,<br class="">>                      _In_reads_(NumRects) CONST RECT *pRects,<br class="">>                      POINT*   pSourcePoint);<br class="">> -    void CopyBits(const RECT& rect, const POINT& sourcePoint);<br class="">> +    QXLDrawable *CopyBits(const RECT& rect, const POINT& sourcePoint);<br class="">>      QXLDrawable *Drawable(UINT8 type,<br class="">>                      CONST RECT *area,<br class="">>                      CONST RECT *clip,<br class=""><br class=""></div></div><span class="HOEnZb"><font color="#888888" class="">Frediano<br class=""></font></span></blockquote></div><br class=""></div></div><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">_______________________________________________</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Spice-devel mailing list</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class=""><a href="mailto:Spice-devel@lists.freedesktop.org" class="">Spice-devel@lists.freedesktop.org</a></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class=""><a href="https://lists.freedesktop.org/mailman/listinfo/spice-devel" class="">https://lists.freedesktop.org/mailman/listinfo/spice-devel</a></span></div></blockquote></div><br class=""></body></html>