RE: RFR: 8151876: (tz) Support tzdata2016b
Hi Masayoshi, Thank you for clarification. Since, the tzdata2016c is already released, I will update the this bug and send a new review request to include both tzdata2016b and tzdata2016c. Regards, Ramanand. -Original Message- From: Masayoshi Okutsu Sent: Monday, March 28, 2016 6:50 PM To: Ramanand Patil; i18n-...@openjdk.java.net Cc: core-libs-dev@openjdk.java.net Subject: Re: RFR: 8151876: (tz) Support tzdata2016b Hi Ramanand, What I meant is to remove those new resources so that "GMT+hh:mm" is used for formatting. There may be some tests that require changes. Thanks, Masayoshi On 3/28/2016 7:31 PM, Ramanand Patil wrote: > Hi Masayoshi, > > Currently I have not defined the new TimeZoneNames with +hh format, instead I > have defined them as per the earlier convention like: > > +{"Asia/Barnaul", new String[] {"Barnaul Time", "BAT", > + "Barnaul Summer Time", "BAST", > + "Barnaul Time", "BAT"}}, > > +{"Europe/Astrakhan", new String[] {"Astrakhan Time", "ASTT", > + "Astrakhan Summer Time", > "ASTST", > + "Astrakhan Time", > + "ASTT"}}, > > +{"Europe/Ulyanovsk", new String[] {"Ulyanovsk Time", "ULT", > + "Ulyanovsk Summer Time", > "ULST", > + "Ulyanovsk Time", > + "ULT"}}, > > > > Please let me know if this is fine. > > > Regards, > Ramanand. > > -Original Message- > From: Masayoshi Okutsu > Sent: Wednesday, March 23, 2016 7:22 PM > To: Ramanand Patil; i18n-...@openjdk.java.net > Cc: core-libs-dev@openjdk.java.net > Subject: Re: RFR: 8151876: (tz) Support tzdata2016b > > Sorry for this belated response. > > I prefer to follow the tzdata abbreviations, like "+04". But that would > require some major changes. An option would be not to define time zone names > for the new zones with the +hh format. > > Thanks, > Masayoshi > > On 3/17/2016 4:38 AM, Ramanand Patil wrote: >> Hi all, >> >> Please review the latest TZDATA (tzdata2016b) integration to JDK9. >> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8151876 >> >> Webrev: http://cr.openjdk.java.net/~rpatil/8151876/webrev.00/ >> >> All the TimeZone related tests are passed after integration. >> >> >> >> Please note that, as per the release notes: As a trial of a new system that >> needs less information to be made up, the new zones use numeric time zone >> abbreviations like "+04" instead of invented abbreviations like "ASTT". >> >> Since this is a trial system, I have ignored this in the current patch which >> adds 3 new timezones. But I think it's worth discussing this new system >> along with this bug and get the experts opinion on this. >> >> Do you think that JDK should also follow these IANA timezone naming system >> for zone abbreviations in future ? Will it be convenient to use such >> numeric time zone abbreviations ? (Also, it looks like the abbreviations >> similar to "+03/+04" will be used for the zones linked to the rule set with >> changing letters like: EST/EDT for US, MSK/MSD for RU, etc..). >> >> >> >> Regards, >> >> Ramanand. >> >>
Re: JAXP default implementation and JDK-8152063
Awesome. I'll look into getting a spec change approved. Best, Joe On 3/28/2016 6:24 PM, David M. Lloyd wrote: I think that either of these options would work great for us. On 03/28/2016 05:46 PM, huizhe wang wrote: Thanks David. So I understand the dynamic nature of the server configuration. There maybe two options to solve it: 1) Add a system property to point to a Layer in order to find an alternative-system-default. This will add a new step to the JAXP process after the current ServiceLoader process. I saw that you had concern over the performance of searching a provider among all modules in a Layer. or 2) Add a new type FinderDelegate for processes such as the "proxy" in your case to implement. If the FinderDelegate process fails to locate a provider, it would signal the jaxp process (by returning null) to fall back to the JDK-default implementation. In other words, when the system property points to a FinderDelegate, the 4-step JAXP process is reduced to two: delegate the process to the FinderDelegate, and fall back to the system default implementation. The benefit of this approach is that container developers are allowed complete flexibility to define the process within their environment, thus probably more adaptable to any changes. -Joe On 3/28/2016 4:42 AM, David M. Lloyd wrote: The reason is because we use a single boot path but we don't know at the time of boot whether or not we will have a JAXP provider, nor where it will come from; that is up to the server configuration that we end up running. With the system properties approach we can just clear all the properties, but if we use ServiceLoader then we can't switch it on and off on demand. On 03/26/2016 04:57 PM, huizhe wang wrote: Going back to the original issue a bit, if you don't mind. The issue was that JBoss wished to configure its own default providers for certain JAXP-defined services. What you've been doing until now was to point the system property to a proxy, which, in essence, took over the JAXP provider-lookup process, and served as a factory finder outside of JAXP. "however if we do this but there is/are no overriding implementation(s) for one or more of these APIs, then AFAICT it will fail with Jigsaw because we can't access the JDK's default implementations". So the question is, why did you have to override the JAXP process for the services that you don't have an AppServer-level provider ( "overriding implementation(s)" )? Thanks, Joe On 3/25/2016 5:53 AM, David M. Lloyd wrote: OK, so disappointment again. :) Thanks for responding; the search continues. On 03/24/2016 06:35 PM, huizhe wang wrote: Right, that sounds like what I thought you would want: an additional step in the factory-lookup process to try locating a provider through the Layer of the caller if TCCL fails. I think the assumption in the previous discussion was that a new method would be introduced to take a Layer as an argument. -Joe On 3/24/2016 3:36 PM, David M. Lloyd wrote: This is all for the case where the user is calling e.g. javax.xml.stream.XMLInputFactory#newFactory() with no arguments. We need some way to choose a specific non-JDK default implementation when there is no service loader info on the TCCL. Using the Layer of the caller of the newFactory() method would be an ideal solution from our perspective. On 03/24/2016 05:18 PM, huizhe wang wrote: So specifying Layer is preferred solution. If a new method is needed however, (similar situation to using the method with ClassLoader), that would mean your users' applications are required to adapt to the new API. Would you expect that would happen or would you still have existing applications that can not be updated? -Joe On 3/24/2016 2:02 PM, David M. Lloyd wrote: On 03/24/2016 03:54 PM, huizhe wang wrote: In this discussion so far, a) I see that you seemed to have successfully used the method with a class loader as Daniel suggested. I assume that solved the issue reported in JDK-8152063. Am I right, that you no longer have issue with using a proxy? Or No, not solved yet, just in the process of prototyping but ran into a road block with XMLReaderFactory. b) you do need JAXP's supporting using a Layer as the context in order to solve your issue completely? I think this should be considered the best solution to the problem. c) On org.xml.sax.helpers.XMLReaderFactory, as Alan and Daniel said, yes I'm working with the SAX project to hopefully get a new release out that would conform to the ServiceLoader spec. I'm also suggesting a new method that takes a ClassLoader that would be useful if (a) worked for you. We have some more testing to do before I can say whether this works or does not work. But either way it is a rather non-optimal solution, and if I can avoid using a proxy layer, I would much prefer to do so. Thanks for your response.
Re: JDK 9 RFR of JDK-8151763; Use more informative format for problem list
> On Mar 28, 2016, at 5:03 PM, Joseph D. Darcy wrote: > > Hello, > > New iteration of the webrev updated after the Jigsaw integration and > incorporating the earlier feedback. > >http://cr.openjdk.java.net/~darcy/8151763.1 > # tools/jimage/JImageTest.java linux-i586,windows-i586 Is this test accidentally removed? Other than this, looks okay. Nit: it’d be good to have most of bug ids aligned in the same column start. Here are a few ones: 210 sun/security/krb5/auto/Unreachable.java 7164518 macosx-all no PortUnreachableException on Mac 212 java/security/KeyPairGenerator/SolarisShortDSA.java 7041639 solaris-all Solaris DSA keypair generation bug 213 sun/security/tools/keytool/standard.sh 7041639 solaris-all Solaris DSA keypair generation bug 346 java/util/Arrays/ParallelPrefix.java 8080165,8085982 generic-all 348 java/util/BitSet/BitSetStreamTest.java 8079538 generic-all 360 sun/tools/jmap/heapconfig/JMapHeapConfigTest.java 8072131,8132452 generic-all 370 sun/tools/jinfo/JInfoSanityTest.java 8059035 generic-all Mandy
Re: JDK 9 RFR of JDK-8152873: java/util/Locale/LocaleProviders.sh fails on several platforms
Looks fine; thanks Amy, -Joe On 3/28/2016 7:33 PM, Amy Lu wrote: java/util/Locale/LocaleProviders.sh starts failing after JDK-8150432, there's simple issue in JDK-8150432. Please review this quick fix. bug: https://bugs.openjdk.java.net/browse/JDK-8152873 webrev: http://cr.openjdk.java.net/~amlu/8152873/webrev.00/ Thanks, Amy --- old/test/java/util/Locale/LocaleProviders.sh2016-03-29 10:15:10.0 +0800 +++ new/test/java/util/Locale/LocaleProviders.sh2016-03-29 10:15:10.0 +0800 @@ -25,7 +25,7 @@ # @test # @bug 6336885 7196799 7197573 7198834 8000245 8000615 8001440 8008577 # 8010666 8013086 8013233 8013903 8015960 8028771 8054482 8062006 - 8150432 +# 8150432 # @summary tests for "java.locale.providers" system property # @modules java.base/sun.util.locale # java.base/sun.util.locale.provider
JDK 9 RFR of JDK-8152873: java/util/Locale/LocaleProviders.sh fails on several platforms
java/util/Locale/LocaleProviders.sh starts failing after JDK-8150432, there's simple issue in JDK-8150432. Please review this quick fix. bug: https://bugs.openjdk.java.net/browse/JDK-8152873 webrev: http://cr.openjdk.java.net/~amlu/8152873/webrev.00/ Thanks, Amy --- old/test/java/util/Locale/LocaleProviders.sh2016-03-29 10:15:10.0 +0800 +++ new/test/java/util/Locale/LocaleProviders.sh2016-03-29 10:15:10.0 +0800 @@ -25,7 +25,7 @@ # @test # @bug 6336885 7196799 7197573 7198834 8000245 8000615 8001440 8008577 # 8010666 8013086 8013233 8013903 8015960 8028771 8054482 8062006 - 8150432 +# 8150432 # @summary tests for "java.locale.providers" system property # @modules java.base/sun.util.locale # java.base/sun.util.locale.provider
Re: JAXP default implementation and JDK-8152063
I think that either of these options would work great for us. On 03/28/2016 05:46 PM, huizhe wang wrote: Thanks David. So I understand the dynamic nature of the server configuration. There maybe two options to solve it: 1) Add a system property to point to a Layer in order to find an alternative-system-default. This will add a new step to the JAXP process after the current ServiceLoader process. I saw that you had concern over the performance of searching a provider among all modules in a Layer. or 2) Add a new type FinderDelegate for processes such as the "proxy" in your case to implement. If the FinderDelegate process fails to locate a provider, it would signal the jaxp process (by returning null) to fall back to the JDK-default implementation. In other words, when the system property points to a FinderDelegate, the 4-step JAXP process is reduced to two: delegate the process to the FinderDelegate, and fall back to the system default implementation. The benefit of this approach is that container developers are allowed complete flexibility to define the process within their environment, thus probably more adaptable to any changes. -Joe On 3/28/2016 4:42 AM, David M. Lloyd wrote: The reason is because we use a single boot path but we don't know at the time of boot whether or not we will have a JAXP provider, nor where it will come from; that is up to the server configuration that we end up running. With the system properties approach we can just clear all the properties, but if we use ServiceLoader then we can't switch it on and off on demand. On 03/26/2016 04:57 PM, huizhe wang wrote: Going back to the original issue a bit, if you don't mind. The issue was that JBoss wished to configure its own default providers for certain JAXP-defined services. What you've been doing until now was to point the system property to a proxy, which, in essence, took over the JAXP provider-lookup process, and served as a factory finder outside of JAXP. "however if we do this but there is/are no overriding implementation(s) for one or more of these APIs, then AFAICT it will fail with Jigsaw because we can't access the JDK's default implementations". So the question is, why did you have to override the JAXP process for the services that you don't have an AppServer-level provider ( "overriding implementation(s)" )? Thanks, Joe On 3/25/2016 5:53 AM, David M. Lloyd wrote: OK, so disappointment again. :) Thanks for responding; the search continues. On 03/24/2016 06:35 PM, huizhe wang wrote: Right, that sounds like what I thought you would want: an additional step in the factory-lookup process to try locating a provider through the Layer of the caller if TCCL fails. I think the assumption in the previous discussion was that a new method would be introduced to take a Layer as an argument. -Joe On 3/24/2016 3:36 PM, David M. Lloyd wrote: This is all for the case where the user is calling e.g. javax.xml.stream.XMLInputFactory#newFactory() with no arguments. We need some way to choose a specific non-JDK default implementation when there is no service loader info on the TCCL. Using the Layer of the caller of the newFactory() method would be an ideal solution from our perspective. On 03/24/2016 05:18 PM, huizhe wang wrote: So specifying Layer is preferred solution. If a new method is needed however, (similar situation to using the method with ClassLoader), that would mean your users' applications are required to adapt to the new API. Would you expect that would happen or would you still have existing applications that can not be updated? -Joe On 3/24/2016 2:02 PM, David M. Lloyd wrote: On 03/24/2016 03:54 PM, huizhe wang wrote: In this discussion so far, a) I see that you seemed to have successfully used the method with a class loader as Daniel suggested. I assume that solved the issue reported in JDK-8152063. Am I right, that you no longer have issue with using a proxy? Or No, not solved yet, just in the process of prototyping but ran into a road block with XMLReaderFactory. b) you do need JAXP's supporting using a Layer as the context in order to solve your issue completely? I think this should be considered the best solution to the problem. c) On org.xml.sax.helpers.XMLReaderFactory, as Alan and Daniel said, yes I'm working with the SAX project to hopefully get a new release out that would conform to the ServiceLoader spec. I'm also suggesting a new method that takes a ClassLoader that would be useful if (a) worked for you. We have some more testing to do before I can say whether this works or does not work. But either way it is a rather non-optimal solution, and if I can avoid using a proxy layer, I would much prefer to do so. Thanks for your response. -- - DML
Re: RFR 9: 8152005: sun/misc/SunMiscSignalTest.java failed intermittently
Hi Roger, If the test is only rarely failing as-is, I'd prefer to keep the first timeout short and only fallback to the longer timeout in the fallback case. What do you think? Thanks, -Joe On 3/28/2016 11:22 AM, Roger Riggs wrote: Please review this improvement to an intermittently failing test of sun.misc.Signal. The timeout value is increased in case of a busy system and for additional debug value the raising of the signal is retried. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-signal-8152005/ Issue: https://bugs.openjdk.java.net/browse/JDK-8152005 Thanks, Roger
Re: JDK 9 RFR of JDK-8151763; Use more informative format for problem list
Hello, New iteration of the webrev updated after the Jigsaw integration and incorporating the earlier feedback. http://cr.openjdk.java.net/~darcy/8151763.1 Thanks, -Joe On 3/16/2016 4:52 PM, Joseph D. Darcy wrote: Hi Jon, Noted; I'll make that improvement in the next round. Thanks for pointing this out, -Joe On 3/16/2016 4:50 PM, Jonathan Gibbons wrote: On 03/11/2016 07:28 PM, joe darcy wrote: Hello, As Jon Gibbons has noted off-list, the problem list entries can directly include the bug number associated with the test in question, enabling better reporting. This format should be used rather than the current convention of putting the bug number in a comment. Please review the webrev to adopt the revised format for the problem list: http://cr.openjdk.java.net/~darcy/8151763.0/ I've verified jtreg produces the same test list with the old and new versions of the problem list. Thanks, -Joe Joe, You can use a comma-separated list when multiple bugs are involved. The only restriction is, no embedded whitespace within the list 342 # Also 8080165 343 java/util/Arrays/ParallelPrefix.java 8085982 generic-all can be 343 java/util/Arrays/ParallelPrefix.java 8085982,8080165 generic-all -- Jon
Re: JAXP default implementation and JDK-8152063
Thanks David. So I understand the dynamic nature of the server configuration. There maybe two options to solve it: 1) Add a system property to point to a Layer in order to find an alternative-system-default. This will add a new step to the JAXP process after the current ServiceLoader process. I saw that you had concern over the performance of searching a provider among all modules in a Layer. or 2) Add a new type FinderDelegate for processes such as the "proxy" in your case to implement. If the FinderDelegate process fails to locate a provider, it would signal the jaxp process (by returning null) to fall back to the JDK-default implementation. In other words, when the system property points to a FinderDelegate, the 4-step JAXP process is reduced to two: delegate the process to the FinderDelegate, and fall back to the system default implementation. The benefit of this approach is that container developers are allowed complete flexibility to define the process within their environment, thus probably more adaptable to any changes. -Joe On 3/28/2016 4:42 AM, David M. Lloyd wrote: The reason is because we use a single boot path but we don't know at the time of boot whether or not we will have a JAXP provider, nor where it will come from; that is up to the server configuration that we end up running. With the system properties approach we can just clear all the properties, but if we use ServiceLoader then we can't switch it on and off on demand. On 03/26/2016 04:57 PM, huizhe wang wrote: Going back to the original issue a bit, if you don't mind. The issue was that JBoss wished to configure its own default providers for certain JAXP-defined services. What you've been doing until now was to point the system property to a proxy, which, in essence, took over the JAXP provider-lookup process, and served as a factory finder outside of JAXP. "however if we do this but there is/are no overriding implementation(s) for one or more of these APIs, then AFAICT it will fail with Jigsaw because we can't access the JDK's default implementations". So the question is, why did you have to override the JAXP process for the services that you don't have an AppServer-level provider ( "overriding implementation(s)" )? Thanks, Joe On 3/25/2016 5:53 AM, David M. Lloyd wrote: OK, so disappointment again. :) Thanks for responding; the search continues. On 03/24/2016 06:35 PM, huizhe wang wrote: Right, that sounds like what I thought you would want: an additional step in the factory-lookup process to try locating a provider through the Layer of the caller if TCCL fails. I think the assumption in the previous discussion was that a new method would be introduced to take a Layer as an argument. -Joe On 3/24/2016 3:36 PM, David M. Lloyd wrote: This is all for the case where the user is calling e.g. javax.xml.stream.XMLInputFactory#newFactory() with no arguments. We need some way to choose a specific non-JDK default implementation when there is no service loader info on the TCCL. Using the Layer of the caller of the newFactory() method would be an ideal solution from our perspective. On 03/24/2016 05:18 PM, huizhe wang wrote: So specifying Layer is preferred solution. If a new method is needed however, (similar situation to using the method with ClassLoader), that would mean your users' applications are required to adapt to the new API. Would you expect that would happen or would you still have existing applications that can not be updated? -Joe On 3/24/2016 2:02 PM, David M. Lloyd wrote: On 03/24/2016 03:54 PM, huizhe wang wrote: In this discussion so far, a) I see that you seemed to have successfully used the method with a class loader as Daniel suggested. I assume that solved the issue reported in JDK-8152063. Am I right, that you no longer have issue with using a proxy? Or No, not solved yet, just in the process of prototyping but ran into a road block with XMLReaderFactory. b) you do need JAXP's supporting using a Layer as the context in order to solve your issue completely? I think this should be considered the best solution to the problem. c) On org.xml.sax.helpers.XMLReaderFactory, as Alan and Daniel said, yes I'm working with the SAX project to hopefully get a new release out that would conform to the ServiceLoader spec. I'm also suggesting a new method that takes a ClassLoader that would be useful if (a) worked for you. We have some more testing to do before I can say whether this works or does not work. But either way it is a rather non-optimal solution, and if I can avoid using a proxy layer, I would much prefer to do so. Thanks for your response.
Re: PING: RFR: JDK-4347142: Need method to set Password protection to Zip entries
Yasumasa It's a tricky call. To be honest, as I said at the very beginning, I'm not sure whether or not it's a good idea and worth the efffort to push this into the j.u.zip package to support the "traditional PKWare encryption", which is known to be "seriously flawed, and in particular is vulnerable to known-plaintext attacks" (from wiki), while I fully understood it is not a concern in your "problem-free" use scenario. Just wonder if there is anyone else on the list that has/had the need for such encryption feature in the past. It would be preferred to have more input (agree, disagree) to make the final decision. Here are some comments regarding the proposed implementation, (1) The changes in native code are probably no longer needed. The native path is left there for vm directly access. (2) I'm concerned about the footprint increase for the ZipEntry class (the proposed change is adding 3 more fields into the entry class). Given this is something rarely needed/used, and we might have hundreds and thousands of zip/jar entries during startup/runtime, it might be desired to avoid adding these fields into ZipEntry class. A possible alternative to have a dedicated entry class extended from the ZipEntry to hold those extra fields and methods, EncryptionZipEntry for example. So the proposed encryption support code will not have any impact to the existing use of ZipInput/OutputStream/File. (3) I'm also not comfortable to add the "encryption engine" logic into the in/deflater stream classes, while it might be convenient to do so from implementation point of view. Given the encryption is something between the raw bites in the zip file and the in/deflating layer, it might need an extra layer there, though I'm not sure if it's easy to do so. The webrev below is what I think might be better for the ZipFile and ZipOutputStream to have an extra layer in between to handle the encryption. Not try to test if it really works, just throw in the idea for discussion. http://cr.openjdk.java.net/~sherman/zencrypt/webrev/ No, I did not try the ZIS, the "pushback" stream there might be a little tricky:-) -Sherman On 03/28/2016 02:34 AM, Yasumasa Suenaga wrote: PING: Could you review it? We want to move forward this enhancement. Thanks, Yasumasa 2016/03/19 22:01 "Yasumasa Suenaga": Hi Alan, I think the main issue here is to decide whether the API should be extended for encryption or not. We've discussed on the premise that we add the API for supporting ZIP encryption. In this context, Sherman tried to implement AES encryption extending current API. [1] IMHO, ZIP encryption should be implemented as extention of current ZIP API because encryption/decryption will process for each ZIP entries. According to Developers' Guide, guideline for adding new API is TBD. [2] What should I do next? Thanks, Yasumsa [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-January/037903.html [2] http://openjdk.java.net/guide/changePlanning.html#api On 2016/03/19 0:25, Alan Bateman wrote: On 18/03/2016 15:02, Yasumasa Suenaga wrote: Hi all, We (including me and Yuji Kubota (ykubota: OpenJDK jdk9 Author)) discussed about this issue from Nov. 2015. [1] We heard several comments and we applied them to our patch. I have not heard new comment for our latest patch. So I send review request for it. Could you review it? webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.04/ Usage of new API: http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.04/Test.java Yasumasa - I think the main issue here is to decide whether the API should be extended for encryption or not. If exposing it in the API make sense then I assume that alternative names to java.util.zip.ZipCryption needs to be explored. -Alan.
Re: RFR: 8152733: Avoid creating Manifest when checking for Multi-Release attribute
On 2016-03-25 17:53, Paul Sandoz wrote: On 25 Mar 2016, at 17:37, Claes Redestad wrote: Interesting, my only concern is we'd add more code/complexity, but it seems the shift will always be == len when not matching a character in src (the proof is left out as an exercise), so this can be simplified further to: byte c = b[i + j]; if (c >= ' ' && c <= 'z') { if (c >= 'a') c -= 32; // Canonicalize if (c != src[j]) { // no match int goodShift = (j < len - 1) ? len : 1; int badShift = lastOcc[c - 32]; i += Math.max(j + 1 - badShift, goodShift); continue next; } } else { // no match, character not valid for name i += len; continue next; } http://cr.openjdk.java.net/~redestad/8152733/webrev.03/ Nice, that’s quite clear in it’s intent. As long as this meets the performance goals i am ok with this being slower than other variants, as i think the reduction in static footprint is a fare trade in this case. Paul. Thanks! Getting a few very marginal but still statistically significant improvements on startup when comparing to earlier variants of this patch, so hearing no objections I'll go ahead and push this variant. /Claes
RFR 9: 8152005: sun/misc/SunMiscSignalTest.java failed intermittently
Please review this improvement to an intermittently failing test of sun.misc.Signal. The timeout value is increased in case of a busy system and for additional debug value the raising of the signal is retried. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-signal-8152005/ Issue: https://bugs.openjdk.java.net/browse/JDK-8152005 Thanks, Roger
Re: RFR(M): 8152172: PPC64: Support AES intrinsics
[Switching to security-dev, core-lib-dev was bcc'ed] I don't see any problem changing it to int[][]. Being an Object[] is odd in my opinion. Tony On 03/25/2016 04:00 PM, Vladimir Kozlov wrote: This question is for core libs. For JIT to generate optimal code we want to change type of AESCrypt::sessionK from Object[] to int[][]. Otherwise JIT have to generate checkcast code to make sure that elements of sessionK array are int[]. I see that sessionK field is private so we are not changing public API. Thanks, Vladimir On 3/24/16 6:17 PM, Hiroshi H Horii wrote: Hi Vladimir, Thank you for your reviewing. Is it possible to modify the type of sessionK from Object[] to int[][]? We can guess, the type was designed for flexibility. However, only int[] is used to store elements of sessionK, so far. In addition, this field is private. Though adding checkcast is another solution, it produces overheads. I attached an additional patch (generated with hg diff -g) to jdk directory. I would like to ask thoughts about this change of java code. Regards, Hiroshi --- Hiroshi Horii, IBM Research - Tokyo Vladimir Kozlov wrote on 03/24/2016 09:09:11: > From: Vladimir Kozlov > To: Hiroshi H Horii/Japan/IBM@IBMJP, "hotspot-compiler- > d...@openjdk.java.net" > Cc: "Doerr, Martin" , Tim Ellison > , "Simonis, Volker" > Date: 03/24/2016 09:19 > Subject: Re: RFR(M): 8152172: PPC64: Support AES intrinsics > > I think we need to add gen_checkcast for objAESCryptKey node since > java code has no guarantee that sessionK array elements are > initialized with int[]. Or we should modify java code to declare sessionK > as int[][]. > > Thanks, > Vladimir > > Note, intrinsics are correctly handle case when > On 3/22/16 8:47 AM, Hiroshi H Horii wrote: > > Dear all: > > > > Can I please request reviews for the following change? > > This change was created for JDK 9 to enable POWER8's AES > > instructions for AES calculation. > > > > This request follows this discussion. > > http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016- > March/021926.html > > > > Description: > > This change adds stub routines support for single-block AES > > encryption and decryption operations on the POWER8 platform. > > They are available only when the application is configured to > > use SunJCE crypto provider on little endian. > > > > These stubs make use of efficient hardware AES instructions > > and thus offer significant performance improvements over > > JITed code on POWER8 as on x86 and SPARC. AES stub routines > > are enabled by default on POWER8 platforms that support AES > > instructions (vcipher). They can be explicitly enabled or > > disabled on the command-line using UseAES and > > UseAESIntrinsics JVM flags. Unlike x86 and SPARC, vcipher > > and vnchiper of POWER8 need the same round keys of AES. > > Therefore, inline_aescrypt_Block in library_call.cpp calls the > > stub with AESCrypt.sessionK[0] as round keys. > > > > Summary of source code changes: > > > > *src/cpu/ppc/vm/assembler_ppc.hpp > > *src/cpu/ppc/vm/assembler_ppc.inline.hpp > > - Adds support for vrld instruction to rotate vector register values > >with left doubleword. > > > > *src/cpu/ppc/vm/stubGenerator_ppc.cpp > > - Defines stubs for single-block AES encryption and decryption > >routines supporting all key sizes (128, 192 and 256-bit). > > - Current POWER AES decryption instructions are not compatible > >with SunJCE expanded decryption key format. Thus decryption > >stubs read the expanded encryption keys (sessionK[0]) with > >descendant order. > > - Encryption stubs use SunJCE expanded encryption key as their > >is no incompatibility issue between POWER8 AES encryption > >instructions and SunJCE expanded encryption keys. > > > > *src/cpu/ppc/vm/vm_version_ppc.cpp > > - Detects AES capabilities of the underlying CPU by using > >has_vcipher(). > > - Enables UseAES and UseAESIntrinsics flags if the underlying > >CPU supports AES instructions and neither of them is explicitly > >disabled on the command-line. Generate warning message if > >either of these flags are enabled on the command-line > >whereas the underlying CPU does not support AES instructions. > > > > *src/share/vm/opto/library_call.cpp > > - Passes the first input parameter, reference to sessionK[0] to the > > AES stubs only on the POWER platform. > > > >*src/share/vm/opto/graphKit.cpp > > - Supports T_NARROWOOP type for GraphKit::load_array_element. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8152172 > > Webrev: http://cr.openjdk.java.net/~mdoerr/8152172_ppc64le_aes/webrev.00/ > > > > > > Regards, > > Hiroshi > > --- > > Hiroshi Horii, > > IBM Research - Tokyo > > >
Re: RFR: JDK-8149925 We don't need jdk.internal.ref.Cleaner any more
Hi Mandy, Kim, Per and Roger I'd like to continue the discussion about the 2nd part of removing jdk.internal.ref.Cleaner in this discussion thread. There was some discussion about whether to synchronize with ReferenceHandler thread and wait for it to enqueue the Reference(s) or simply detect that there are no more pending Reference(s) by timing out on waiting for cleanup actions in discussion thread: "Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently". Based on that discussion, I have prepared a webrev that uses an approach where the detection is performed using timeout: http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.10.part2/ While this webrev passes the DirectBufferAllocTest, I don't have a good feeling about this approach since it is not very robust. I can imagine situations where it would not behave optimally - it would either trigger reference discovery (System.gc()) more frequently that necessary or it would cause delays in execution. So I still prefer the approach where allocating thread(s) explicitly synchronize with ReferenceHandler thread and wait for it to enqueue pending Reference(s). Luckily this can be performed in an easy way (as I will show you shortly). Waiting on discovery of pending references by ReferenceHandler thread and handing them to it could be moved to native code so that no notification would have to be performed in native code from the ReferenceHandler thread to the allocating thread(s). But first, let me reply to Mandy's comments... On 03/25/2016 11:20 PM, Mandy Chung wrote: On Mar 19, 2016, at 7:00 AM, Peter Levart wrote: Here's the webrev: http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.08.part2/ On 03/07/2016 07:35 PM, Mandy Chung wrote: I studied webrev.06priv and the history of JDK-6857566. I’m not comfortable for any arbitrary thread to handle the enqueuing of the pending references (this change is more about the fix for JDK-6857566). Why? A Thread is a Thread is a Thread... When legacy Cleaner is removed, ReferenceHandler thread will be left with swapping pointers only - no custom code will be involved. The only things I can think of against using arbitrary thread are: : My uncomfort was the fix for JDK-6857566 - both enqueuing pending ref and invoking the cleaning code in an arbitrary thread. Looking at it again - enqueuing the pending reference is not so much of a concern (simply updating the link) but the common cleaner could be used by other code that may only expect to be invoked in system thread that’s still my concern (thinking of thread locals). As you'll see in the webrev below, enqueueing is performed solely be ReferenceHandler thread. Allocating thread(s) just wait for it to do its job. There's a little synchronization action performed at the end of enqueueing a chunk of pending references that notifies waiters (allocating threads) so that they can continue. This actually improves throughput (compared to helping enqueue Reference(s) one by one) because there's not much actual work to be done (just swapping pointers) so synchronization dominates. The goal here is to minimize synchronization among threads and by executing enqueuing of the whole bunch of pending references in private by a single thread achieves a reduction in synchronization when lots of Reference(s) are discovered at once - precisely the situation when it matters. OTOH helping the Cleaner thread is beneficial as cleanup actions take time to execute and this is the easiest way to retry allocation while there's still chance it will succeed. As the common Cleaner is using InnocuousThread, cleanup actions can't rely on any thread locals to be preserved from invocation to invocation anyway - they are cleared after each cleanup action so each action gets empty thread locals. We could simulate this in threads that help execute cleanup actions by saving thread-locals to local variables, clearing thread-locals, executing cleanup action and then restoring thread-locals from local variables. Mandy, if you think this is important I'll add such save/clear/restore code to appropriate place. On the other hand, invoking Deallocator::run (deallocating the native memory) in arbitrary threads has no problem. Consider me being paranoid of the fix for JDK-6857566. The current list of clients using CleanerFactory::cleaner may be safe being called from arbitrary threads but I can’t say what will be added in the future. Right, save/clear/restore thread locals then (left for next webrev)... The allocating thread may do a System.gc() that may discover phantom reachable references. All it’s interested is only the direct byte buffer ones so that it can deallocate the native memory. What is the downside of having a dedicated Cleaner for direct byte buffer that could special case for it? A dedicated Cleaner for direct buffers might
Re: RFR: 8151876: (tz) Support tzdata2016b
Hi Ramanand, What I meant is to remove those new resources so that "GMT+hh:mm" is used for formatting. There may be some tests that require changes. Thanks, Masayoshi On 3/28/2016 7:31 PM, Ramanand Patil wrote: Hi Masayoshi, Currently I have not defined the new TimeZoneNames with +hh format, instead I have defined them as per the earlier convention like: +{"Asia/Barnaul", new String[] {"Barnaul Time", "BAT", + "Barnaul Summer Time", "BAST", + "Barnaul Time", "BAT"}}, +{"Europe/Astrakhan", new String[] {"Astrakhan Time", "ASTT", + "Astrakhan Summer Time", "ASTST", + "Astrakhan Time", "ASTT"}}, +{"Europe/Ulyanovsk", new String[] {"Ulyanovsk Time", "ULT", + "Ulyanovsk Summer Time", "ULST", + "Ulyanovsk Time", "ULT"}}, Please let me know if this is fine. Regards, Ramanand. -Original Message- From: Masayoshi Okutsu Sent: Wednesday, March 23, 2016 7:22 PM To: Ramanand Patil; i18n-...@openjdk.java.net Cc: core-libs-dev@openjdk.java.net Subject: Re: RFR: 8151876: (tz) Support tzdata2016b Sorry for this belated response. I prefer to follow the tzdata abbreviations, like "+04". But that would require some major changes. An option would be not to define time zone names for the new zones with the +hh format. Thanks, Masayoshi On 3/17/2016 4:38 AM, Ramanand Patil wrote: Hi all, Please review the latest TZDATA (tzdata2016b) integration to JDK9. Bug: https://bugs.openjdk.java.net/browse/JDK-8151876 Webrev: http://cr.openjdk.java.net/~rpatil/8151876/webrev.00/ All the TimeZone related tests are passed after integration. Please note that, as per the release notes: As a trial of a new system that needs less information to be made up, the new zones use numeric time zone abbreviations like "+04" instead of invented abbreviations like "ASTT". Since this is a trial system, I have ignored this in the current patch which adds 3 new timezones. But I think it's worth discussing this new system along with this bug and get the experts opinion on this. Do you think that JDK should also follow these IANA timezone naming system for zone abbreviations in future ? Will it be convenient to use such numeric time zone abbreviations ? (Also, it looks like the abbreviations similar to "+03/+04" will be used for the zones linked to the rule set with changing letters like: EST/EDT for US, MSK/MSD for RU, etc..). Regards, Ramanand.
Re: JAXP default implementation and JDK-8152063
The reason is because we use a single boot path but we don't know at the time of boot whether or not we will have a JAXP provider, nor where it will come from; that is up to the server configuration that we end up running. With the system properties approach we can just clear all the properties, but if we use ServiceLoader then we can't switch it on and off on demand. On 03/26/2016 04:57 PM, huizhe wang wrote: Going back to the original issue a bit, if you don't mind. The issue was that JBoss wished to configure its own default providers for certain JAXP-defined services. What you've been doing until now was to point the system property to a proxy, which, in essence, took over the JAXP provider-lookup process, and served as a factory finder outside of JAXP. "however if we do this but there is/are no overriding implementation(s) for one or more of these APIs, then AFAICT it will fail with Jigsaw because we can't access the JDK's default implementations". So the question is, why did you have to override the JAXP process for the services that you don't have an AppServer-level provider ( "overriding implementation(s)" )? Thanks, Joe On 3/25/2016 5:53 AM, David M. Lloyd wrote: OK, so disappointment again. :) Thanks for responding; the search continues. On 03/24/2016 06:35 PM, huizhe wang wrote: Right, that sounds like what I thought you would want: an additional step in the factory-lookup process to try locating a provider through the Layer of the caller if TCCL fails. I think the assumption in the previous discussion was that a new method would be introduced to take a Layer as an argument. -Joe On 3/24/2016 3:36 PM, David M. Lloyd wrote: This is all for the case where the user is calling e.g. javax.xml.stream.XMLInputFactory#newFactory() with no arguments. We need some way to choose a specific non-JDK default implementation when there is no service loader info on the TCCL. Using the Layer of the caller of the newFactory() method would be an ideal solution from our perspective. On 03/24/2016 05:18 PM, huizhe wang wrote: So specifying Layer is preferred solution. If a new method is needed however, (similar situation to using the method with ClassLoader), that would mean your users' applications are required to adapt to the new API. Would you expect that would happen or would you still have existing applications that can not be updated? -Joe On 3/24/2016 2:02 PM, David M. Lloyd wrote: On 03/24/2016 03:54 PM, huizhe wang wrote: In this discussion so far, a) I see that you seemed to have successfully used the method with a class loader as Daniel suggested. I assume that solved the issue reported in JDK-8152063. Am I right, that you no longer have issue with using a proxy? Or No, not solved yet, just in the process of prototyping but ran into a road block with XMLReaderFactory. b) you do need JAXP's supporting using a Layer as the context in order to solve your issue completely? I think this should be considered the best solution to the problem. c) On org.xml.sax.helpers.XMLReaderFactory, as Alan and Daniel said, yes I'm working with the SAX project to hopefully get a new release out that would conform to the ServiceLoader spec. I'm also suggesting a new method that takes a ClassLoader that would be useful if (a) worked for you. We have some more testing to do before I can say whether this works or does not work. But either way it is a rather non-optimal solution, and if I can avoid using a proxy layer, I would much prefer to do so. Thanks for your response. -- - DML
Re: [DING] Re: [PING] Potential infinite waiting at JMXConnection#createConnection
Hi Mark, Could you please review my attached patch and reproducer of JDK-8151212 [1] ? If do you have a comment(s), please let me know. [1]: https://bugs.openjdk.java.net/browse/JDK-8151212 Thanks, Yuji 2016-03-05 3:19 GMT+09:00 KUBOTA Yuji : > Hi Roger and all, > > Thanks your help to share patch and link to here :) > > For the assignee, more detailed about the attachments is as below. > http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-February/038593.html > > Thanks, > Yuji > > 2016-03-05 0:56 GMT+09:00 Roger Riggs : >> Hi Yuji, >> >> The patch and reproducer have been attached to the issue 8151212[1]. >> >> Thanks, Roger >> >> [1] https://bugs.openjdk.java.net/browse/JDK-8151212 >> >> On 3/3/2016 9:06 PM, KUBOTA Yuji wrote: >> >> Hi Roger, >> >> Thank you for your help! >> My patch and reproducer are as below. >> >> >>
RE: RFR: 8151876: (tz) Support tzdata2016b
Hi Masayoshi, Currently I have not defined the new TimeZoneNames with +hh format, instead I have defined them as per the earlier convention like: +{"Asia/Barnaul", new String[] {"Barnaul Time", "BAT", + "Barnaul Summer Time", "BAST", + "Barnaul Time", "BAT"}}, +{"Europe/Astrakhan", new String[] {"Astrakhan Time", "ASTT", + "Astrakhan Summer Time", "ASTST", + "Astrakhan Time", "ASTT"}}, +{"Europe/Ulyanovsk", new String[] {"Ulyanovsk Time", "ULT", + "Ulyanovsk Summer Time", "ULST", + "Ulyanovsk Time", "ULT"}}, Please let me know if this is fine. Regards, Ramanand. -Original Message- From: Masayoshi Okutsu Sent: Wednesday, March 23, 2016 7:22 PM To: Ramanand Patil; i18n-...@openjdk.java.net Cc: core-libs-dev@openjdk.java.net Subject: Re: RFR: 8151876: (tz) Support tzdata2016b Sorry for this belated response. I prefer to follow the tzdata abbreviations, like "+04". But that would require some major changes. An option would be not to define time zone names for the new zones with the +hh format. Thanks, Masayoshi On 3/17/2016 4:38 AM, Ramanand Patil wrote: > Hi all, > > Please review the latest TZDATA (tzdata2016b) integration to JDK9. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8151876 > > Webrev: http://cr.openjdk.java.net/~rpatil/8151876/webrev.00/ > > All the TimeZone related tests are passed after integration. > > > > Please note that, as per the release notes: As a trial of a new system that > needs less information to be made up, the new zones use numeric time zone > abbreviations like "+04" instead of invented abbreviations like "ASTT". > > Since this is a trial system, I have ignored this in the current patch which > adds 3 new timezones. But I think it's worth discussing this new system along > with this bug and get the experts opinion on this. > > Do you think that JDK should also follow these IANA timezone naming system > for zone abbreviations in future ? Will it be convenient to use such numeric > time zone abbreviations ? (Also, it looks like the abbreviations similar to > "+03/+04" will be used for the zones linked to the rule set with changing > letters like: EST/EDT for US, MSK/MSD for RU, etc..). > > > > Regards, > > Ramanand. > >
PING: RFR: JDK-4347142: Need method to set Password protection to Zip entries
PING: Could you review it? We want to move forward this enhancement. Thanks, Yasumasa 2016/03/19 22:01 "Yasumasa Suenaga" : > Hi Alan, > > > I think the main issue here is to decide whether the API > > should be extended for encryption or not. > > We've discussed on the premise that we add the API for supporting ZIP > encryption. > In this context, Sherman tried to implement AES encryption extending > current API. [1] > > IMHO, ZIP encryption should be implemented as extention of current ZIP API > because encryption/decryption will process for each ZIP entries. > > According to Developers' Guide, guideline for adding new API is TBD. [2] > What should I do next? > > > Thanks, > > Yasumsa > > > [1] > http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-January/037903.html > [2] http://openjdk.java.net/guide/changePlanning.html#api > > > On 2016/03/19 0:25, Alan Bateman wrote: > > > > On 18/03/2016 15:02, Yasumasa Suenaga wrote: > >> Hi all, > >> > >> We (including me and Yuji Kubota (ykubota: OpenJDK jdk9 Author)) > discussed > >> about this issue from Nov. 2015. [1] > >> We heard several comments and we applied them to our patch. > >> > >> I have not heard new comment for our latest patch. > >> So I send review request for it. Could you review it? > >> > >>webrev: > >> http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.04/ > >> > >>Usage of new API: > >> > http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.04/Test.java > >> > > Yasumasa - I think the main issue here is to decide whether the API > > should be extended for encryption or not. If exposing it in the API make > > sense then I assume that alternative names to java.util.zip.ZipCryption > > needs to be explored. > > > > -Alan. > > >