Re: RFR 8056249 Improve CompletableFuture resource usage

2014-09-01 Thread Paul Sandoz
On Aug 29, 2014, at 5:56 PM, Martin Buchholz marti...@google.com wrote:
 Looks fine.

Thanks Martin  Chris.


 Instead of using Contributed-by: for Doug's work, you should make him the hg 
 user, as is done in previous changesets.  E.g. hg import has a --user flag.
 

Ok, i will do that.

Paul.


Re: [9] RFR (S): 8056926: Improve caching of GuardWithTest combinator

2014-09-01 Thread Paul Sandoz

On Aug 29, 2014, at 7:20 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 http://cr.openjdk.java.net/~vlivanov/8056926/webrev.00/
 https://bugs.openjdk.java.net/browse/JDK-8056926
 
 Cache GuardWithTest per erased to basic types signature.
 
 GWT shape is made friendly to sharing:
  * GWT MH is implemented as BMH which stores 3 method handles
  * LF loads them from the associated MethodHandle
 
 Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ -ea 
 -esa and COMPILE_THRESHOLD={0,30}.
 
 Reviewed-by: vlivanov, ?
 Contributed-by: john.r.r...@oracle.com
 

+1

To be on the safe side should a @ForceInline be stuffed on the 
selectAlternative method?

Paul.


Re: [9] RFR (S): 8056926: Improve caching of GuardWithTest combinator

2014-09-01 Thread Vladimir Ivanov

Thanks, Paul.

There's no need to add @ForceInline on selectAlternative.
It is used only during LF interpretation. There's an intrinsic for GWT 
combinator, which encodes it as a branch (see 
InvokerBytecodeGenerator.emitSelectAlternative).


Best regards,
Vladimir Ivanov

On 9/1/14, 1:48 PM, Paul Sandoz wrote:


On Aug 29, 2014, at 7:20 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:


http://cr.openjdk.java.net/~vlivanov/8056926/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8056926

Cache GuardWithTest per erased to basic types signature.

GWT shape is made friendly to sharing:
  * GWT MH is implemented as BMH which stores 3 method handles
  * LF loads them from the associated MethodHandle

Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ -ea 
-esa and COMPILE_THRESHOLD={0,30}.

Reviewed-by: vlivanov, ?
Contributed-by: john.r.r...@oracle.com



+1

To be on the safe side should a @ForceInline be stuffed on the 
selectAlternative method?

Paul.



___
mlvm-dev mailing list
mlvm-...@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev



Re: [9] RFR (S): 8056926: Improve caching of GuardWithTest combinator

2014-09-01 Thread Paul Sandoz

On Sep 1, 2014, at 12:29 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:

 Thanks, Paul.
 
 There's no need to add @ForceInline on selectAlternative.
 It is used only during LF interpretation. There's an intrinsic for GWT 
 combinator, which encodes it as a branch (see 
 InvokerBytecodeGenerator.emitSelectAlternative).
 

Ah yes, i forgot about those recently added intrinsics,
Paul.


Re: Process API Updates (JEP 102)

2014-09-01 Thread Ron Pressler
If the second process is indeed a JVM, is that any different from spawning a 
Java process with a shell-script exec (something that’s quite commonly done, I 
assume)?


On Thursday, August 28, 2014 at 7:58 PM, roger riggs wrote:

 Hi Ron,
  
 I have not looked at that idea closely but I would be concerned
 about the robustness of the 2nd, execve'd Java runtime. Since it would  
 not be
 a brand new process, there might be leftover state from the previous
 execution that would break the usual assumptions of a newly started Java  
 Runtime.
 Anything from signal handler state to open file descriptors to the specific
 memory mapping and there may be others.
  
 Roger
  
  
 On 8/26/2014 7:25 AM, Ron Pressler wrote:
  I might be a little late to this party, but recently I've had a (rather
  frustrating) need for the ability to execve a process rather than fork-exec
  it. I understand that the ability to exec (replace the current process's
  image) is also available on Windows. This operation (on ProcessBuilder?),
  which never returns, would have the same semantics as
  System.exit(pb.start().waitFor()), only it would replace the current JVM
  process (i.e. maintain the same pid/handle) with the command.
   
  This is required when a JVM process is used to set up and launch another,
  JVM or other, process, but we'd want the user running the program to be
  oblivious to the setup process (because, say, they want to monitor the
  running program with some OS tool).
   
  Ron  



Re: [9] RFR (S): 8056926: Improve caching of GuardWithTest combinator

2014-09-01 Thread Vladimir Ivanov

FTR, selectAlternative intrinsic is there from the very beginning.
Recent changes improved how intrinsics are represented on LF level + 
added a bunch of new intrinsics.


Best regards,
Vladimir Ivanov

On 9/1/14, 2:49 PM, Paul Sandoz wrote:


On Sep 1, 2014, at 12:29 PM, Vladimir Ivanov vladimir.x.iva...@oracle.com 
wrote:


Thanks, Paul.

There's no need to add @ForceInline on selectAlternative.
It is used only during LF interpretation. There's an intrinsic for GWT 
combinator, which encodes it as a branch (see 
InvokerBytecodeGenerator.emitSelectAlternative).



