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 <[email protected]>; 2d-dev <[email protected]> *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 <[email protected]>
    <mailto:[email protected]>; Shashidhara Veerabhadraiah
    <[email protected]>
    <mailto:[email protected]>
    *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 <[email protected]> <mailto:[email protected]>

    *Organization: *

        

    Oracle Corporation

    *To: *

        

    Shashidhara Veerabhadraiah <[email protected]>
    <mailto:[email protected]>

    *CC: *

        

    [email protected] <mailto:[email protected]>,
    [email protected] <mailto:[email protected]>



    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


Reply via email to