[OpenJDK 2D-Dev] <Swing Dev> [11] JDK-8153732: Windows remote printer changes do not reflect in lookupPrintServices()

[Phil Race]

I thought you were going to make the refresh time 4 minutes ?
I don't see that it has to be the same on Windows as it was on Unix,
if you say 4 minutes is a sensible value there .. and 4 mins will be
less CPU wake up, so I'd back that (4 mins) ahead of 2 minutes as the 
default here.

You still have missing white space

432 while(true) {


With these two changes you have my +1 ..

BTW the native diff looks MUCH better now - thanks !

-phil.

On 06/21/2018 01:33 PM, Shashidhara Veerabhadraiah wrote:
>
> Hi Phil, Here is the new Webrev for changes you suggested.
>
> http://cr.openjdk.java.net/~sveerabhadra/8153732/webrev.03/ 
> <http://cr.openjdk.java.net/%7Esveerabhadra/8153732/webrev.03/>
>
> Thanks and regards,
>
> Shashi
>
> *From:*Phil Race
> *Sent:* Friday, June 22, 2018 1:02 AM
> *To:* Shashidhara Veerabhadraiah 
> <shashidhara.veerabhadraiah at oracle.com>; 2d-dev <2d-dev at openjdk.java.net>
> *Subject:* Re: <Swing Dev> [11] JDK-8153732: Windows remote printer 
> changes do not reflect in lookupPrintServices()
>
> +        private static final long DELAY = 1000 * 60 * 4; // 4 min pooling
> I think we need a System Property that can control this.
> I suggest the name "sun.java2d.print.minRefreshTime" which is what we 
> use on Unix.
> and similarly to there we should have "sun.java2d.print.polling" which 
> is a boolean
> and controls whether we do this at all ..
> See 
> src/java.desktop/unix/classes/sun/print/PrintServiceLookupProvider.java
> Lots of places where you are missing a space before "("
> +            if(str1.length != str2.length) {
> +                for(int i = 0;i < str1.length;i++) {
> +                    for(int j = 0;j < str2.length;j++) {
> +                        if(!str1[i].equals(str2[j])) {
> +            while(true) {
> +                if(doCompare(prevRemotePrinters, 
> currentRemotePrinters)) {
> There's some of that in native too
> 266                 if(info4->Attributes & 0x00000010) {
> In 
> http://cr.openjdk.java.net/~sveerabhadra/8153732/webrev.01/src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp.sdiff.html 
> <http://cr.openjdk.java.net/%7Esveerabhadra/8153732/webrev.01/src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp.sdiff.html>
> you seem to have moved all the existing and (I think) unchanged 
> related functions so the DIFF appears
> much bigger than it really is. Please move them back and re-generate.
> The test really needs to provide an "out" for someone running the test 
> who has no way
> to add a network printer .. they don't want to have to fail the test.
> As well as that, arguably this should be an @ignore test, so it is not 
> run unless
> you are trying to run all the tests.
> -phil.
>
> On 06/21/2018 12:08 PM, Shashidhara Veerabhadraiah wrote:
>
>     Hi Phil, Here is the new Webrev. I chose 4 mins because I think it
>     takes around 2 mins to add a new network printer, hence I felt 4
>     mins is a good time.
>
>     The windows **PrinterChangeNotifications** calls are a blocking
>     function calls hence I could not add the remote printers monitor
>     to the existing thread. Hence there is a new thread being added to
>     listen to remote printers status changes.
>
>     http://cr.openjdk.java.net/~sveerabhadra/8153732/webrev.01/
>     <http://cr.openjdk.java.net/%7Esveerabhadra/8153732/webrev.01/>
>
>     Thanks and regards,
>
>     Shashi
>
>     *From:*Philip Race
>     *Sent:* Thursday, June 21, 2018 5:38 AM
>     *To:* 2d-dev <2d-dev at openjdk.java.net>
>     <mailto:2d-dev at openjdk.java.net>; Shashidhara Veerabhadraiah
>     <shashidhara.veerabhadraiah at oracle.com>
>     <mailto:shashidhara.veerabhadraiah at oracle.com>
>     *Subject:* Fwd: Re: <Swing Dev> [11] JDK-8153732: Windows remote
>     printer changes do not reflect in lookupPrintServices()
>
>     The main concern I have is we now have a busy thread burning CPU ..
>     bad for laptops .. and if we add a delay we have less prompt
>     notification
>     of a new local printer.
>
>     I think the compromise is that the existing thread maybe kept as is,
>     and we add a new thread that pools every 10 minutes for a remote
>     printer.
>
>     If we can make the existing thread wake up from its wait and do
>     that, even better.
>
>     -phil.
>
>     -------- Original Message --------
>
>     *Subject: *
>
>     	
>
>     Re: [11] JDK-8153732: Windows remote printer changes do not
>     reflect in lookupPrintServices()
>
>     *Date: *
>
>     	
>
>     Wed, 20 Jun 2018 17:03:56 -0700
>
>     *From: *
>
>     	
>
>     Philip Race <philip.race at oracle.com> <mailto:philip.race at oracle.com>
>
>     *Organization: *
>
>     	
>
>     Oracle Corporation
>
>     *To: *
>
>     	
>
>     Shashidhara Veerabhadraiah <shashidhara.veerabhadraiah at oracle.com>
>     <mailto:shashidhara.veerabhadraiah at oracle.com>
>
>     *CC: *
>
>     	
>
>     awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>,
>     swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
>
>
>
>     This is on the wrong lists. Not Swing. Not AWT. Should be 2d.
>     I'll forward it there and continue there. Consider the AWT+Swing
>     threads dead.
>
>     -phil.
>
>     On 6/20/18, 3:12 AM, Shashidhara Veerabhadraiah wrote:
>
>         Hi All, Please review this code changes for the below enhancement.
>
>         Enhancement: https://bugs.openjdk.java.net/browse/JDK-8153732
>
>         Webrev:
>         http://cr.openjdk.java.net/~sveerabhadra/8153732/webrev.00/
>         <http://cr.openjdk.java.net/%7Esveerabhadra/8153732/webrev.00/>
>
>         Details of the changes: Windows provides
>         *PrinterChangeNotification* functions that provides
>         information about printer status changes of the local
>         printers(subset) but not network printers.
>         Alternatively, Windows provides a way thro' which one can get
>         the network printer status changes by using WMI,
>         RegistryKeyChange combination, which is a slightly complex
>         mechanism.
>         The Windows WMI offers an async and sync method to read thro'
>         registry via the WQL query. The async method is considered
>         dangerous as it leaves open a channel until we close it. But
>         the async method has the advantage of being notified of a
>         change in registry by calling callback without polling for
>         it. The sync method uses the polling mechanism to notify.
>         RegistryValueChange cannot be used in combination with WMI to
>         get registry value change notification because of an error
>         that may be generated because the scope of the query would be
>         too big to handle(at times).
>         Hence an alternative mechanism is choosen via the EnumPrinters
>         by polling for the count of printer status changes(add\remove)
>         and based on it update the printers list(both local and remote
>         printers - superset).
>
>         Thanks and regards,
>
>         Shashi
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20180621/2754b74d/attachment-0001.html>