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

Philip Race philip.race at oracle.com
Thu Jun 11 18:46:01 UTC 2020 

one more update to the tests
cr.openjdk.java.net/~prr/8240654.2/

I added
@requires vm.gc.Z per the VM folks, ZGC needs Windows Server 2019 or the 
same vintage Window 10. if run on Windows Server 2016
ZGC errors out. This should prevent that. -phil.
On 6/11/2020 11:13 AM, Kevin Rushforth wrote:
> +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/983f3eb7/attachment-0001.htm>

More information about the 2d-dev mailing list