<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 14:52, Christophe de Dinechin <<a href="mailto:cdupontd@redhat.com" class="">cdupontd@redhat.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div 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=""><blockquote type="cite" class=""><div class=""><br class="Apple-interchange-newline">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" class="" 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;"><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 class=""><br class=""></div><div class="">Good.</div><div class=""><br class=""></div><div class="">If we use a non-paged placement new, shouldn’t we use a corresponding placement delete?</div></div></div></blockquote><div><br class=""></div><div>Just checked by looking at the Microsoft sample code. They use plain delete. This still seems wrong to me. The underlying pool is not the same. It probably works because the pools share the same internal layout, so the same operator delete works in both cases.</div><div><br class=""></div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div 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=""><br class=""></div><div class=""><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class="" 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;"><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 class=""><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class="" 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;"><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 class="" 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;">_______________________________________________</span><br class="" 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;"><span class="" 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;">Spice-devel mailing list</span><br class="" 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;"><span class="" 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;"><a href="mailto:Spice-devel@lists.freedesktop.org" class="">Spice-devel@lists.freedesktop.org</a></span><br class="" 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;"><span class="" 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;"><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="" 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;"><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=""><a href="mailto:Spice-devel@lists.freedesktop.org" 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-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">Spice-devel@lists.freedesktop.org</a><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=""><a href="https://lists.freedesktop.org/mailman/listinfo/spice-devel" 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-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">https://lists.freedesktop.org/mailman/listinfo/spice-devel</a></div></blockquote></div><br class=""></body></html>