[OpenJDK 2D-Dev] [14] RFR JDK-8235147: Release HDC from passiveDCList sooner

[Alexey Ivanov]

Hi Sergey,

On 09/12/2019 21:26, Alexey Ivanov wrote:
> Hi Sergey
>
> On 04/12/2019 22:17, Sergey Bylokhov wrote:
>> On 12/4/19 6:06 am, Alexey Ivanov wrote:
>>> Yes, we still need passiveDCList. A DC can be moved to the passive 
>>> list by DisposeThreadGraphicsInfo function in 
>>> GDIWindowSurfaceData.cpp. This is called not on the toolkit thread, 
>>> so we have to defer releasing the DC in this case.
>>>
>>> However, I agree, we can simplify releasing DCs when on the toolkit 
>>> thread by skipping the passive list. Yet having two different 
>>> approaches could be error prone.
>
> I actually tried this approach. It may look a bit simpler with three 
> functions:
> ReleaseActiveDCs(HDC hDC, HWND hWnd)
> ReleaseActiveDCs(HWND hWnd)
> ReleasePassiveDCs()
>
> Yet in the majority of cases, the DC is moved to passive list using 
> MoveDCToPassiveList from SetupThreadGraphicsInfo. It's done before a 
> new DC is gotten.
>
> I implemented ReleaseActiveDCs so that the DCs are released bypassing 
> the passive list. However in my testing, DCs are never released from 
> the active list; the exception to this rule being frame destruction 
> where all DCs for a window are released.
>
>>
>> Ok, then let's leave the current fix as-is.
>>
>
> Taking into account my comments above, I believe it's better to leave 
> the fix as is without further modifications.
>
> I'm not sure if replacing
> ReleaseDCList(passiveDCList);
> with
> ReleasePassiveDCs()
> would make the code clearer. Probably not. With the explicit list 
> parameter, the call ReleaseDCList(GetHWnd(), activeDCList) in 
> WM_AWT_RELEASE_ALL_DCS seems more consistent.
>
> Alternatively, we can add MoveDCToPassiveList(HWND) which will move 
> all DCs for the HWND from active list to passive list. Then the code 
> would be consistent:
> MoveDCToPassiveList(HDC, HWND) or MoveDCToPassiveList(HWND)
> followed by ReleasePassiveDCs()
>
> It could work. I'll try this approach.
>

It does not work good. I found serious issues in either approach I've 
tried so far.

Thus, we're down to the original proposal:

JBS: https://bugs.openjdk.java.net/browse/JDK-8235147
Webrev: http://cr.openjdk.java.net/~aivanov/8235147/webrev.00/

-- 
Regards,
Alexey