[OpenJDK 2D-Dev] <AWT Dev> RFR: 8240654 : ZGC can cause severe UI application repaint issues

Thu Jun 11 18:13:14 UTC 2020

+1

The updated LargeWindowPaintTest fails for me without the fix without my 
having to manually set uiScale. It passes with the fix.

Interestingly enough, I finally saw the problem that Jay reported with 
AlphaPrintTest: without the fix I initially get a blank (all white) 
window. If I resize it then it is drawn. With the fix everything is fine.

-- Kevin

On 6/11/2020 10:48 AM, Philip Race wrote:
> Updated webrev here
>
> http://cr.openjdk.java.net/~prr/8240654.1/index.html
>
> The only changes are to the tests - to add -Dsun.java2d.uiScale=1 to 
> the onscreen test
> and to add printer to the keys for the printing test.
>
> It was pointed out by Stefan from the ZGC team that the changes in 
> awt_TrayIcon.cpp and awt_Cursor.cpp
> should not be needed because the GDI code in Create_BMP that 
> ultimately consumes the data
> has processed and copied it into memory allocated by CreateDIBSection 
> before passing it to
> CreateBitmap. I considered reverting those two files but decided to 
> keep them because I
> think I would like this fix anyway. We really don't need to lock down 
> the VM in these cases.
>
> -phil.
>
>
> On 6/11/20, 9:55 AM, Philip Race wrote:
>> I have confirmed hit a different code path. It goes through generic 
>> 2D s/w loops in this case.
>> ie we don't use GDIBlitLoops at all. The code in 
>> sun/java2d/pipe/DrawImage.java ends up in
>> scaleSurfaceData which uses the loops in ScaledBlit.c.
>>
>> It is a bit surprising to me since I'd expect us to be able to blit 
>> directly at device resolution.
>> Could we also be taking a performance hit here ? The D3D case doesn't 
>> not go through this loop.
>>
>> However all that is outside the scope of this fix ... I think setting 
>> uiScale=1 in the test is all that needs to be done.
>>
>> -phil.
>>
>> On 6/11/2020 7:51 AM, Philip Race wrote:
>>> Or, maybe we hit a different code path. I'll check that.
>>> uiScale=1 is the way to ensure we hit this code path.
>>>
>>> -phil.
>>>
>>> On 6/11/20, 7:44 AM, Philip Race wrote:
>>>> Can I get clarification here.
>>>>
>>>> > I do, and had to run with "-Dsun.java2d.uiScale=1" in order to 
>>>> see the failure with LargeWindowPaintTest.
>>>>
>>>> So you both mean a JDK 15 promoted build without this fix and 
>>>> without this property passes because you have
>>>> a hidpi setup. And to see the failure without the fix you needed 
>>>> the above property.
>>>> If so we could just be looking at a similar anomaly as I saw with 
>>>> printing which uses a very large
>>>> image - it reported failure but actually worked !
>>>>
>>>> Also - for both of you - with the fix and without forcing uiScale=1 
>>>> does the test pass ?
>>>>
>>>> -phil.
>>>>
>>>> On 6/11/20, 7:10 AM, Jayathirth D v wrote:
>>>>> Yes my machine was at 150% scaling.
>>>>>
>>>>> If I force uiScale = 1, I see that:
>>>>> LargeWindowPaintTest fails without patch and passes with patch.
>>>>> AlphaPrintTest shows instructions without patch also.
>>>>>
>>>>> @Phil : I think its better if we test at uiScale=1(larger memory 
>>>>> footprint). Please clarify.
>>>>>
>>>>> Thanks,
>>>>> Jay
>>>>>
>>>>>> On 11-Jun-2020, at 5:53 PM, Kevin Rushforth 
>>>>>> <kevin.rushforth at oracle.com <mailto:kevin.rushforth at oracle.com>> 
>>>>>> wrote:
>>>>>>
>>>>>> Do you have a Hi-DPI machine? I do, and had to run with 
>>>>>> "-Dsun.java2d.uiScale=1" in order to see the failure with 
>>>>>> LargeWindowPaintTest.
>>>>>>
>>>>>> For AlphaPrintTest, the test deliberately ensures that you print 
>>>>>> before saying whether it passes or not. FWIW, I verified that the 
>>>>>> printing test on my system was hitting the fallback code with the 
>>>>>> patch, but it seemed to print correctly even without the patch.
>>>>>>
>>>>>> -- Kevin
>>>>>>
>>>>>>
>>>>>> On 6/11/2020 1:58 AM, Jayathirth D v wrote:
>>>>>>> Typo : I tried tested -> I tried testing
>>>>>>>
>>>>>>>> On 11-Jun-2020, at 2:27 PM, Jayathirth D v 
>>>>>>>> <JAYATHIRTH.D.V at ORACLE.COM <mailto:JAYATHIRTH.D.V at ORACLE.COM>> 
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hi Phil,
>>>>>>>>
>>>>>>>> I tried tested the fix in my Windows 10 machine with Intel 
>>>>>>>> integrated UHD Graphics 620.
>>>>>>>>
>>>>>>>> LargeWindowPaintTest.java passes with/without fix in my machine.
>>>>>>>> AlphaPrintTest.java without fix just opens up blank frame 
>>>>>>>> without any instructions and with fix it shows instructions for 
>>>>>>>> the test.
>>>>>>>> Is this expected behaviour?
>>>>>>>>
>>>>>>>> AlphaPrintTest.java with fix when it shows instructions if I 
>>>>>>>> click on Pass(Since I don’t have printer right now) it doesn’t 
>>>>>>>> pass/close the window. Only after I click on Print button and 
>>>>>>>> then close print dialog it allows me to click on Pass button.
>>>>>>>>
>>>>>>>> Also how does these tests behave in our internal CI machines?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Jay
>>>>>>>>
>>>>>>>>> On 11-Jun-2020, at 2:18 AM, Philip Race 
>>>>>>>>> <philip.race at oracle.com <mailto:philip.race at oracle.com>> wrote:
>>>>>>>>>
>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8240654
>>>>>>>>> Webrev: http://cr.openjdk.java.net/~prr/8240654/index.html
>>>>>>>>>
>>>>>>>>> This is for JDK 15 so review ASAP please since RDP 1 and the 
>>>>>>>>> test cycle are looming.
>>>>>>>>>
>>>>>>>>> This is not a fix for a JDK bug. It is a bunch of workarounds 
>>>>>>>>> for a Microsoft Windows bug affecting
>>>>>>>>> GDI in the context of ZGC (http://openjdk.java.net/jeps/333).
>>>>>>>>> Some extra details about the Windows bug at the end, but first 
>>>>>>>>> the technical details of the fix.
>>>>>>>>>
>>>>>>>>> With ZGC's memory allocation requirement of reserving memory 
>>>>>>>>> in 2Mb chunks  some Windows GDI
>>>>>>>>> functions, mostly involving some bitmaps APIs may return a 
>>>>>>>>> failure code (ie fail!)
>>>>>>>>> This typically occurs when Java heap memory is used for a Java 
>>>>>>>>> image and then in a JNI
>>>>>>>>> call we use GetPrimitiveArrayCritical so that Java heap 
>>>>>>>>> allocated memory is passed to a GDI
>>>>>>>>> function AND the Java heap memory spans one of the 2Mb 
>>>>>>>>> boundaries.
>>>>>>>>> This is very easy to trigger in almost any Java UI app if the 
>>>>>>>>> window is of a large enough (ie typical) size.
>>>>>>>>> NB: if you have an Nvidia or ATI card, then you won't see it, 
>>>>>>>>> because the D3D pipeline doesn't
>>>>>>>>> call the affected method but if you have an Intel chip as do 
>>>>>>>>> 90% (?) of laptops you will see it.
>>>>>>>>> There are also several other places we found that are 
>>>>>>>>> affected. Printing is the other one
>>>>>>>>> somewhat easy to trigger. The others : custom cursors and tray 
>>>>>>>>> icons are less common.
>>>>>>>>> The painful thing here is that there is no definitive list (a 
>>>>>>>>> list of the known ones is below) of
>>>>>>>>> affected Windows GDI APIs and we are just hunting around our 
>>>>>>>>> code trying to see where it
>>>>>>>>> might be side-swiped by this bug.
>>>>>>>>>
>>>>>>>>> The basic approach in these workarounds is that for cases 
>>>>>>>>> where performance does not matter we now copy
>>>>>>>>> and for cases where performance does matter or larger amounts 
>>>>>>>>> of memory is involved we check if
>>>>>>>>> the return value of the GDI function indicates failure and 
>>>>>>>>> then re-try with a copy of the heap memory.
>>>>>>>>> Unless GDI was randomly failing already (unlikely) this should 
>>>>>>>>> be a no-risk solution in the high profile cases.
>>>>>>>>> We have done performance measurements on the important screen 
>>>>>>>>> case and the failures
>>>>>>>>> happen fast so the penalty is then in the re-try which is only 
>>>>>>>>> if ZGC is enabled.
>>>>>>>>> Always copying the memory is slower (and memcpy is the slow 
>>>>>>>>> operation) than an alternative approach
>>>>>>>>> that "knows" about the memory allocation of ZGC but this 
>>>>>>>>> coupling and the complexity seem like they aren't
>>>>>>>>> worth it since I haven't seen any visible performance 
>>>>>>>>> consequence. That can be revisited
>>>>>>>>> some day if need be, but for now we have correctness which is 
>>>>>>>>> the key as well as sufficient performance.
>>>>>>>>>
>>>>>>>>> I've created an automated test for the most important 
>>>>>>>>> on-screen case.
>>>>>>>>> Also a manual printing test case which invokes ZGC is provided 
>>>>>>>>> since there we also only
>>>>>>>>> conditionally copy. In the other cases we now always copy so 
>>>>>>>>> existing test cases should over those.
>>>>>>>>>
>>>>>>>>> There is some clean up in this fix - one completely unused  
>>>>>>>>> (provably so because it was #if'd out)
>>>>>>>>> JNI method in awt_PrintJob.cpp is removed since it had code 
>>>>>>>>> that looked like it needed a workaround,
>>>>>>>>> which would be somewhat of a waste of effort.
>>>>>>>>>
>>>>>>>>> the doPrintBand code and its callee bitsToDevice has code I 
>>>>>>>>> think we can remove too since
>>>>>>>>> I don't see how it ever gets executed (the top down case for 
>>>>>>>>> browserPrint == true) but
>>>>>>>>> I think I'll save that for a P4 follow-on since it does 
>>>>>>>>> nothing that would be affected by this
>>>>>>>>> Windows bug.
>>>>>>>>>
>>>>>>>>> One oddity is the in the printing case I observed that some 
>>>>>>>>> times the rendering is performed
>>>>>>>>> even if an error code is returned. I don't know why, but in 
>>>>>>>>> code we can't tell that it was actually
>>>>>>>>> rendered and in any case there is no harm in repeating the 
>>>>>>>>> call with copied memory.
>>>>>>>>>
>>>>>>>>> We are right before the JDK15 stabilisation fork and this fix 
>>>>>>>>> needs to go there and will
>>>>>>>>> but the webrev is against jdk/client simply because jdk15 does 
>>>>>>>>> not exist yet !
>>>>>>>>>
>>>>>>>>> Please test and review ASAP.
>>>>>>>>>
>>>>>>>>> About the bug:
>>>>>>>>> Microsoft has acknowleged the bug and will publish a knowledge 
>>>>>>>>> base article about it
>>>>>>>>> but a fix may show up only in a future version of Windows. 
>>>>>>>>> Not, it seems, any time soon.
>>>>>>>>> Below is a list of potentially affected GDI APIs. Per 
>>>>>>>>> microsoft whether it actually manifests in
>>>>>>>>> any specific case depends on "branching"
>>>>>>>>>
>>>>>>>>> https://docs.microsoft.com/en-us/previous-versions/windows/desktop/wcs/checkbitmapbits
>>>>>>>>>
>>>>>>>>> https://docs.microsoft.com/en-us/previous-versions/windows/desktop/wcs/createcolortransform
>>>>>>>>>
>>>>>>>>> https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-setdibitstodevice
>>>>>>>>>
>>>>>>>>> https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-stretchdibits
>>>>>>>>>
>>>>>>>>> https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-getbitmapbits
>>>>>>>>>
>>>>>>>>> https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-createdibitmap
>>>>>>>>>
>>>>>>>>> https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-createdibsection
>>>>>>>>>
>>>>>>>>> https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-polydraw
>>>>>>>>>
>>>>>>>>> https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-drawescape
>>>>>>>>>
>>>>>>>>> https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-createbitmap
>>>>>>>>>
>>>>>>>>> https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-setbitmapbits
>>>>>>>>>
>>>>>>>>> https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-getdibits
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> -phil.
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/2d-dev/attachments/20200611/787719b1/attachment-0001.htm>