Re: Review request: 8055856: checkdeps build target doesn't work for cross-compilation builds

2014-08-27 Thread Erik Joelsson

Hello Mandy,

That certainly looks better. A couple of more thoughts, and sorry for 
not pointing this out earlier, but the new structure is still new to me too.


* The rmic targets also generate classes, so for modules.xml to be 
correct, I suspect you need to depend on that too. Simply add "rmic" 
after java on the dependency line. I assume the verification doesn't 
care about resources? If it does, then you would also need to depend on 
the rest of gendata, something like $(filter-out jdk.dev-gendata, 
$(GENDATA_TARGETS)).


* In Gendata-jdk.dev.gmk, there is an ifndef OPENJDK. We are trying to 
move away from that construct when possible. It's a bit cumbersome but 
to avoid it. To do it in the current model, create a closed version of 
Gendata-jdk.dev.gmk. Add "$(eval $(call IncludeCustomExtension, jdk, 
gendata/Gendata-jdk.dev.gmk))" after "include GendataCommon.gmk". Change 
the first assignment of METADATA_FILES to += and move the closed 
addition to the closed version of the file. There is no need for ifndef 
OPENJDK in the closed file. It only gets included when we build closed.


/Erik

On 2014-08-27 18:00, Mandy Chung wrote:

Erik, Magnus,

This is much easier than I have thought.  I really like this new build.
I have separated out Gendata-jdk.dev.gmk and removed the modules-xml
target completely.

Webrev at:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055856/webrev.01/

Mandy





Re: Review request for JDK-8051540: Convert JAXP functin tests: org.xml.sax to jtreg (testNG) tests

2014-08-27 Thread huizhe wang
The patches I mentioned would disable security manager or grant specific 
permissions needed to run those code, so that no test needs to be 
excluded from a target (whether with or without security manager).


Thanks,
Joe

On 8/27/2014 8:42 PM, Frank Yuan wrote:


Hi Joe

The test Tristan mentioned that is unable to run with security manager 
is 
http://sqe-hgi.us.oracle.com/hg/index.cgi/testbase/javase/functional/9/xml/file/3f7ae9b99933/src/JAXP/unittests/unit-test/bug6513892 
, it is used to test xslt redirect extension. When it runs with 
security manager, fails due to;


/java.lang.RuntimeException: Use of the extension element 'redirect' 
is not allowed when the secure processing feature is set to true./


Anyway, Jibing would communicate with sustaining team about their 
patch, the discussion is in another mail chain.


Best Regards

Frank

-Original Message-
From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On 
Behalf Of huizhe wang

Sent: Thursday, August 28, 2014 7:39 AM
To: Tristan Yan
Cc: Core-Libs-Dev
Subject: Re: Review request for JDK-8051540: Convert JAXP functin 
tests: org.xml.sax to jtreg (testNG) tests


On 8/27/2014 4:03 PM, Tristan Yan wrote:

> Hi Joe and others

>

> I updated the tests with putting them in jaxp repo. I also run these

> tests with security manager and they all passed

> http://cr.openjdk.java.net/~tyan/JDK-8051540/webrev01/ 



> 

>

Awesome.

> Also I’d like to propose our way for handling jaxp tests run with

> security manager.  The way we’d use is creating two targets for

> running jaxp tests. One is for normal run; which will run all the

> tests without security manager. One is secure run; the target only run

> the tests that have to be run with security manager. This could be

> easy to be handled with adding two targets in makefile. And for most

> of people they only care about the function. They only need run normal

> run target. We would run two targets for any of our formal tests like

> nightly, ci build and jprt tests.

Yes, please coordinate with Frank and Eric so that all of the jaxp 
tests share the same configuration.


> For the tests which can not be run in secure mode(like tests for xsltc

> direct extension), we'd add testng group called “secure-hostile”. We

> won’t run these tests in secure mode by bypassing them in secure run

> target. By this way we could easily transform our tests as usual

> without additional effort.

I had been previously updated them so that all of the tests were 
capable of running with and without security manager. Sustaining SQE 
had invested several month to incorporate the changes into that hosted 
in Aurora. Please consider taking the patches from them if you haven't 
already done so.


Thanks,

Joe

>

> Thank you

> Tristan

>

>> On Aug 19, 2014, at 10:32 AM, huizhe wang > > wrote:

>>

>> By the way, the plan has been that all of the JAXP SQE and Unit tests

>> be migrated into [openjdk]/jaxp repo under jaxp/test. Tests currently

>> in the jdk repo shall be moved to jaxp/test as well. I see that your

>> webrev was generated in jdk9/dev/jdk. I hope it doesn't mean you're

>> checking tests into the jdk repo.

>>

>> Thanks,

>> Joe

>>

>> On 8/18/2014 4:42 PM, Tristan Yan wrote:

>>> Thanks Joe

>>> We intend to replace the base class with test library because that

>>> doesn’t look like a real base class but an utilities class.

>>> I haven’t tried to run these tests with security manager, I will run

>>> them with security manager then get back you soon.

>>> Thank you.

>>> Tristan

>>>

 On Aug 18, 2014, at 4:32 PM, huizhe wang >>> > wrote:





>>>

>>

>





Re: RFR: 8049343: (tz) Support tzdata2014f

2014-08-27 Thread Masayoshi Okutsu

 src/java.base/share/classes/sun/util/resources/TimeZoneNames.java:

 239 String SLST[] = new String[] {"Greenwich Mean Time", "GMT",
 240   "Sierra Leone Summer Time", "SLST",
 241   "Sierra Leone Time", "SLT"};

 251 String WART[] = new String[] {"Western Argentine Time", "WART",
 252   "Western Argentine Summer Time", 
"WARST",
 253   "Western Argentine Time", "WART"};

It seems these are no longer used.

