Re: RFR 8056249 Improve CompletableFuture resource usage
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
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
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
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)
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
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
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
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
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
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.