Re: Review request for JDK-8030099 Memory usage of java process increases after pressing start button in test window

2015-11-15 Thread Rajeev Chamyal 
Hello Phil,

I have updated the test case as per your review comments.
Could you please review it.
http://cr.openjdk.java.net/~rchamyal/8030099/webrev.02/

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 12 November 2015 16:02
To: Rajeev Chamyal; Philip Race; Alexander Scherbatiy; 
swing-dev@openjdk.java.net
Subject: Re: Review request for JDK-8030099 Memory usage of java process 
increases after pressing start button in test window

Still looks fine to me.

On 02.11.15 19:19, Rajeev Chamyal wrote:
> Hello Phil,
>
> Please review the updated webrev.
> http://cr.openjdk.java.net/~rchamyal/8030099/webrev.02/
>
> The test case has been updated as per review comments. Added tests for both 
> Parallel and default collector.
>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Phil Race
> Sent: 30 October 2015 01:10
> To: Rajeev Chamyal
> Cc: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net
> Subject: Re: Review request for JDK-8030099 Memory usage of java 
> process increases after pressing start button in test window
>
> It is arguably most important to run this with the *default* collector.
> If that changes to G1 (I think it is at least temporarily so changed in JDK 
> 9) then we will never see problems in the case people end up using.
>
> But you obviously also want to test the case that shows the bug.
>
> So I would say the best option is to test both : explicit parallel GC and 
> again with default .. even if they are the same there should be no harm done.
>
> You may want to increase timeout if running twice but you will have to verify 
> that.
>
> -phil.
>
>
> On 10/29/2015 05:48 AM, Rajeev Chamyal wrote:
>> Hello Sergey,
>>
>> Please review the updated test case.
>> Webrev : http://cr.openjdk.java.net/~rchamyal/8030099/webrev.01/
>>
>> As this issue is reported for Parallel GC collector. So, I have added 
>> -XX:+UseParallelGC to child process VM arguments in test case.
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -Original Message-
>> From: Sergey Bylokhov
>> Sent: 29 October 2015 02:17
>> To: Rajeev Chamyal; Philip Race
>> Cc: Alexander Scherbatiy; swing-dev@openjdk.java.net
>> Subject: Re: Review request for JDK-8030099 Memory usage of java 
>> process increases after pressing start button in test window
>>
>> Thanks for clarification.
>> The fix looks fine.
>>
>> On 20.10.15 11:49, Rajeev Chamyal wrote:
>>> Hello Sergey,
>>>
>>> I mentioned incorrectly that JDK8 is using CMS as default garbage 
>>> collector. JDK 8 uses Parallel GC as default collector.
>>>
>>> Following are the results with  Parallel GC as default collector in JDK9.
>>>
>>> Max java process size before fix while running test case(webrev) : 220MB 
>>> (approx.)  JDK version build 1.9.0-ea-b86.
>>> Max java process size after fix while running test case(webrev) : 
>>> 130MB(approx.). JDK9 version 1.9.0-internal-_2015_10_15_21_12-b00.
>>>
>>> Regards,
>>> Rajeev Chamyal
>>>
>>> -Original Message-
>>> From: Rajeev Chamyal
>>> Sent: 20 October 2015 12:10
>>> To: Sergey Bylokhov; Philip Race
>>> Cc: Alexander Scherbatiy; swing-dev@openjdk.java.net
>>> Subject: RE: Review request for JDK-8030099 Memory usage of java 
>>> process increases after pressing start button in test window
>>>
>>> Hello Sergey,
>>>
>>> Below are my observation while testing the fix.
>>>
>>> Max java process size before fix while running test case(webrev) : 170MB 
>>> (approx.)  JDK version build 1.9.0-ea-b86.
>>> Max java process size after fix while running test case(webrev) : 
>>> 165MB(approx.). JDK9 version 1.9.0-internal-_2015_10_15_21_12-b00.
>>> The results are not consistent always in both cases.
>>>
>>> We cannot compare these results with JDK8 because it uses CMS collector as 
>>> default garbage collector while JDK9 is using G1 collector.
>>> G1 collector is giving better results because it does heap compaction as 
>>> well.
>>>
>>> We are using different reference types  in different files ( files 
>>> registering instances with Disposer) e.g. StrikeCache uses SoftReferences.
>>>
>>> Regards,
>>> Rajeev Chamyal
>>>
>>> -Original Message-
>>> From: Sergey Bylokhov
>>> Sent: 20 October 2015 03:54
>>> To: Philip Race; Rajeev Chamyal
>>> Cc: Alexander Scherbatiy; swing-dev@openjdk.java.net
>>> Subject: Re: Review request for JDK-8030099 Memory usage of java 
>>> process increases after pressing start button in test window
>>>
>>> On 20.10.15 0:50, Philip Race wrote:
 The code change looks fine. The test is delightfully complicated.