-{"Antarctica/Macquarie", new String[] {"Macquarie Island Time", 
"MIST",
-   "Macquarie Island Summer Time", 
"MIST",
+{"Antarctica/Macquarie", new String[] {"Macquarie Island Standard Time", 
"MIST",
+   "Macquarie Island Daylight Time", 
"MIST",

The daylight saving time abbreviation should be MIDT.

Thanks,
Masayoshi

On 8/27/2014 10:53 PM, Aleksej Efimov wrote:

Masayoshi,
Thank you for a detailed review - I tried to address all your 
comments. Please, see the updated review: 
http://cr.openjdk.java.net/~aefimov/8049343/9/webrev.03
About the workarounds - I will start discussion with Sherman when the 
tzdata2014f (and I suppose the 2014g - it will be available soon 
according to tzdata mail-list [1]) integration will be complete.


-Aleksej

[1] tzdata2014g is coming: 
http://mm.icann.org/pipermail/tz/2014-August/021528.html


On 08/27/2014 12:34 PM, Masayoshi Okutsu wrote:

Here are additional comments.

src/java.base/share/classes/sun/util/calendar/ZoneInfoFile.java:

I'm concerned about the workarounds. It's not new in this update, but 
this problem would make tzupdater data void until the workaround is 
added to the next update release. Could you please work with Sherman 
to eliminate the workaround (outside of this 2014f update)?


src/java.base/share/classes/sun/util/resources/TimeZoneNames.java:

 String LORD_HOWE[] = new String[] {"Lord Howe Standard 
Time", "LHST",
-   "Lord Howe Summer Time", 
"LHST",
+   "Lord Howe Summer Time", 
"LHDT",


The S-D convention is applied to Lord Howe as well.

 567 {"Antarctica/Macquarie", new String[] {"Macquarie 
Island Time", "MIST",

 568 "Macquarie Island Summer Time", "MIST",
 569 "Macquarie Island Time", "MIST"}},

This one should also follow the S-D convention, although this time 
zone doesn't observe daylight saving time.



+String XJT[] = new String[] {"China Standard Time", "XJT",
+ "China Daylight Time", "XJDT",
+ "China Time", "XJT"};

Should the long names be based on Xinjiang?

Africa/Freetown is now a link to Africa/Abidjan. Those should be the 
same time zone.


-{"America/Metlakatla", new String[] {"Metlakatla 
Standard Time", "MeST",
- "Metlakatla 
Daylight Time", "MeDT",
- "Metlakatla Time", 
"MeT"}},
+{"America/Metlakatla", new String[] {"Metlakatla 
Standard Time", "PST",
+ "Metlakatla 
Daylight Time", "PDT",
+ "Metlakatla Time", 
"PT"}},


-{"Europe/Volgograd", new String[] {"Volgograd Time", 
"VOLT",
-   "Volgograd Summer 
Time", "VOLST",
-   "Volgograd Time", 
"VOLT"}},

+{"Europe/Volgograd", new String[] {"Volgograd Time", "MSK",
+   "Volgograd Summer 
Time", "MSK",
+   "Volgograd Time", 
"MSK"}},



The long names should be changed accordingly.

Thanks,
Masayoshi

On 8/22/2014 9:17 PM, Aleksej Efimov wrote:

Masayoshi,
You're right that the "root names" should be changed as part of this 
update. The names were changed according to Australian official 
document here: http://australia.gov.au/about-australia/our-country/time
The fixed version of the webrev can be found here: 
http://cr.openjdk.java.net/~aefimov/8049343/9/webrev.02


Thanks,
Aleksej


On 08/21/2014 03:51 PM, Masayoshi Okutsu wrote:
We used to make name changes in the root (base) bundle as part of 
time zone data maintenance and deter only translations. But if you 
want to handle name changes later, that would be fine. It's your call.


Thanks,
Masayoshi

On 8/21/2014 5:05 PM, Aleksej Efimov wrote:

Masayoshi,
I agree that we should change the long names to match the new 
short names introduced by tzdata.
But I suggest to log a different CR for such activity to handle 
long name changes and their translations (this activity is 
slightly out of tzdata update scope). Does it make sense?


-Aleksej

On 08/21/

RE: Review request for JDK-8051540: Convert JAXP functin tests: org.xml.sax to jtreg (testNG) tests

2014-08-27 Thread Frank Yuan
Hi Joe

 

The test Tristan mentioned that is unable to run with security manager is 
http://sqe-hgi.us.oracle.com/hg/index.cgi/testbase/javase/functional/9/xml/file/3f7ae9b99933/src/JAXP/unittests/unit-test/bug6513892
 , it is used to test xslt redirect extension. When it runs with security 
manager, fails due to;

java.lang.RuntimeException: Use of the extension element 'redirect' is not 
allowed when the secure processing feature is set to true.

 

Anyway, Jibing would communicate with sustaining team about their patch, the 
discussion is in another mail chain. 

 

Best Regards

Frank

 

 

-Original Message-
From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf 
Of huizhe wang
Sent: Thursday, August 28, 2014 7:39 AM
To: Tristan Yan
Cc: Core-Libs-Dev
Subject: Re: Review request for JDK-8051540: Convert JAXP functin tests: 
org.xml.sax to jtreg (testNG) tests

 

 

On 8/27/2014 4:03 PM, Tristan Yan wrote:

> Hi Joe and others

> 

> I updated the tests with putting them in jaxp repo. I also run these 

> tests with security manager and they all passed 

>   
> http://cr.openjdk.java.net/~tyan/JDK-8051540/webrev01/

> <  
> http://cr.openjdk.java.net/%7Etyan/JDK-8051540/webrev01/>

> 

 

Awesome.

 

> Also I’d like to propose our way for handling jaxp tests run with 

> security manager.  The way we’d use is creating two targets for 

> running jaxp tests. One is for normal run; which will run all the 

> tests without security manager. One is secure run; the target only run 

> the tests that have to be run with security manager. This could be 

> easy to be handled with adding two targets in makefile. And for most 

> of people they only care about the function. They only need run normal 

> run target. We would run two targets for any of our formal tests like 

> nightly, ci build and jprt tests.

 

Yes, please coordinate with Frank and Eric so that all of the jaxp tests share 
the same configuration.

 

> For the tests which can not be run in secure mode(like tests for xsltc 

> direct extension), we'd add testng group called “secure-hostile”. We 

> won’t run these tests in secure mode by bypassing them in secure run 

> target. By this way we could easily transform our tests as usual 

> without additional effort.

 

I had been previously updated them so that all of the tests were capable of 
running with and without security manager. Sustaining SQE had invested several 
month to incorporate the changes into that hosted in Aurora. Please consider 
taking the patches from them if you haven't already done so.

 

Thanks,

Joe

 

> 

> Thank you

> Tristan

> 

>> On Aug 19, 2014, at 10:32 AM, huizhe wang > <  mailto:huizhe.w...@oracle.com>> wrote:

>> 

>> By the way, the plan has been that all of the JAXP SQE and Unit tests 

>> be migrated into [openjdk]/jaxp repo under jaxp/test. Tests currently 

>> in the jdk repo shall be moved to jaxp/test as well. I see that your 

>> webrev was generated in jdk9/dev/jdk. I hope it doesn't mean you're 

>> checking tests into the jdk repo.

>> 

>> Thanks,

>> Joe

>> 

>> On 8/18/2014 4:42 PM, Tristan Yan wrote:

>>> Thanks Joe

>>> We intend to replace the base class with test library because that 

>>> doesn’t look like a real base class but an utilities class.

>>> I haven’t tried to run these tests with security manager, I will run 

>>> them with security manager then get back you soon.

>>> Thank you.

>>> Tristan

>>> 

 On Aug 18, 2014, at 4:32 PM, huizhe wang >>> <  mailto:huizhe.w...@oracle.com>> wrote:

 

 

>>> 

>> 

> 

 



Re: Replace concat String to append in StringBuilder parameters

2014-08-27 Thread Ivan Gerasimov


On 28.08.2014 3:15, Wang Weijun wrote:

OK, I'll remember that.

Thanks.


  So you will include the StringBuilder changes into your fix?

No. I reimplemented MimeEntry.toProperty() with StringJoiner:
http://hg.openjdk.java.net/jdk9/dev/jdk/diff/8be081fb8db1/src/java.base/share/classes/sun/net/www/MimeEntry.java

Sincerely yours,
Ivan



Re: Review request for JDK-8051540: Convert JAXP functin tests: org.xml.sax to jtreg (testNG) tests

2014-08-27 Thread huizhe wang


On 8/27/2014 4:03 PM, Tristan Yan wrote:

Hi Joe and others

I updated the tests with putting them in jaxp repo. I also run these 
tests with security manager and they all passed
http://cr.openjdk.java.net/~tyan/JDK-8051540/webrev01/ 





Awesome.

Also I’d like to propose our way for handling jaxp tests run with 
security manager.  The way we’d use is creating two targets for 
running jaxp tests. One is for normal run; which will run all the 
tests without security manager. One is secure run; the target only run 
the tests that have to be run with security manager. This could be 
easy to be handled with adding two targets in makefile. And for most 
of people they only care about the function. They only need run normal 
run target. We would run two targets for any of our formal tests like 
nightly, ci build and jprt tests.


Yes, please coordinate with Frank and Eric so that all of the jaxp tests 
share the same configuration.


For the tests which can not be run in secure mode(like tests for xsltc 
direct extension), we'd add testng group called “secure-hostile”. We 
won’t run these tests in secure mode by bypassing them in secure run 
target. By this way we could easily transform our tests as usual 
without additional effort.


I had been previously updated them so that all of the tests were capable 
of running with and without security manager. Sustaining SQE had 
invested several month to incorporate the changes into that hosted in 
Aurora. Please consider taking the patches from them if you haven't 
already done so.


Thanks,
Joe



Thank you
Tristan

On Aug 19, 2014, at 10:32 AM, huizhe wang > wrote:


By the way, the plan has been that all of the JAXP SQE and Unit tests 
be migrated into [openjdk]/jaxp repo under jaxp/test. Tests currently 
in the jdk repo shall be moved to jaxp/test as well. I see that your 
webrev was generated in jdk9/dev/jdk. I hope it doesn't mean you're 
checking tests into the jdk repo.


Thanks,
Joe

On 8/18/2014 4:42 PM, Tristan Yan wrote:

Thanks Joe
We intend to replace the base class with test library because that 
doesn’t look like a real base class but an utilities class.
I haven’t tried to run these tests with security manager, I will run 
them with security manager then get back you soon.

Thank you.
Tristan

On Aug 18, 2014, at 4:32 PM, huizhe wang > wrote:













Re: Replace concat String to append in StringBuilder parameters

2014-08-27 Thread Wang Weijun
OK, I'll remember that. So you will include the StringBuilder changes into your 
fix?

--Max

On Aug 28, 2014, at 2:10, Ivan Gerasimov  wrote:

> Hi Max!
> 
>> The core part is updated again at
>> 
>>   http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/
> 
> Can you please revert changes to 
> src/java.base/share/classes/sun/net/www/MimeEntry.java, as they're 
> conflicting with the fix for JDK-8054714?
> 
> Apologizing for adding you more work.
> 
> Sincerely yours,
> Ivan
> 



Re: Review request for JDK-8051540: Convert JAXP functin tests: org.xml.sax to jtreg (testNG) tests

2014-08-27 Thread Tristan Yan
Hi Joe and others

I updated the tests with putting them in jaxp repo. I also run these tests with 
security manager and they all passed
http://cr.openjdk.java.net/~tyan/JDK-8051540/webrev01/ 


Also I’d like to propose our way for handling jaxp tests run with security 
manager.  The way we’d use is creating two targets for running jaxp tests. One 
is for normal run; which will run all the tests without security manager. One 
is secure run; the target only run the tests that have to be run with security 
manager. This could be easy to be handled with adding two targets in makefile. 
And for most of people they only care about the function. They only need run 
normal run target. We would run two targets for any of our formal tests like 
nightly, ci build and jprt tests.
For the tests which can not be run in secure mode(like tests for xsltc direct 
extension), we'd add testng group called “secure-hostile”. We won’t run these 
tests in secure mode by bypassing them in secure run target. By this way we 
could easily transform our tests as usual without additional effort.

Thank you
Tristan 

> On Aug 19, 2014, at 10:32 AM, huizhe wang  wrote:
> 
> By the way, the plan has been that all of the JAXP SQE and Unit tests be 
> migrated into [openjdk]/jaxp repo under jaxp/test. Tests currently in the jdk 
> repo shall be moved to jaxp/test as well. I see that your webrev was 
> generated in jdk9/dev/jdk. I hope it doesn't mean you're checking tests into 
> the jdk repo.
> 
> Thanks,
> Joe
> 
> On 8/18/2014 4:42 PM, Tristan Yan wrote:
>> Thanks Joe
>> We intend to replace the base class with test library because that doesn’t 
>> look like a real base class but an utilities class.
>> I haven’t tried to run these tests with security manager, I will run them 
>> with security manager then get back you soon.
>> Thank you.
>> Tristan
>> 
>>> On Aug 18, 2014, at 4:32 PM, huizhe wang >> > wrote:
>>> 
>>> 
>> 
> 



Re: Impact of code difference in Collection#contains() worth improving?

2014-08-27 Thread John Rose
On Aug 27, 2014, at 6:48 AM, Fabian Lange  wrote:

> Hi all,
> I have been involved recently in some theoretical or nonsensical
> discussions about microbenchmarking, jit compiling assemblies and so fort.
> One example was LinkedList vs ArrayList.
> 
> What I noticed is that those two have a different implementation for
> contains():
> 
> ArrayList:
> 
>*public* *boolean* contains(Object o) {
>*return* indexOf(o) >= 0;
>}
> 
> LinkedList:
> 
>*public* *boolean* contains(Object o) {
>*return* indexOf(o) != -1;
>}
> 
> Logically this is of course identical due to the contract of contains which
> returns either -1 or the >=0 index of the element.
> 
> This code has been like this almost forever, and I was wondering if this
> actually makes a difference in CPU cycles.

If it makes a difference, (a) that difference is unimportant, and (b) it is not 
controllable at the source code level.

Ad (a), the only time the choice of an instruction or two would make even a 
slight difference is if the list is empty.  Otherwise, the cost of traversing 
the list and testing each element will swamp any small difference in the 
comparison.

Ad (b), the JIT will unrecognizably optimize both codes into surprising 
instruction sequences.  If they differ, the differences will be suprising, and 
depend on stuff you weren't looking at, such as branch frequencies or the 
complexity of nearby code structures.

> And in fact this code compiles into different assembler instructions. The
> array list does a test against 0 and conditional move, while the linked
> list does a jump equals on -1.

On Thursdays, they compile to the same thing.  On every full moon, they swap 
instruction sequences.  ...That's not strictly true, but you get the idea.  
Changing the Java source code operators in response to observed instruction 
sequences is a losing game, unless there is a large profit directly available.

So what's the winning game?  That might be to file a bug against the optimizer 
(server compiler) if you see truly bad instructions.  But the "badness" has to 
be more than speculative.  Unless it must has a measurable and significant 
cost, for plausibly applications (not nanobenchmarks), the bug will not (and 
should not) get time from compiler engineers.

Often, as with a bit of dust on your camera lens, the best course is not to 
touch it at all, for fear of making things worse.

— John

> Again that is not surprising, because the actual java source is different.
> But I wonder if both options are equally good in cold performance and when
> jitted based on parameter values.
> 
> Wouldn't one implementation be better than the other? And why is not the
> "better" implementation taken in both classes (and maybe other Collections
> which use indexOf) ?
> 
> Is the answer that this has always been like this and the benefit is not
> worth the risk of touching ancient code?
> 
> And if not for performance, would code clarify and similarity be an
> argument?
> 
> Or can the answer be found on a different mailing list? Then let me know :)
> 
> 
> Fabian



Re: Replace concat String to append in StringBuilder parameters

2014-08-27 Thread Ivan Gerasimov

Hi Max!


The core part is updated again at

   http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/


Can you please revert changes to 
src/java.base/share/classes/sun/net/www/MimeEntry.java, as they're 
conflicting with the fix for JDK-8054714?


Apologizing for adding you more work.

Sincerely yours,
Ivan



Re: [8u-dev] RFR: 8036981: JAXB not preserving formatting for xsd:any Mixed content

2014-08-27 Thread Aleksej Efimov

Thank you Lance!

-Aleksej
On 27.08.2014 19:56, Lance Andersen wrote:

I will review it by the end of the week


On Aug 27, 2014, at 9:45 AM, Aleksej Efimov > wrote:



Hi,
I'm adding a core-libs-dev mail list - I might be lucky to find a 
JDK8 reviewer for my fix there.


Thank you,
Aleksej

On 08/26/2014 06:12 PM, Aleksej Efimov wrote:

Hi Dalibor,

It's a partial backport (logged with a different bugID) of JDK9 fix, 
I got an approval from Miran (Thank you Miran!) and right now I'm 
waiting for a JDK8 reviewer approval.


-Aleksej

On 08/26/2014 04:58 PM, dalibor topic wrote:

Hi Aleksej - are you looking for approval to push into jdk8u-dev?

cheers,
dalibor topic

On 22.08.2014 22:01, Miroslav Kos wrote:

Hi Aleksej,
looks good to me.

Thanks
Miran


On 18/08/14 17:29, Aleksej Efimov wrote:

Hi,

Can I ask for a review of 8036981 bug [1] fix:
   JAXWS: 
http://cr.openjdk.java.net/~aefimov/8036981/8/webrev.00/jaxws/ 

   JDK: 
http://cr.openjdk.java.net/~aefimov/8036981/8/webrev.00/jdk/ 



The proposed fix is a partial backport of "Update JAX-WS RI
integration to latest version" bug [2] to JDK8.
The proposed patch fixes the broken formatting of the input xml 
during

JAXB marshalling/unmarshalling. Regression test is included.
Testing:
   - regtests: xml related tests from 'core' testset - no failures
   - JCK: api/xinclude api/xsl api/javax_xml api/org_xml xml_schema
api/xinclude - no failures

Thank you,
Aleksej

[1] https://bugs.openjdk.java.net/browse/JDK-8036981
[2] https://bugs.openjdk.java.net/browse/JDK-8044656
[3] 8044656 JDK9 review thread:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-June/027458.html 
















Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 







Re: Impact of code difference in Collection#contains() worth improving?

2014-08-27 Thread Mike Duigou
Hi Fabian;

The correct mailing list for this discussion would be 
core-libs-dev@openjdk.java.net

The difference between these two implementations is probably of not much 
consequence though it seems that one or the other could be promoted to 
AbstractList. This implementation would be marginally better than that offered 
by AbstractCollection.contains() by generally avoiding creation of an Iterator.

There is always some risk associated with making a change even when we believe 
that the regression testing adequately exercises the functionality. In this 
case the factors which have resulted in different implementations are; lack of 
attention, effort relative to benefit and the extremely small but non-zero risk.

A nano-benchmark would tell the tale of which of the two implementations is 
more efficient though I suspect that the difference is negligible if even 
measurable.

Mike

On Aug 27 2014, at 06:48 , Fabian Lange  wrote:

> Hi all,
> I have been involved recently in some theoretical or nonsensical
> discussions about microbenchmarking, jit compiling assemblies and so fort.
> One example was LinkedList vs ArrayList.
> 
> What I noticed is that those two have a different implementation for
> contains():
> 
> ArrayList:
> 
>*public* *boolean* contains(Object o) {
>*return* indexOf(o) >= 0;
>}
> 
> LinkedList:
> 
>*public* *boolean* contains(Object o) {
>*return* indexOf(o) != -1;
>}
> 
> Logically this is of course identical due to the contract of contains which
> returns either -1 or the >=0 index of the element.
> 
> This code has been like this almost forever, and I was wondering if this
> actually makes a difference in CPU cycles.
> 
> And in fact this code compiles into different assembler instructions. The
> array list does a test against 0 and conditional move, while the linked
> list does a jump equals on -1.
> 
> Again that is not surprising, because the actual java source is different.
> But I wonder if both options are equally good in cold performance and when
> jitted based on parameter values.
> 
> Wouldn't one implementation be better than the other? And why is not the
> "better" implementation taken in both classes (and maybe other Collections
> which use indexOf) ?
> 
> Is the answer that this has always been like this and the benefit is not
> worth the risk of touching ancient code?
> 
> And if not for performance, would code clarify and similarity be an
> argument?
> 
> Or can the answer be found on a different mailing list? Then let me know :)
> 
> 
> Fabian



Re: Review request: 8055856: checkdeps build target doesn't work for cross-compilation builds

2014-08-27 Thread Mandy Chung

Erik, Magnus,

This is much easier than I have thought.  I really like this new build.
I have separated out Gendata-jdk.dev.gmk and removed the modules-xml
target completely.

Webrev at:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055856/webrev.01/

Mandy



Re: [8u-dev] RFR: 8036981: JAXB not preserving formatting for xsd:any Mixed content

2014-08-27 Thread Lance Andersen
I will review it by the end of the week


On Aug 27, 2014, at 9:45 AM, Aleksej Efimov  wrote:

> Hi,
> I'm adding a core-libs-dev mail list - I might be lucky to find a JDK8 
> reviewer for my fix there.
> 
> Thank you,
> Aleksej
> 
> On 08/26/2014 06:12 PM, Aleksej Efimov wrote:
>> Hi Dalibor,
>> 
>> It's a partial backport (logged with a different bugID) of JDK9 fix, I got 
>> an approval from Miran (Thank you Miran!) and right now I'm waiting for a 
>> JDK8 reviewer approval.
>> 
>> -Aleksej
>> 
>> On 08/26/2014 04:58 PM, dalibor topic wrote:
>>> Hi Aleksej - are you looking for approval to push into jdk8u-dev?
>>> 
>>> cheers,
>>> dalibor topic
>>> 
>>> On 22.08.2014 22:01, Miroslav Kos wrote:
 Hi Aleksej,
 looks good to me.
 
 Thanks
 Miran
 
 
 On 18/08/14 17:29, Aleksej Efimov wrote:
> Hi,
> 
> Can I ask for a review of 8036981 bug [1] fix:
>JAXWS: http://cr.openjdk.java.net/~aefimov/8036981/8/webrev.00/jaxws/
>JDK: http://cr.openjdk.java.net/~aefimov/8036981/8/webrev.00/jdk/
> 
> The proposed fix is a partial backport of "Update JAX-WS RI
> integration to latest version" bug [2] to JDK8.
> The proposed patch fixes the broken formatting of the input xml during
> JAXB marshalling/unmarshalling. Regression test is included.
> Testing:
>- regtests: xml related tests from 'core' testset - no failures
>- JCK: api/xinclude api/xsl api/javax_xml api/org_xml xml_schema
> api/xinclude - no failures
> 
> Thank you,
> Aleksej
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8036981
> [2] https://bugs.openjdk.java.net/browse/JDK-8044656
> [3] 8044656 JDK9 review thread:
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-June/027458.html
>  
> 
> 
 
>>> 
>> 
> 



Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com





Re: Impact of code difference in Collection#contains() worth improving?

2014-08-27 Thread Martin Buchholz
The ArrayList version saves one byte of bytecode, and is therefore very
slightly better.  We should bless that version and use it consistently.


javap -c -private java.util.ArrayList | grep -A10 'boolean.*contains'
  public boolean contains(java.lang.Object);
Code:
   0: aload_0
   1: aload_1
   2: invokevirtual #31 // Method
indexOf:(Ljava/lang/Object;)I
   5: iflt  12
   8: iconst_1
   9: goto  13
  12: iconst_0
  13: ireturn



On Wed, Aug 27, 2014 at 7:02 AM, Fabian Lange 
wrote:

> Hi all,
> I have been involved recently in some theoretical or nonsensical
> discussions about microbenchmarking, jit compiling assemblies and so
> fort.
> One example was LinkedList vs ArrayList.
>
> What I noticed is that those two have a different implementation for
> contains():
>
> ArrayList:
>
> public boolean contains(Object o) {
> return indexOf(o) >= 0;
> }
>
> LinkedList:
>
> public boolean contains(Object o) {
> return indexOf(o) != -1;
> }
>
> Logically this is of course identical due to the contract of contains
> which returns either -1 or the >=0 index of the element.
>
> This code has been like this almost forever, and I was wondering if
> this actually makes a difference in CPU cycles.
>
> And in fact this code compiles into different assembler instructions.
> The array list does a test against 0 and conditional move, while the
> linked list does a jump equals on -1.
>
> Again that is not surprising, because the actual java source is
> different. But I wonder if both options are equally good in cold
> performance and when jitted based on parameter values.
>
> Wouldn't one implementation be better than the other? And why is not
> the "better" implementation taken in both classes (and maybe other
> Collections which use indexOf) ?
>
> Is the answer that this has always been like this and the benefit is
> not worth the risk of touching ancient code?
>
> And if not for performance, would code clarify and similarity be an
> argument?
>
> (this message was posted to jdk8-dev initially, thanks to Dalibor
> Topic for the pointer to this list)
>
> Fabian
>


Re: Review request: 8055856: checkdeps build target doesn't work for cross-compilation builds

2014-08-27 Thread Erik Joelsson


On 2014-08-27 16:39, Mandy Chung wrote:


On 8/27/2014 3:19 AM, Magnus Ihse Bursie wrote:

On 2014-08-27 10:26, Erik Joelsson wrote:

Hello Mandy,

Looking at this, I just realized that 
$(JDK_OUTPUTDIR)/modules/jdk.dev/com/sun/tools/jdeps/resources/jdeps-modules.xml 
is a generated resource for a module and that you correctly added it 
to the gendata target. Then to make it fit with the new makefile 
model, the running of TOOL_GENMODULESXML should be moved to 
jdk/make/gendata/Gendata-jdk.dev.gmk, which would make it be run 
automatically with correct dependencies. ModulesXml.gmk should also 
probably be renamed to something better describing the checkdeps 
target, which is all it would be doing then. Perhaps it would also 
fit better in the root make dir.


I can understand if fixing the cross compilation issue is urgent and 
am fine with you pushing this to fix that, but would like to see it 
further improved eventually.


I agree. The changes look good, but as Erik suggests, they can be 
taken even further, possibly in a separate fix. Perhaps the name 
"CheckModules.gmk" would be better suite than ModulesXml.gmk, when 
the gendata part has been separated out?


jdeps-modules.xml contains the membership information of all modules.  
I considered adding Gendata-jdk.dev.gmk but it would have to the last 
target to run for building exploded image (after jdk.dev classes are 
compiled).   Do you think that'd be doable?


Currently modules-xml has a dependency on "java" target.

It's still doable and would work. You would need the extra explicit 
dependency "jdk.dev-gendata: java", but I can't see any problem with that.


/Erik



Re: Impact of code difference in Collection#contains() worth improving?

2014-08-27 Thread Vitaly Davidovich
As far as I know, hotspot jit will favor cmov only when the branch appears
to be unpredictable (or conversely, not strongly predictable); otherwise,
jmp is used.  To get feedback on predictability, the code needs to run in
the interpreter, which you've not done by using -Xcomp.

Generally, if you're going to analyze generated assembly for performance,
you should not force compilation out of the gate.

Personally, it doesn't bother me that these use slightly diff condition
since semantics are same, but I take your point about consistency.

Sent from my phone
On Aug 27, 2014 10:32 AM, "Fabian Lange"  wrote:

> Hi Vitaly,
> The code comes from a single invocation of a contains(1) on an empty
> list. I forced -Xcomp to get a compilation result.
> But as stated in the initial mail, this was just curiosity.
> If the >=0 and !=-1 checks are 100% equal in performance and
> optimization, then it does not matter.
> If one of them is faster it should be used in both places.
>
> I tried also to provoke hot spot compiled code, and it looks that
> after inlining the code is much more similar.
>
> And yes the cache miss discussion has spurred my investigation, but I
> think the argument that other factors dominate the runtime should not
> be an excuse to use different (java code) implementations for the same
> thing in different places.
>
> Fabian
>
> On Wed, Aug 27, 2014 at 5:12 PM, Vitaly Davidovich 
> wrote:
> > There's no clear winner between cmov and jmp in the general case. When
> you
> > looked at the generated assembly, what code did you run to warm it up?
> Were
> > most instances found in the list or not or some mix?
> >
> > Using 0 as a test does have benefits in some places (e.g. when flags
> > register can be used from prior operations that set the zero bit), but
> this
> > one seems unlikely to be one of those.
> >
> > Also, LL traversal is likely to suffer cache misses, which would trump
> > anything else here (AL as well to a lesser degree).
> >
> > Sent from my phone
> >
> > On Aug 27, 2014 10:02 AM, "Fabian Lange"  wrote:
> >>
> >> Hi all,
> >> I have been involved recently in some theoretical or nonsensical
> >> discussions about microbenchmarking, jit compiling assemblies and so
> >> fort.
> >> One example was LinkedList vs ArrayList.
> >>
> >> What I noticed is that those two have a different implementation for
> >> contains():
> >>
> >> ArrayList:
> >>
> >> public boolean contains(Object o) {
> >> return indexOf(o) >= 0;
> >> }
> >>
> >> LinkedList:
> >>
> >> public boolean contains(Object o) {
> >> return indexOf(o) != -1;
> >> }
> >>
> >> Logically this is of course identical due to the contract of contains
> >> which returns either -1 or the >=0 index of the element.
> >>
> >> This code has been like this almost forever, and I was wondering if
> >> this actually makes a difference in CPU cycles.
> >>
> >> And in fact this code compiles into different assembler instructions.
> >> The array list does a test against 0 and conditional move, while the
> >> linked list does a jump equals on -1.
> >>
> >> Again that is not surprising, because the actual java source is
> >> different. But I wonder if both options are equally good in cold
> >> performance and when jitted based on parameter values.
> >>
> >> Wouldn't one implementation be better than the other? And why is not
> >> the "better" implementation taken in both classes (and maybe other
> >> Collections which use indexOf) ?
> >>
> >> Is the answer that this has always been like this and the benefit is
> >> not worth the risk of touching ancient code?
> >>
> >> And if not for performance, would code clarify and similarity be an
> >> argument?
> >>
> >> (this message was posted to jdk8-dev initially, thanks to Dalibor
> >> Topic for the pointer to this list)
> >>
> >> Fabian
>


Re: Review request: 8055856: checkdeps build target doesn't work for cross-compilation builds

2014-08-27 Thread Mandy Chung


On 8/27/2014 3:19 AM, Magnus Ihse Bursie wrote:

On 2014-08-27 10:26, Erik Joelsson wrote:

Hello Mandy,

Looking at this, I just realized that 
$(JDK_OUTPUTDIR)/modules/jdk.dev/com/sun/tools/jdeps/resources/jdeps-modules.xml 
is a generated resource for a module and that you correctly added it 
to the gendata target. Then to make it fit with the new makefile 
model, the running of TOOL_GENMODULESXML should be moved to 
jdk/make/gendata/Gendata-jdk.dev.gmk, which would make it be run 
automatically with correct dependencies. ModulesXml.gmk should also 
probably be renamed to something better describing the checkdeps 
target, which is all it would be doing then. Perhaps it would also 
fit better in the root make dir.


I can understand if fixing the cross compilation issue is urgent and 
am fine with you pushing this to fix that, but would like to see it 
further improved eventually.


I agree. The changes look good, but as Erik suggests, they can be 
taken even further, possibly in a separate fix. Perhaps the name 
"CheckModules.gmk" would be better suite than ModulesXml.gmk, when the 
gendata part has been separated out?


jdeps-modules.xml contains the membership information of all modules.  I 
considered adding Gendata-jdk.dev.gmk but it would have to the last 
target to run for building exploded image (after jdk.dev classes are 
compiled).   Do you think that'd be doable?


Currently modules-xml has a dependency on "java" target.

Mandy


Re: Impact of code difference in Collection#contains() worth improving?

2014-08-27 Thread Fabian Lange
Hi Vitaly,
The code comes from a single invocation of a contains(1) on an empty
list. I forced -Xcomp to get a compilation result.
But as stated in the initial mail, this was just curiosity.
If the >=0 and !=-1 checks are 100% equal in performance and
optimization, then it does not matter.
If one of them is faster it should be used in both places.

I tried also to provoke hot spot compiled code, and it looks that
after inlining the code is much more similar.

And yes the cache miss discussion has spurred my investigation, but I
think the argument that other factors dominate the runtime should not
be an excuse to use different (java code) implementations for the same
thing in different places.

Fabian

On Wed, Aug 27, 2014 at 5:12 PM, Vitaly Davidovich  wrote:
> There's no clear winner between cmov and jmp in the general case. When you
> looked at the generated assembly, what code did you run to warm it up? Were
> most instances found in the list or not or some mix?
>
> Using 0 as a test does have benefits in some places (e.g. when flags
> register can be used from prior operations that set the zero bit), but this
> one seems unlikely to be one of those.
>
> Also, LL traversal is likely to suffer cache misses, which would trump
> anything else here (AL as well to a lesser degree).
>
> Sent from my phone
>
> On Aug 27, 2014 10:02 AM, "Fabian Lange"  wrote:
>>
>> Hi all,
>> I have been involved recently in some theoretical or nonsensical
>> discussions about microbenchmarking, jit compiling assemblies and so
>> fort.
>> One example was LinkedList vs ArrayList.
>>
>> What I noticed is that those two have a different implementation for
>> contains():
>>
>> ArrayList:
>>
>> public boolean contains(Object o) {
>> return indexOf(o) >= 0;
>> }
>>
>> LinkedList:
>>
>> public boolean contains(Object o) {
>> return indexOf(o) != -1;
>> }
>>
>> Logically this is of course identical due to the contract of contains
>> which returns either -1 or the >=0 index of the element.
>>
>> This code has been like this almost forever, and I was wondering if
>> this actually makes a difference in CPU cycles.
>>
>> And in fact this code compiles into different assembler instructions.
>> The array list does a test against 0 and conditional move, while the
>> linked list does a jump equals on -1.
>>
>> Again that is not surprising, because the actual java source is
>> different. But I wonder if both options are equally good in cold
>> performance and when jitted based on parameter values.
>>
>> Wouldn't one implementation be better than the other? And why is not
>> the "better" implementation taken in both classes (and maybe other
>> Collections which use indexOf) ?
>>
>> Is the answer that this has always been like this and the benefit is
>> not worth the risk of touching ancient code?
>>
>> And if not for performance, would code clarify and similarity be an
>> argument?
>>
>> (this message was posted to jdk8-dev initially, thanks to Dalibor
>> Topic for the pointer to this list)
>>
>> Fabian


Re: Impact of code difference in Collection#contains() worth improving?

2014-08-27 Thread Vitaly Davidovich
There's no clear winner between cmov and jmp in the general case. When you
looked at the generated assembly, what code did you run to warm it up? Were
most instances found in the list or not or some mix?

Using 0 as a test does have benefits in some places (e.g. when flags
register can be used from prior operations that set the zero bit), but this
one seems unlikely to be one of those.

Also, LL traversal is likely to suffer cache misses, which would trump
anything else here (AL as well to a lesser degree).

Sent from my phone
On Aug 27, 2014 10:02 AM, "Fabian Lange"  wrote:

> Hi all,
> I have been involved recently in some theoretical or nonsensical
> discussions about microbenchmarking, jit compiling assemblies and so
> fort.
> One example was LinkedList vs ArrayList.
>
> What I noticed is that those two have a different implementation for
> contains():
>
> ArrayList:
>
> public boolean contains(Object o) {
> return indexOf(o) >= 0;
> }
>
> LinkedList:
>
> public boolean contains(Object o) {
> return indexOf(o) != -1;
> }
>
> Logically this is of course identical due to the contract of contains
> which returns either -1 or the >=0 index of the element.
>
> This code has been like this almost forever, and I was wondering if
> this actually makes a difference in CPU cycles.
>
> And in fact this code compiles into different assembler instructions.
> The array list does a test against 0 and conditional move, while the
> linked list does a jump equals on -1.
>
> Again that is not surprising, because the actual java source is
> different. But I wonder if both options are equally good in cold
> performance and when jitted based on parameter values.
>
> Wouldn't one implementation be better than the other? And why is not
> the "better" implementation taken in both classes (and maybe other
> Collections which use indexOf) ?
>
> Is the answer that this has always been like this and the benefit is
> not worth the risk of touching ancient code?
>
> And if not for performance, would code clarify and similarity be an
> argument?
>
> (this message was posted to jdk8-dev initially, thanks to Dalibor
> Topic for the pointer to this list)
>
> Fabian
>


Re: Replace concat String to append in StringBuilder parameters

2014-08-27 Thread Wang Weijun

On Aug 27, 2014, at 10:07, Wang Weijun  wrote:

> Webrev updated again, this time include more changes.
> 
>  http://cr.openjdk.java.net/~weijun/8055723/client/webrev.02/
>  http://cr.openjdk.java.net/~weijun/8055723/core/webrev.02/

The core part is updated again at

  http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/

including a fix s/(int)c/c/ in com/sun/tools/hat/internal/util/Misc.java and 
literal string merge in X509CRLEntryImpl.java, X509CRLImpl.java, and 
X509CertInfo.java.

No change for the client part.

Thanks
Max



Impact of code difference in Collection#contains() worth improving?

2014-08-27 Thread Fabian Lange
Hi all,
I have been involved recently in some theoretical or nonsensical
discussions about microbenchmarking, jit compiling assemblies and so
fort.
One example was LinkedList vs ArrayList.

What I noticed is that those two have a different implementation for contains():

ArrayList:

public boolean contains(Object o) {
return indexOf(o) >= 0;
}

LinkedList:

public boolean contains(Object o) {
return indexOf(o) != -1;
}

Logically this is of course identical due to the contract of contains
which returns either -1 or the >=0 index of the element.

This code has been like this almost forever, and I was wondering if
this actually makes a difference in CPU cycles.

And in fact this code compiles into different assembler instructions.
The array list does a test against 0 and conditional move, while the
linked list does a jump equals on -1.

Again that is not surprising, because the actual java source is
different. But I wonder if both options are equally good in cold
performance and when jitted based on parameter values.

Wouldn't one implementation be better than the other? And why is not
the "better" implementation taken in both classes (and maybe other
Collections which use indexOf) ?

Is the answer that this has always been like this and the benefit is
not worth the risk of touching ancient code?

And if not for performance, would code clarify and similarity be an argument?

(this message was posted to jdk8-dev initially, thanks to Dalibor
Topic for the pointer to this list)

Fabian


Re: RFR: 8049343: (tz) Support tzdata2014f

2014-08-27 Thread Aleksej Efimov

Masayoshi,
Thank you for a detailed review - I tried to address all your comments. 
Please, see the updated review: 
http://cr.openjdk.java.net/~aefimov/8049343/9/webrev.03
About the workarounds - I will start discussion with Sherman when the 
tzdata2014f (and I suppose the 2014g - it will be available soon 
according to tzdata mail-list [1]) integration will be complete.


-Aleksej

[1] tzdata2014g is coming: 
http://mm.icann.org/pipermail/tz/2014-August/021528.html


On 08/27/2014 12:34 PM, Masayoshi Okutsu wrote:

Here are additional comments.

src/java.base/share/classes/sun/util/calendar/ZoneInfoFile.java:

I'm concerned about the workarounds. It's not new in this update, but 
this problem would make tzupdater data void until the workaround is 
added to the next update release. Could you please work with Sherman 
to eliminate the workaround (outside of this 2014f update)?


src/java.base/share/classes/sun/util/resources/TimeZoneNames.java:

 String LORD_HOWE[] = new String[] {"Lord Howe Standard Time", 
"LHST",
-   "Lord Howe Summer Time", 
"LHST",
+   "Lord Howe Summer Time", 
"LHDT",


The S-D convention is applied to Lord Howe as well.

 567 {"Antarctica/Macquarie", new String[] {"Macquarie 
Island Time", "MIST",
 568"Macquarie 
Island Summer Time", "MIST",
 569"Macquarie 
Island Time", "MIST"}},


This one should also follow the S-D convention, although this time 
zone doesn't observe daylight saving time.



+String XJT[] = new String[] {"China Standard Time", "XJT",
+ "China Daylight Time", "XJDT",
+ "China Time", "XJT"};

Should the long names be based on Xinjiang?

Africa/Freetown is now a link to Africa/Abidjan. Those should be the 
same time zone.


-{"America/Metlakatla", new String[] {"Metlakatla Standard 
Time", "MeST",
- "Metlakatla Daylight 
Time", "MeDT",
- "Metlakatla Time", 
"MeT"}},
+{"America/Metlakatla", new String[] {"Metlakatla Standard 
Time", "PST",
+ "Metlakatla Daylight 
Time", "PDT",
+ "Metlakatla Time", 
"PT"}},


-{"Europe/Volgograd", new String[] {"Volgograd Time", "VOLT",
-   "Volgograd Summer 
Time", "VOLST",
-   "Volgograd Time", 
"VOLT"}},

+{"Europe/Volgograd", new String[] {"Volgograd Time", "MSK",
+   "Volgograd Summer 
Time", "MSK",
+   "Volgograd Time", 
"MSK"}},



The long names should be changed accordingly.

Thanks,
Masayoshi

On 8/22/2014 9:17 PM, Aleksej Efimov wrote:

Masayoshi,
You're right that the "root names" should be changed as part of this 
update. The names were changed according to Australian official 
document here: http://australia.gov.au/about-australia/our-country/time
The fixed version of the webrev can be found here: 
http://cr.openjdk.java.net/~aefimov/8049343/9/webrev.02


Thanks,
Aleksej


On 08/21/2014 03:51 PM, Masayoshi Okutsu wrote:
We used to make name changes in the root (base) bundle as part of 
time zone data maintenance and deter only translations. But if you 
want to handle name changes later, that would be fine. It's your call.


Thanks,
Masayoshi

On 8/21/2014 5:05 PM, Aleksej Efimov wrote:

Masayoshi,
I agree that we should change the long names to match the new short 
names introduced by tzdata.
But I suggest to log a different CR for such activity to handle 
long name changes and their translations (this activity is slightly 
out of tzdata update scope). Does it make sense?


-Aleksej

On 08/21/2014 06:32 AM, Masayoshi Okutsu wrote:
I think the long names of the Australia time zones should be 
revisited to be consistent with the abbreviation changes. The new 
abbreviations follow the S[tandard] and D[aylight saving] 
convention rather than the S[tandard] and S[ummer time] one. The 
long names, such as "Eastern Summer Time (Queensland)", no longer 
make sense.


On the other hand, you will need to access impact of the name 
changes, including abbreviations. Also, if you change the long 
names, their translations will need to be changed as well.


Thanks,
Masayoshi

On 8/20/2014 11:59 PM, Aleksej Efimov wrote:

Hi,

Please, review the tzdata2014f integration (with tzdata2014e 
related changes included too) [1] fix to JDK9: 
http://cr.openjdk.java.net/~aefimov/8049343/9/webrev.01/


The tzdata2014f changes are extensive and relates mostly to 
timezone short names changes + "Asia/Srednekolymsk" time zone 
were added.
Almost complete list o

Re: [8u-dev] RFR: 8036981: JAXB not preserving formatting for xsd:any Mixed content

2014-08-27 Thread Aleksej Efimov

Hi,
I'm adding a core-libs-dev mail list - I might be lucky to find a JDK8 
reviewer for my fix there.


Thank you,
Aleksej

On 08/26/2014 06:12 PM, Aleksej Efimov wrote:

Hi Dalibor,

It's a partial backport (logged with a different bugID) of JDK9 fix, I 
got an approval from Miran (Thank you Miran!) and right now I'm 
waiting for a JDK8 reviewer approval.


-Aleksej

On 08/26/2014 04:58 PM, dalibor topic wrote:

Hi Aleksej - are you looking for approval to push into jdk8u-dev?

cheers,
dalibor topic

On 22.08.2014 22:01, Miroslav Kos wrote:

Hi Aleksej,
looks good to me.

Thanks
Miran


On 18/08/14 17:29, Aleksej Efimov wrote:

Hi,

Can I ask for a review of 8036981 bug [1] fix:
JAXWS: 
http://cr.openjdk.java.net/~aefimov/8036981/8/webrev.00/jaxws/

JDK: http://cr.openjdk.java.net/~aefimov/8036981/8/webrev.00/jdk/

The proposed fix is a partial backport of "Update JAX-WS RI
integration to latest version" bug [2] to JDK8.
The proposed patch fixes the broken formatting of the input xml during
JAXB marshalling/unmarshalling. Regression test is included.
Testing:
- regtests: xml related tests from 'core' testset - no failures
- JCK: api/xinclude api/xsl api/javax_xml api/org_xml xml_schema
api/xinclude - no failures

Thank you,
Aleksej

[1] https://bugs.openjdk.java.net/browse/JDK-8036981
[2] https://bugs.openjdk.java.net/browse/JDK-8044656
[3] 8044656 JDK9 review thread:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-June/027458.html 














Re: RFR: [8054714] Use StringJoiner where it makes the code cleaner

2014-08-27 Thread Paul Sandoz
On Aug 9, 2014, at 8:32 AM, Ivan Gerasimov  wrote:
> Yes, this will surely work faster. I've incorporated your suggestion:
> http://cr.openjdk.java.net/~igerasim/8054714/1/webrev/
> 

Looks ok.

V. minor point, rogue space:

sun/net/www/protocol/http/HttpURLConnection.java2014-08-09 
10:34:38.656983033 +0400

+if (! cookie.isHttpOnly())

Paul.




Re: Replace concat String to append in StringBuilder parameters

2014-08-27 Thread Pavel Rappo
Could you please explain what you mean by "javac optimization fails" here?

-Pavel

On 27 Aug 2014, at 10:41, Ulf Zibis  wrote:

> 4.) Now we see, that javac optimization fails again if StringBuilder, 
> concatenation, toString(), append(String), append(Collection) etc. and 
> StringJoiner use is mixed.



Re: RFR: 8055949: ByteArrayOutputStream capacity should be maximal array size permitted by VM

2014-08-27 Thread Ulf Zibis

Am 25.08.2014 um 19:37 schrieb Martin Buchholz:

https://bugs.openjdk.java.net/browse/JDK-8055949
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/ByteArrayOutputStream-MAX_ARRAY_SIZE/

The 2x capacity gap was noticed by real users!


Hi Martin,

the MAX_ARRAY_SIZE code now is copied to many places in JDK. Couldn't you better centralize the code 
to one place, e.g. j.u.Arrays or some hidden class of sun.java...?

Isn't there a property to retrieve MAX_ARRAY_SIZE from the running VM?

Imagine, some VM throws OOME above Integer.MAX_VALUE-4 and minCapacity is 
Integer.MAX_VALUE-4.
With this code a OOME will happen:
 124 return (minCapacity > MAX_ARRAY_SIZE) ?
 125 Integer.MAX_VALUE :
 126 MAX_ARRAY_SIZE;
With this code we would avoid the OOME:
 124 return (minCapacity > MAX_ARRAY_SIZE) ?
 125 minCapacity :
 126 MAX_ARRAY_SIZE;

-Ulf



Re: Review request: 8055856: checkdeps build target doesn't work for cross-compilation builds

2014-08-27 Thread Magnus Ihse Bursie

On 2014-08-27 10:26, Erik Joelsson wrote:

Hello Mandy,

Looking at this, I just realized that 
$(JDK_OUTPUTDIR)/modules/jdk.dev/com/sun/tools/jdeps/resources/jdeps-modules.xml 
is a generated resource for a module and that you correctly added it 
to the gendata target. Then to make it fit with the new makefile 
model, the running of TOOL_GENMODULESXML should be moved to 
jdk/make/gendata/Gendata-jdk.dev.gmk, which would make it be run 
automatically with correct dependencies. ModulesXml.gmk should also 
probably be renamed to something better describing the checkdeps 
target, which is all it would be doing then. Perhaps it would also fit 
better in the root make dir.


I can understand if fixing the cross compilation issue is urgent and 
am fine with you pushing this to fix that, but would like to see it 
further improved eventually.


I agree. The changes look good, but as Erik suggests, they can be taken 
even further, possibly in a separate fix. Perhaps the name 
"CheckModules.gmk" would be better suite than ModulesXml.gmk, when the 
gendata part has been separated out?


/Magnus


Re: The future of Serialization

2014-08-27 Thread Paul Sandoz

On Aug 9, 2014, at 7:20 PM, Brian Goetz  wrote:

>> I've noticed there's not much interest in improving Serialization on
>> these lists.  This makes me wonder if java Serialization has lost
>> relevance in recent years with the rise of protocol buffers apache
>> thrift and other means of data transfer over byte streams.
> 
> I sense your frustration, but I think you may be reaching the wrong 
> conclusion.  The lack of response is probably not evidence that there's no 
> interest in fixing serialization; its that fixing serialization, with all the 
> constraints that "fix" entails, is just really really hard, and its much 
> easier to complain about it (and even say "let's just get rid of it") than to 
> fix it.
> 
>> Should Serializable eventually be deprecated? Should Serialization be
>> disabled by default? Should a new mechanism be developed? If a new
>> mechanism is developed, what about circular object relationships?
> 
> As I delved into my own explorations of serialization, I started to realize 
> why such a horrible approach was the one that was ultimately chosen; while 
> serialization is horrible and awful and leaky and insecure and complex and 
> brittle, it does address problems like cyclic data structures and independent 
> evolution of subclass and superclass better than the "clean" models.
> 
> My conclusion is, at best, a new mechanism would have to live side-by-side 
> with the old one, since it could only handle 95% of the cases.  It might 
> handle those 95% much better -- more cleanly, securely, and allowing easier 
> schema evolution -- but the hard cases are still there.  Still, reducing the 
> use of the horrible old mechanism may still be a worthy goal, even if it 
> can't be killed outright.
> 


Also many serialization-based libraries use sun.misc.Unsafe or 
sun.reflect.ReflectionFactory for various reasons (with backup plans if such 
classes are not available or accessible).

As part to the future of serialization i think we need to evaluate libraries 
such as XStream and Objenesis  to see what unsafe/internal mechanisms can be 
replaced by functionally equivalent safe public mechanisms.

I have more questions than answers at the moment with regards to that :-(

Paul.


Re: Replace concat String to append in StringBuilder parameters

2014-08-27 Thread Ulf Zibis

Hi all,

a critical question!

1.) Why we have String concatenation in Java? ... I would answer: for 
_readability_ purpose.

2.) In the early javac times, we were asked to refactor to StringBuffer for 
performance critical code.

3.) Since 1.6 ? javac is capable to replace multiple concatenation with one Stringbuilder, so we are 
again asked to refactor the code for better readability.


4.) Now we see, that javac optimization fails again if StringBuilder, concatenation, toString(), 
append(String), append(Collection) etc. and StringJoiner use is mixed.


*
5.) Couldn't we force the better optimization of javac instead again refactoring the code and 
decrease readability again?

*

+ manual .append("x") should be translated to .append('x')
+ Javac could calculate a reasonable buffer size init value.


-Ulf


Am 11.08.2014 um 16:01 schrieb Ulf Zibis:


Am 11.08.2014 um 15:12 schrieb Andrej Golovnin:

In the most classes I mentioned in my previous mail only the
#toString()-methods would be affected by the proposal. And in the most
cases, maybe in all cases, the #toString()-methods in this classes exists
only to provide nice output.


So why not "nice input" from the java sources ...i.e.: use concatenation only if possible. The 
performance problem occurs, if both strategies are mixed.



Btw. I see here a nice opportunity to create an RFE
for the Javac team. Following code:

Object o1 = ...;
Object o2 = ...;
String s = "abc" + o1 + "c" + o2 + "\n";

should be translated to:

String s = new
StringBuilder().append("abc").append(o1).append('c').append(o2).append('\n').toString();

instead of:

String s = new
StringBuilder().append("abc").append(o1).append("c").append(o2).append("\n").toString();


+ manual .append("x") should be translated to .append('x')
+ Javac could avoid to instantiate multiple SBs from mixed concatenation/SB 
source code.
+ Javac could calculate a reasonable buffer size init value.

-Ulf






Re: RFR: 8049343: (tz) Support tzdata2014f

2014-08-27 Thread Masayoshi Okutsu

Here are additional comments.

src/java.base/share/classes/sun/util/calendar/ZoneInfoFile.java:

I'm concerned about the workarounds. It's not new in this update, but 
this problem would make tzupdater data void until the workaround is 
added to the next update release. Could you please work with Sherman to 
eliminate the workaround (outside of this 2014f update)?


src/java.base/share/classes/sun/util/resources/TimeZoneNames.java:

 String LORD_HOWE[] = new String[] {"Lord Howe Standard Time", "LHST",
-   "Lord Howe Summer Time", "LHST",
+   "Lord Howe Summer Time", "LHDT",

The S-D convention is applied to Lord Howe as well.

 567 {"Antarctica/Macquarie", new String[] {"Macquarie Island Time", 
"MIST",
 568"Macquarie Island Summer Time", 
"MIST",
 569"Macquarie Island Time", 
"MIST"}},

This one should also follow the S-D convention, although this time zone 
doesn't observe daylight saving time.



+String XJT[] = new String[] {"China Standard Time", "XJT",
+ "China Daylight Time", "XJDT",
+ "China Time", "XJT"};

Should the long names be based on Xinjiang?

Africa/Freetown is now a link to Africa/Abidjan. Those should be the 
same time zone.


-{"America/Metlakatla", new String[] {"Metlakatla Standard Time", 
"MeST",
- "Metlakatla Daylight Time", 
"MeDT",
- "Metlakatla Time", "MeT"}},
+{"America/Metlakatla", new String[] {"Metlakatla Standard Time", 
"PST",
+ "Metlakatla Daylight Time", 
"PDT",
+ "Metlakatla Time", "PT"}},

-{"Europe/Volgograd", new String[] {"Volgograd Time", "VOLT",
-   "Volgograd Summer Time", 
"VOLST",
-   "Volgograd Time", "VOLT"}},
+{"Europe/Volgograd", new String[] {"Volgograd Time", "MSK",
+   "Volgograd Summer Time", "MSK",
+   "Volgograd Time", "MSK"}},
 


The long names should be changed accordingly.

Thanks,
Masayoshi

On 8/22/2014 9:17 PM, Aleksej Efimov wrote:

Masayoshi,
You're right that the "root names" should be changed as part of this 
update. The names were changed according to Australian official 
document here: http://australia.gov.au/about-australia/our-country/time
The fixed version of the webrev can be found here: 
http://cr.openjdk.java.net/~aefimov/8049343/9/webrev.02


Thanks,
Aleksej


On 08/21/2014 03:51 PM, Masayoshi Okutsu wrote:
We used to make name changes in the root (base) bundle as part of 
time zone data maintenance and deter only translations. But if you 
want to handle name changes later, that would be fine. It's your call.


Thanks,
Masayoshi

On 8/21/2014 5:05 PM, Aleksej Efimov wrote:

Masayoshi,
I agree that we should change the long names to match the new short 
names introduced by tzdata.
But I suggest to log a different CR for such activity to handle long 
name changes and their translations (this activity is slightly out 
of tzdata update scope). Does it make sense?


-Aleksej

On 08/21/2014 06:32 AM, Masayoshi Okutsu wrote:
I think the long names of the Australia time zones should be 
revisited to be consistent with the abbreviation changes. The new 
abbreviations follow the S[tandard] and D[aylight saving] 
convention rather than the S[tandard] and S[ummer time] one. The 
long names, such as "Eastern Summer Time (Queensland)", no longer 
make sense.


On the other hand, you will need to access impact of the name 
changes, including abbreviations. Also, if you change the long 
names, their translations will need to be changed as well.


Thanks,
Masayoshi

On 8/20/2014 11:59 PM, Aleksej Efimov wrote:

Hi,

Please, review the tzdata2014f integration (with tzdata2014e 
related changes included too) [1] fix to JDK9: 
http://cr.openjdk.java.net/~aefimov/8049343/9/webrev.01/


The tzdata2014f changes are extensive and relates mostly to 
timezone short names changes + "Asia/Srednekolymsk" time zone were 
added.
Almost complete list of changes can be found in the JBS bug 
description [1], plus some changes wasn't documented in tzdata 
release notes - for such cases raw tzdata diff was used for the 
names modifications.


Two issues with JSR310 implementation were discovered during 
integration process:
First issue is related to the internal representation of the 
'24:00' value. The JSR310 implementation treats this value as a 
next day 00:00 time. The workaround already exists in JSR310 code 
for similar entries and this failure is resolved in similar way 
[

Re: Review request: 8055856: checkdeps build target doesn't work for cross-compilation builds

2014-08-27 Thread Erik Joelsson

Hello Mandy,

Looking at this, I just realized that 
$(JDK_OUTPUTDIR)/modules/jdk.dev/com/sun/tools/jdeps/resources/jdeps-modules.xml 
is a generated resource for a module and that you correctly added it to 
the gendata target. Then to make it fit with the new makefile model, the 
running of TOOL_GENMODULESXML should be moved to 
jdk/make/gendata/Gendata-jdk.dev.gmk, which would make it be run 
automatically with correct dependencies. ModulesXml.gmk should also 
probably be renamed to something better describing the checkdeps target, 
which is all it would be doing then. Perhaps it would also fit better in 
the root make dir.


I can understand if fixing the cross compilation issue is urgent and am 
fine with you pushing this to fix that, but would like to see it further 
improved eventually.


/Erik

On 2014-08-27 00:40, Mandy Chung wrote:
JDK-8055856: checkdeps build target doesn't work for cross-compilation 
builds

JDK-8056113: [build] tools.jar missing modules.xml

Webrev at:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055856/

This patch fixes a few things about modules.xml

1. jdeps is invoked at build time to verify the module boundaries.
   For cross-compilation, it has to run on the host system.
   jdeps is added in the langtools interim build so that it
   can run with the BOOT_JDK.  jdeps is also modified to
   support a system property to specify the path to modules.xml
   generated later.

2. The generated modules.xml is solely for jdeps to use until
   the module system is in place.  The build tool takes the
   modules.xml in the top repo and generates a new file
   to include the module membership for jdeps to use.  It
   is better to name it differently to jdeps-modules.xml
   to avoid confusion as Magnus suggests.

3. main-jars is missing the dependency of modules-xml target
   and thus jdeps in the JDK images fails.  To help catch
   this issue, jdeps now throws an exception if jdeps-modules.xml
   is missing.

thanks
Mandy