Ah yes, i forgot about those recently added intrinsics,
Paul.



___
mlvm-dev mailing list
mlvm-...@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev



RFR: 8049343: (tz) Support tzdata2014g

2014-09-01 Thread Aleksej Efimov

Masayoshi,

I have addressed all your comments with proposed resolution. Thank you 
for such thorough analysis of this changes.


Also the new tzdata is out (2014g) - I have updated the JBS bug by 
adding 2014g related change notes and renamed this bug appropriately + 
I'm renaming this thread.

The new webrev contains new changes related to 2014g:
-{America/Grand_Turk, EST},
+{America/Grand_Turk, AST},


The 2014e/f related changes, discussed previously, are still in place.

New webrev can be found here: 
http://cr.openjdk.java.net/~aefimov/8049343/9/webrev.04


The bug for the incomplete localization of new/updated time zone names 
was filed: https://bugs.openjdk.java.net/browse/JDK-8057004


With Best Regards,
Aleksej

On 08/28/2014 07:43 AM, Masayoshi Okutsu wrote:

 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: 

Re: RFR: JDK-8056934: ZipInputStream does not correctly handle local header data descriptors with the optional signature missing

2014-09-01 Thread Alan Bateman

On 29/08/2014 20:12, Martin Buchholz wrote:

Hi Xueming and Alan,

I'd like you to do a code review.

https://bugs.openjdk.java.net/browse/JDK-8056934
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/zip-DataDescriptorSignatureMissing/ 
http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk9/zip-DataDescriptorSignatureMissing/


This seems like an atypical off-by-one, so I'm not sure how it 
happened, and I have the nagging feeling I'm missing something.


Greg, this java code review contains a python program!
I looked at 4.3.9.3. For ZIP64 then we'll read 24 bytes which is 4 bytes 
too much if the signature is not present. The offset of those 4 bytes is 
position 20 (ZIP64_EXTHDR - ZIP64_EXTHDR with the current constants). 
With ZIP then we'll need 16 bytes and need to unread 4 bytes from 
position 12 (EXTHDR - EXTCRC with the current constants).


So I think you're right that the -1, I can only assume that we haven't 
encountered zip files that have a data descriptor without the signature.


For the comment in ZipInputStream then it might be better to move that 
to readEnd. My preference would be to not include the part starting As 
of 2014-08, phyton ... as that might not interesting in years to come.


-Alan.






Re: RFR: JDK-8056934: ZipInputStream does not correctly handle local header data descriptors with the optional signature missing

2014-09-01 Thread Xueming Shen

On 8/29/14 12:12 PM, Martin Buchholz wrote:

Hi Xueming and Alan,

I'd like you to do a code review.

https://bugs.openjdk.java.net/browse/JDK-8056934
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/zip-DataDescriptorSignatureMissing/ 
http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk9/zip-DataDescriptorSignatureMissing/


This seems like an atypical off-by-one, so I'm not sure how it 
happened, and I have the nagging feeling I'm missing something.


Greg, this java code review contains a python program!


Agreed, it appears to be off-by-1. I don't know how it happened as well. 
The code was put in
back to 1.4.2 (4635869) to fix exactly the same issue (a local 
descriptors without signature bits).

Obviously we did not put into the regression test for it :-(

The fix looks fine. Though the comment part is a little long:-) It might 
be desirable to simply keep

the first part and move it into readEND?

-Sherman


Re: RFR: JDK-8056934: ZipInputStream does not correctly handle local header data descriptors with the optional signature missing

2014-09-01 Thread Xueming Shen

On 9/1/14 9:05 AM, Alan Bateman wrote:

On 29/08/2014 20:12, Martin Buchholz wrote:

Hi Xueming and Alan,

I'd like you to do a code review.

https://bugs.openjdk.java.net/browse/JDK-8056934
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/zip-DataDescriptorSignatureMissing/ 
http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk9/zip-DataDescriptorSignatureMissing/ 



This seems like an atypical off-by-one, so I'm not sure how it 
happened, and I have the nagging feeling I'm missing something.


Greg, this java code review contains a python program!
I looked at 4.3.9.3. For ZIP64 then we'll read 24 bytes which is 4 
bytes too much if the signature is not present. The offset of those 4 
bytes is position 20 (ZIP64_EXTHDR - ZIP64_EXTHDR with the current 
constants). With ZIP then we'll need 16 bytes and need to unread 4 
bytes from position 12 (EXTHDR - EXTCRC with the current constants).


So I think you're right that the -1, I can only assume that we haven't 
encountered zip files that have a data descriptor without the signature.


The zip64 part was the copy/paste of the existing code, and we did not 
add any test for this corner case we
implemented zip64. Before the fix of 4635869, the implementation simply 
throws an exception. With the fix
it appears it can work well with the first entry of such a zip file, but 
skip the rest...my guess is that the test

case zip file in original bug report happens to be a single entry zip file.

-Sherman



For the comment in ZipInputStream then it might be better to move that 
to readEnd. My preference would be to not include the part starting 
As of 2014-08, phyton ... as that might not interesting in years to 
come.


-Alan.