>>> I still have not got a benefit of weak references in this use case. Does it 
>>> mean that weak are always better? If yes, then we should at some point 
>>> change default policy for all other cases as well.
>>>
 Did you run it under jtreg ?

 -phil.


 On 10/19/15, 5:10 AM, Rajeev Chamyal wrote:
> Hello,
>
> Please review the following fix for Jdk9:
> Bug:

Re: Review request for JDK-6288609 JInternalFrame.setDefaultCloseOperation() interferes with "close" behavior

2015-11-15 Thread prasanta sadhukhan 

Hi Rajeev,

The fix looks ok to me.
   But,  Don't we need to put this testcode in EDT?

JInternalFrame internalFrame = desktopPane.getSelectedFrame();
 133 internalFrame.doDefaultCloseAction();
 134 internalFrame = desktopPane.getSelectedFrame();

Also, there are wildcard imports in test?
Also, there is a repetition of code in lin119-130.

Regards
Prasanta
On 11/12/2015 4:37 PM, Alexander Scherbatiy wrote:


  The fix looks good to me.

  Thanks,
  Alexandr.

On 11/12/2015 12:05 PM, Rajeev Chamyal wrote:

Hello Alexander,

At all other places we call firePropertyChange(IS_CLOSED_PROPERTY, 
Boolean.FALSE,Boolean.TRUE); before calling dispose. Which will 
result in unselecting and closing the frame.
If API user is extending the InternalFrameUI classes then they have 
to care of selection/deselection themselves.
I have tested the fix with all look and feels on Windows and Ubuntu  
and on MAC with Aqua LAF as well.


Regards,
Rajeev Chamyal

-Original Message-
From: Alexander Scherbatiy
Sent: 06 November 2015 19:41
To: Rajeev Chamyal
Cc: Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re: Review request for JDK-6288609 
JInternalFrame.setDefaultCloseOperation() interferes with "close" 
behavior


On 10/29/2015 10:41 AM, Rajeev Chamyal wrote:

Hello All,

Please review the following fix for Jdk9:
   *Bug*:https://bugs.openjdk.java.net/browse/JDK-6288609

*Webrev*:http://cr.openjdk.java.net/~rchamyal/6288609/webrev.00/


*Issue*: On disposing the Top level JInternalFrame focus is not
shifting to the JInternalFrame below it.

*Cause*: Dispose method is changing the selection of currently closing
frame and then it calls the DefaultDeskTopManager:closeFrame which

finds the JInternalFrame below the closing frame based on selection of
the closing frame and then shifts the focus to frame below it.

Since selection is already changed by dispose method
DefaultDeskTopManager:closeFrame is unable to find reference to
previous frame.

*Fix*: Removed the selection change code from Dispose method.
 Are there any cases that the JInternalFrame is still selected 
after the IS_CLOSED_PROPERTY is fired in the dispose() method so it 
is still necessary to set the selection to false?


Thanks,
Alexandr.


   Regards,
Rajeev Chamyal

Re: Review for 8132770: Test javax/swing/JRadioButton/FocusTraversal/FocusTraversal.java fails in MacOSX

2015-11-15 Thread Avik Niyogi 
Hi All,

The bug fix has been updated as per the comments.
http://cr.openjdk.java.net/~rchamyal/avik/8132770/webrev.01/ 



With Regards,
Avik Niyogi

> On 13-Nov-2015, at 9:46 am, Avik Niyogi  wrote:
> 
> Hi Rajeev,
> 
> Will fix and create webrev.01 with inputs provided. Also, we have verified on 
> Mac OS X, Windows and Ubuntu.
> 
> With Regards,
> Avik Niyogi
> 
>> On 12-Nov-2015, at 4:52 pm, Rajeev Chamyal > > wrote:
>> 
>> Hello Avik,
>>  
>> 1)  In the tryLookAndFeel exception handling is skipped. createUI and 
>> other calls should be skipped in case of exception..
>> 2)  What all platforms you have tested the bug?
>>  
>> Regards,
>> Rajeev Chamyal
>>  
>> From: Avik Niyogi 
>> Sent: 04 November 2015 14:51
>> To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net 
>> 
>> Subject: Review for 8132770: Test 
>> javax/swing/JRadioButton/FocusTraversal/FocusTraversal.java fails in MacOSX
>>  
>> Hi All,
>>  
>> Kindly review the fix for JDK9.
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8132770 
>> 
>>  
>> Webrev: http://cr.openjdk.java.net/~rchamyal/avik/8132770/webrev.00/ 
>> 
>>  
>> Issue: Test javax/swing/JRadioButton/FocusTraversal/FocusTraversal.java 
>> fails in MacOSX where non-eligible Look and Feels are not taken into account.
>>  
>> Cause: TestCase flow for non-Basic JRadioButton UI such as Aqua, Nimbus and 
>> GTK have different focus traversal track as compared to the rest and require 
>> different test case flow with regards to focus owner.
>>  
>> Fix: Modified test case with appropriate deviant test case for Look and 
>> Feels which are not eligible for focus traversal as expected previously in 
>> test file.
>>  
>> With Regards,
>> Avik Niyogi
>

