+1

Just curious about what performance degradation we will have in the worst case.

On 6/10/20 3:57 pm, Kevin Rushforth wrote:
+1

Code changes look good.

I verified that the bug happens on my Windows 10 laptop with SwingSet2 and with 
the LargeWindowPaintTest without the patch, and everything looks good with the 
patch.

-- Kevin


On 6/10/2020 1:48 PM, Philip Race 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.




--
Best regards, Sergey.

Reply via email to