Re: RFR: [9] [JDK-8081491] The case print incomplete.

2015-11-15 Thread Rajeev Chamyal 
The fix looks good to me.

 

Regards,

Rajeev Chamyal

 

Subject: 

Re:  RFR: [9] [JDK-8081491] The case print incomplete.

Date: 

Fri, 16 Oct 2015 13:19:37 +0300

From: 

Alexander Scherbatiy HYPERLINK 
"mailto:alexandr.scherba...@oracle.com;

To: 

prasanta sadhukhan HYPERLINK 
"mailto:prasanta.sadhuk...@oracle.com;

CC: 

Phil Race HYPERLINK "mailto:philip.r...@oracle.com;, 
Sergey Bylokhov HYPERLINK 
"mailto:sergey.bylok...@oracle.com;, HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net

 

  The fix looks good to me.
 
  Thanks,
  Alexandr.
 
On 10/14/2015 2:26 PM, prasanta sadhukhan wrote:
> 
> 
> On 10/9/2015 3:00 PM, Alexander Scherbatiy wrote:
>> On 10/7/2015 3:10 PM, prasanta sadhukhan wrote:
>>> Hi All,
>>> 
>>> @Sergey,Phil,Alexander Z
>>> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.10/
>> 
>> Have you checked the corner cases like printing a table which has 
>> only one row (with and without a scroll pane)?
>> Could you add these scenarios to the test?
>> 
> Yes, I have checked this cases. JTable with 1 row will not show a 
> scrollpane so it will be same as a table with 1 row without a scrollpane.
> I have added one row without scrollpane subtest
> 
> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.11/
> 
> Regards
> Prasanta
>> Thanks,
>>    Alexandr,
>> 
>>> 
>>> Any more comments/feedback? Can I get a +1 for this?
>>> 
>>> Regards
>>> Prasanta
 
 
 On 09/25/2015 04:47 AM, prasanta sadhukhan wrote:
> Thanks Alexander. Can I get a +1 for this?
> 
> Regards
> Prasanta
> On 9/25/2015 2:43 PM, Alexander Scherbatiy wrote:
>> 
>>   The fix looks good to me.
>> 
>>   Thanks,
>>   Alexandr.
>> 
>> On 9/25/2015 9:26 AM, prasanta sadhukhan wrote:
>>> Added null check for SwingUtilities.getUnwrappedParent(table).
>>> Please review the updated webrev
>>> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.08/
>>> 
>>> Regards
>>> Prasanta
>>> On 9/24/2015 5:19 PM, Alexander Scherbatiy wrote:
 On 9/23/2015 12:26 PM, prasanta sadhukhan wrote:
> 
> 
> On 9/23/2015 2:46 PM, Alexander Scherbatiy wrote:
>> On 9/23/2015 9:42 AM, prasanta sadhukhan wrote:
>>> I have updated the code as per your comment.
>>> Please review this webrev
>>> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.07/
>> 
>>    - Is it possible that 
>> SwingUtilities.getUnwrappedParent(table) returns null?
> I have not seen it. It will return at least JRootPane, I 
> guess.
 
    Is it possible just crate a JTable with some rows and 
 print it, without adding to a frame or some others components?
 
    Thanks,
    Alexandr.
>>    - Does the fix work correctly for a case when rMax has 
>> initial zero value but it is decremented on line 1857?
> rMax can have -1 from table.rowAtPoint() in which case it 
> will be changed to total rowCount so it will not be 0 
> before line 1857.
> 
> Regards
> Prasanta
>> 
>>   Thanks,
>>   Alexandr.
>> 
>>> 
>>> Regards
>>> Prasanta
>>> On 9/22/2015 7:01 PM, Alexander Scherbatiy wrote:
 On 9/21/2015 12:05 PM, prasanta sadhukhan wrote:
> 
> 
> On 9/21/2015 2:20 PM, Alexandr Scherbatiy wrote:
>> 18.09.2015 10:16, prasanta sadhukhan пишет:
>>> 
>>> 
>>> On 9/17/2015 8:18 PM, Alexander Scherbatiy wrote:
 On 9/16/2015 2:04 PM, prasanta sadhukhan wrote:
> Hi Alexander, Sergey,
> 
> Waiting for your review on this.
> http://cr.openjdk.java.net/~psadhukhan/8081491/webrev.06/ 
> 
 
    Could you describe why the paint artifacts are 
 drawn when a scroll pane is present?
>>> I see that normally JTable has always been 
>>> associated with JScrollpane and it uses
>>> // Paint the grid.
>>>  paintGrid(g, rMin, rMax, cMin, cMax);
>>> // Paint the cells.
>>> paintCells(g, rMin, rMax, cMin, cMax);
>>> to paint the cells in the table.
>>> When we scroll the table, rMin can be say 41 and 
>>> rMax can be 43 so it expects to