Re: JDK-8036003: Add variable not to separate debug information.
I don't think this quite works as there are other variations not captured here. Rather than zipped it should just be external. Whether the debuginfo files are zipped or not is then an additional build time option. Additionally we still have to be able to control the degree of stripping that is carried out. It doesn't make sense to me to try and enumerate all possible combinations as top-level configure options. Further, as Dan was saying, this doesn't capture the overlap between internal and external because we still leave some symbols internal even when creating the debuginfo file. So I don't think this proposed categorization works. We still have three aspects: - generating symbols into the object files - stripping symbols from the final binaries - saving symbols into an external form Each of which can potentally be varied (of course if you don't generate symbols in the obj files stripping and saving are non issues). But they are not independent on each other! Unless you generate debug symbols, you can't strip them, nor save them elsewhere. So generating debug symbols (your item #1) is a prerequisite for the rest of your items. And while, technically, you can save symbols externally, and at the same time keep them in the binary, that does not make sense. So, in a practical sense, you are going to do #2 if you are going to do #3. And you can't zip external symbols unless you create a non-zipped version. And if you zip them, it does not make sense to keep the non-zipped version. And yes, we do not strip all debug information when creating external debug info. But there seems to be no real use case (not even for IcedTea, as it turned out) to want a completely stripped binary, I don't think that's worth discussing much. That's just part of how the external debuginfo scheme works. Can you give a more concrete example of the variations of your aspects that you think my suggested solution would not capture? /Magnus
Re: JDK-8036003: Add variable not to separate debug information.
On 21/03/2014 6:14 PM, Magnus Ihse Bursie wrote: I don't think this quite works as there are other variations not captured here. Rather than zipped it should just be external. Whether the debuginfo files are zipped or not is then an additional build time option. Additionally we still have to be able to control the degree of stripping that is carried out. It doesn't make sense to me to try and enumerate all possible combinations as top-level configure options. Further, as Dan was saying, this doesn't capture the overlap between internal and external because we still leave some symbols internal even when creating the debuginfo file. So I don't think this proposed categorization works. We still have three aspects: - generating symbols into the object files - stripping symbols from the final binaries - saving symbols into an external form Each of which can potentally be varied (of course if you don't generate symbols in the obj files stripping and saving are non issues). But they are not independent on each other! Unless you generate debug symbols, you can't strip them, nor save them elsewhere. So generating debug symbols (your item #1) is a prerequisite for the rest of your items. Yes I just said that. :) And while, technically, you can save symbols externally, and at the same time keep them in the binary, that does not make sense. So, in a practical sense, you are going to do #2 if you are going to do #3. But you can vary what is kept internally independent of what is made external. And you can't zip external symbols unless you create a non-zipped version. And if you zip them, it does not make sense to keep the non-zipped version. zip vs non-zip is just an issue of disk space. It is not a fundamental configuration choice, just a variation on how external symbols are packaged. And yes, we do not strip all debug information when creating external debug info. But there seems to be no real use case (not even for IcedTea, as it turned out) to want a completely stripped binary, I don't think that's worth discussing much. That's just part of how the external debuginfo scheme works. Umm we fully strip all our binaries in embedded. Can you give a more concrete example of the variations of your aspects that you think my suggested solution would not capture? I think I already have. There aren't tree simple choices here, there are three aspects, each of which could have different variants. If I could draw a flow chart easily I would but basically if you generate debug symbols you can then select what symbols are kept internally (the strip policy) and what are put externally (objcopy options), then for the external symbol file you can choose zipped or unzipped. Multiple inter-connected choices, not just three (or four if you add zipped) David /Magnus
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
You’re right but we’ve never received a report of any charset interop. issues. Probably such a scenario has never been encountered by customers. On 21 Mar 2014, at 05:54, Xueming Shen xueming.s...@oracle.com wrote: Obj.java:#482 It appears sun.misc.BASE64Decoder.decodeBuffer(String) uses String's deprecated String.getBytes(int srcBegin, int srcEnd, byte[] dst, int dstBegin). The proposed change now uses the jvm's default charset. It might trigger incompatible behavior if the default charset is not an ASCII compatible charset. But if the Java object in LDAP was encoded with the platform default charset (as the new comment suggested), the old implementation actually did not work on platform that the default encoding is not ASCII compatible, such as the IBM ebcdic. -Sherman On 3/20/14 3:48 PM, Mandy Chung wrote: On 3/19/14 12:28 PM, Xueming Shen wrote: On 03/19/2014 11:37 AM, Mandy Chung wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy The sun.misc.BASE64En/Decoder is MIME type, so it outputs the \r\n per 76 characters during encoding, and ignores/skip \r or \n when decoding. The new Base64.getEncoder/Decoder() returns the basic Base64 coder, which it never inserts line separator when output, and throws exception for any non-base64- alphabet character, including \r and \n. The only disadvantage/incompatibility (j.u.Base64.getMimeDecoer() vs sun.misc.BASE64Decoder) of switching to j.u.Base64 MIME type en/decoder is that the Base64 Mime decoder ignores/skips any non-base64-alphabet (including \r and \n), while sun.misc.BASE64Decoder appears to simply use the init value -1 for any non-base64-alphabet character for decoding. I'm not familiar with the use scenario of ldap's Obj class, so I'm not sure if it matters (if it ever outputs/inputs 76 character data, or even it does,if the difference matters). Btw, except getMimeEncoder(int ...) all other Base64.getXXXEn/Decoder() returns singleton, so the de/encoder cache might not be necessary. Thanks Sherman. Vinnie confirms that it should retain the current behavior as there could be long-lived Java object in LDAP encoded with JDK 8 for example and then retrieved with JDK 9. Here is the updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.01/ Thanks Mandy
Re: JDK-8036003: Add variable not to separate debug information.
David, In practice, we don't have to much to keep internally. There are no reason to copy out some of .debug_* sections but keep other ones. So we have a matrix: (a) Strip mode: 1. full 2. keep symbols 3. keep symbols and .debug_* (b) Copy mode: 1. Copy all to ext file 2. Copy none to ext file (c) Compression mode: 1. none 2. per-section compression, SHF_GNU_COMPRESSED [1] 3. zip entire file Where *a2 + b2 + c2* have perfect sense, *a1 + b1 + c3* have no sense etc. -Dmitry Refs: 1. https://sourceware.org/bugzilla/show_bug.cgi?id=11819 http://gcc.gnu.org/ml/gcc-patches/2013-04/msg01780.html -Dmitry On 2014-03-21 12:57, David Holmes wrote: On 21/03/2014 6:14 PM, Magnus Ihse Bursie wrote: I don't think this quite works as there are other variations not captured here. Rather than zipped it should just be external. Whether the debuginfo files are zipped or not is then an additional build time option. Additionally we still have to be able to control the degree of stripping that is carried out. It doesn't make sense to me to try and enumerate all possible combinations as top-level configure options. Further, as Dan was saying, this doesn't capture the overlap between internal and external because we still leave some symbols internal even when creating the debuginfo file. So I don't think this proposed categorization works. We still have three aspects: - generating symbols into the object files - stripping symbols from the final binaries - saving symbols into an external form Each of which can potentally be varied (of course if you don't generate symbols in the obj files stripping and saving are non issues). But they are not independent on each other! Unless you generate debug symbols, you can't strip them, nor save them elsewhere. So generating debug symbols (your item #1) is a prerequisite for the rest of your items. Yes I just said that. :) And while, technically, you can save symbols externally, and at the same time keep them in the binary, that does not make sense. So, in a practical sense, you are going to do #2 if you are going to do #3. But you can vary what is kept internally independent of what is made external. And you can't zip external symbols unless you create a non-zipped version. And if you zip them, it does not make sense to keep the non-zipped version. zip vs non-zip is just an issue of disk space. It is not a fundamental configuration choice, just a variation on how external symbols are packaged. And yes, we do not strip all debug information when creating external debug info. But there seems to be no real use case (not even for IcedTea, as it turned out) to want a completely stripped binary, I don't think that's worth discussing much. That's just part of how the external debuginfo scheme works. Umm we fully strip all our binaries in embedded. Can you give a more concrete example of the variations of your aspects that you think my suggested solution would not capture? I think I already have. There aren't tree simple choices here, there are three aspects, each of which could have different variants. If I could draw a flow chart easily I would but basically if you generate debug symbols you can then select what symbols are kept internally (the strip policy) and what are put externally (objcopy options), then for the external symbol file you can choose zipped or unzipped. Multiple inter-connected choices, not just three (or four if you add zipped) David /Magnus -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the source code.
Re: JDK-8036003: Add variable not to separate debug information.
On 21/03/2014 7:36 PM, Dmitry Samersoff wrote: David, In practice, we don't have to much to keep internally. There are no reason to copy out some of .debug_* sections but keep other ones. I'm not familiar with exactly what the different strip options do but the point is this is covered by the strip-policy. So we have a matrix: (a) Strip mode: 1. full 2. keep symbols 3. keep symbols and .debug_* (b) Copy mode: 1. Copy all to ext file 2. Copy none to ext file (c) Compression mode: 1. none 2. per-section compression, SHF_GNU_COMPRESSED [1] 3. zip entire file Where *a2 + b2 + c2* have perfect sense, So now your compression mode may apply to either the external file or the original? That's even more permutations. *a1 + b1 + c3* have no sense etc. Why does full strip + copy-all + zip make no sense? It is exactly what we do with embedded builds. (Naturally you have to copy before you strip) David - -Dmitry Refs: 1. https://sourceware.org/bugzilla/show_bug.cgi?id=11819 http://gcc.gnu.org/ml/gcc-patches/2013-04/msg01780.html -Dmitry On 2014-03-21 12:57, David Holmes wrote: On 21/03/2014 6:14 PM, Magnus Ihse Bursie wrote: I don't think this quite works as there are other variations not captured here. Rather than zipped it should just be external. Whether the debuginfo files are zipped or not is then an additional build time option. Additionally we still have to be able to control the degree of stripping that is carried out. It doesn't make sense to me to try and enumerate all possible combinations as top-level configure options. Further, as Dan was saying, this doesn't capture the overlap between internal and external because we still leave some symbols internal even when creating the debuginfo file. So I don't think this proposed categorization works. We still have three aspects: - generating symbols into the object files - stripping symbols from the final binaries - saving symbols into an external form Each of which can potentally be varied (of course if you don't generate symbols in the obj files stripping and saving are non issues). But they are not independent on each other! Unless you generate debug symbols, you can't strip them, nor save them elsewhere. So generating debug symbols (your item #1) is a prerequisite for the rest of your items. Yes I just said that. :) And while, technically, you can save symbols externally, and at the same time keep them in the binary, that does not make sense. So, in a practical sense, you are going to do #2 if you are going to do #3. But you can vary what is kept internally independent of what is made external. And you can't zip external symbols unless you create a non-zipped version. And if you zip them, it does not make sense to keep the non-zipped version. zip vs non-zip is just an issue of disk space. It is not a fundamental configuration choice, just a variation on how external symbols are packaged. And yes, we do not strip all debug information when creating external debug info. But there seems to be no real use case (not even for IcedTea, as it turned out) to want a completely stripped binary, I don't think that's worth discussing much. That's just part of how the external debuginfo scheme works. Umm we fully strip all our binaries in embedded. Can you give a more concrete example of the variations of your aspects that you think my suggested solution would not capture? I think I already have. There aren't tree simple choices here, there are three aspects, each of which could have different variants. If I could draw a flow chart easily I would but basically if you generate debug symbols you can then select what symbols are kept internally (the strip policy) and what are put externally (objcopy options), then for the external symbol file you can choose zipped or unzipped. Multiple inter-connected choices, not just three (or four if you add zipped) David /Magnus
Re: JDK-8036003: Add variable not to separate debug information.
David, My point was that we don't need in fine grained selection of elf sections on strip - three options are enough. Why does full strip + copy-all + zip make no sense? It is exactly what we do with embedded builds. (Naturally you have to copy before you strip) Sorry! it should be: full strip + copy-none + zip -Dmitry On 2014-03-21 15:13, David Holmes wrote: On 21/03/2014 7:36 PM, Dmitry Samersoff wrote: David, In practice, we don't have to much to keep internally. There are no reason to copy out some of .debug_* sections but keep other ones. I'm not familiar with exactly what the different strip options do but the point is this is covered by the strip-policy. So we have a matrix: (a) Strip mode: 1. full 2. keep symbols 3. keep symbols and .debug_* (b) Copy mode: 1. Copy all to ext file 2. Copy none to ext file (c) Compression mode: 1. none 2. per-section compression, SHF_GNU_COMPRESSED [1] 3. zip entire file Where *a2 + b2 + c2* have perfect sense, So now your compression mode may apply to either the external file or the original? That's even more permutations. *a1 + b1 + c3* have no sense etc. Why does full strip + copy-all + zip make no sense? It is exactly what we do with embedded builds. (Naturally you have to copy before you strip) David - -Dmitry Refs: 1. https://sourceware.org/bugzilla/show_bug.cgi?id=11819 http://gcc.gnu.org/ml/gcc-patches/2013-04/msg01780.html -Dmitry On 2014-03-21 12:57, David Holmes wrote: On 21/03/2014 6:14 PM, Magnus Ihse Bursie wrote: I don't think this quite works as there are other variations not captured here. Rather than zipped it should just be external. Whether the debuginfo files are zipped or not is then an additional build time option. Additionally we still have to be able to control the degree of stripping th *Subject:* Fwd: Re: Compute sizes of classes loat is carried out. It doesn't make sense to me to try and enumerate all possible combinations as top-level configure options. Further, as Dan was saying, this doesn't capture the overlap between internal and external because we still leave some symbols internal even when creating the debuginfo file. So I don't think this proposed categorization works. We still have three aspects: - generating symbols into the object files - stripping symbols from the final binaries - saving symbols into an external form Each of which can potentally be varied (of course if you don't generate symbols in the obj files stripping and saving are non issues). But they are not independent on each other! Unless you generate debug symbols, you can't strip them, nor save them elsewhere. So generating debug symbols (your item #1) is a prerequisite for the rest of your items. Yes I just said that. :) And while, technically, you can save symbols externally, and at the same time keep them in the binary, that does not make sense. So, in a practical sense, you are going to do #2 if you are going to do #3. But you can vary what is kept internally independent of what is made external. And you can't zip external symbols unless you create a non-zipped version. And if you zip them, it does not make sense to keep the non-zipped version. zip vs non-zip is just an issue of disk space. It is not a fundamental configuration choice, just a variation on how external symbols are packaged. And yes, we do not strip all debug information when creating external debug info. But there seems to be no real use case (not even for IcedTea, as it turned out) to want a completely stripped binary, I don't think that's worth discussing much. That's just part of how the external debuginfo scheme works. Umm we fully strip all our binaries in embedded. Can you give a more concrete example of the variations of your aspects that you think my suggested solution would not capture? I think I already have. There aren't tree simple choices here, there are three aspects, each of which could have different variants. If I could draw a flow chart easily I would but basically if you generate debug symbols you can then select what symbols are kept internally (the strip policy) and what are put externally (objcopy options), then for the external symbol file you can choose zipped or unzipped. Multiple inter-connected choices, not just three (or four if you add zipped) David /Magnus -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the source code.
Re: AWT Dev [9] Review request: new macro for conversion to jboolean
On 3/21/14 7:10 PM, roger riggs wrote:
The macro would just as useful (if I understand the cases) without the
cast.
How useful is a simple definition as:
#define IS_TRUE(obj) ((obj) ? JNI_TRUE : JNI_FALSE)
then it would look ok to see these in sources:
return IS_TRUE(obj);
if (IS_TRUE(obj)) {}
jboolean ret = IS_TRUE(obj);
The general purpose usage matches the general C conventions for
true and false and match the JNI semantics.
Actually that was my initial suggestion(name and usage).
Roger
On 3/19/2014 7:43 PM, Sergey Bylokhov wrote:
Thanks.
So if nobody objects, the final version will be:
#define IS_NULL(obj) ((obj) == NULL)
#define JNU_IsNull(env,obj) ((obj) == NULL)
+#define TO_JBOOLEAN(obj) (jboolean) ((obj) ? JNI_TRUE : JNI_FALSE)
On 3/20/14 12:07 AM, roger riggs wrote:
Hi,
Well... When JNU_RETURN was added, there was a long discussion about
NOT using JNU unless the JNI environment was an argument to the macro.
So, the simple name is preferred.
Roger
On 3/19/2014 4:08 PM, Phil Race wrote:
PS .. so maybe whilst you are touching this file you could do
#define JNU_CHECK_NULL CHECK_NULL
#define JNU_CHECK_NULL_RETURN CHECK_NULL_RETURN
so we could migrate to these (clearer) ones
-phil.
On 3/19/2014 1:05 PM, Phil Race wrote:
I think having it start with JNU_ is a good suggestion.
I've been wondering over the last week or so if it would not have
been
better to have CHECK_NULL called JNU_CHECK_NULL to reduce
collision chances
and to make it clearer where it comes from ..
-phil.
On 3/19/2014 12:49 PM, Mike Duigou wrote:
Definitely a useful macro.
I too would prefer a name like TO_JBOOLEAN since it reveals the
result type. Also all uppercase to identify it as a macro. If we
are paranoid and want to reduce the chance of a name collision
then JNU_TO_JBOOLEAN perhaps.
I would also define the macro as:
#define JNU_TO_JBOOLEAN(obj) (jboolean) ((obj) ? JNI_TRUE :
JNI_FALSE)
so that the type of the result is explicit. Unfortunately jni.h
doesn't define JNI_TRUE or false with a cast to jboolean as they
probably should.
Mike
On Mar 19 2014, at 11:36 , Sergey Bylokhov
sergey.bylok...@oracle.com wrote:
Thanks Anthony!
Can somebody from the core-libs team take a look?
On 3/17/14 10:31 PM, Anthony Petrov wrote:
Personally, I'd call it to_jboolean(obj), but IS_TRUE(obj)
sounds good to me too. Either way, I'm fine with the fix.
--
best regards,
Anthony
On 3/17/2014 7:01 PM, Sergey Bylokhov wrote:
Hello.
This review request is for the new macro, which simplify
conversion to
jboolean. It will be useful for fixing parfait warnings.
We have a lot of places, where we cast some type to jboolean:
BOOL = retVal;
return (jboolean) retVal;
WARNING: Expecting value of JNI primitive type jboolean:
mismatched
value retVal with size 32 bits, retVal used for conversion to
int8 in return
+++ b/src/share/native/common/jni_util.hMon Mar 17
18:28:48 2014 +0400
@@ -277,6 +277,7 @@
#define IS_NULL(obj) ((obj) == NULL)
#define JNU_IsNull(env,obj) ((obj) == NULL)
+#define IS_TRUE(obj) ((obj) ? JNI_TRUE : JNI_FALSE)
I am not sure about the name, probably someone have a better
suggestion?
The fix is for jdk9/dev.
--
Best regards, Sergey.
--
Best regards, Sergey.
--
Best regards, Sergey.
Re: RFR (L) 8037210: Get rid of char-based descriptions 'J' of basic types
Chris,
Thanks for the feedback.
What do you think about the following:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/
Best regards,
Vladimir Ivanov
On 3/19/14 5:18 AM, Christian Thalinger wrote:
On Mar 18, 2014, at 2:35 PM, John Rose john.r.r...@oracle.com
mailto:john.r.r...@oracle.com wrote:
On Mar 18, 2014, at 1:36 PM, Christian Thalinger
christian.thalin...@oracle.com
mailto:christian.thalin...@oracle.com wrote:
Why are we not using an Enum instead of an untyped byte?
Byte is moderately typed, in the sense (which I rely on during
development) that you can't assign an int or char to a byte w/o a cast.
That's why it is not just a plain int.
But those values (L_TYPE etc.) are used a lot as numbers, and
specifically as low-level array indexes, and also comparisons (x
V_TYPE).
To turn them into enums, we'd have to add lots of calls to
'.ordinal()' to turn them right back to numbers. That dilutes
(completely IMO) the value they have as enums to raise the level of
the code.
But without being strongly typed we get bugs like this one:
@Override
-MethodHandle bindArgument(int pos, char basicType, Object value) {
+MethodHandle bindArgument(int pos, byte basicType, Object value) {
// If the member needs dispatching, do so.
if (pos == 0 basicType == 'L') {
I’m just saying that for the sake of maintainability and correctness an
Enum would be better.
— John
___
mlvm-dev mailing list
mlvm-...@openjdk.java.net mailto:mlvm-...@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
___
mlvm-dev mailing list
mlvm-...@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
RFR: 8034944: (process) Improve subprocess handling on Solaris
Hi folks, I'd like to push this change into JDK8 7. There is other work going on in 9 which will supersede this fix so there is likely no point in pushing to that release. This is backport of the threadpool specific changes from https://bugs.openjdk.java.net/browse/JDK-6944584 along with some cosmetic changes in order to keep the code somewhat similar. In a nutshell a new process reaper thread was spawned for every Process created by the JDK. This fix runs these reaper threads in a thread pool to save on thread creation when creating a lot of new processes. http://cr.openjdk.java.net/~robm/8034944/webrev.01/ -Rob
Re: RFR: 8034944: (process) Improve subprocess handling on Solaris
..just realised I had an out of date webrev up there, I've just updated in place in case anyone is looking at it. -Rob On 21/03/14 17:43, Rob McKenna wrote: Hi folks, I'd like to push this change into JDK8 7. There is other work going on in 9 which will supersede this fix so there is likely no point in pushing to that release. This is backport of the threadpool specific changes from https://bugs.openjdk.java.net/browse/JDK-6944584 along with some cosmetic changes in order to keep the code somewhat similar. In a nutshell a new process reaper thread was spawned for every Process created by the JDK. This fix runs these reaper threads in a thread pool to save on thread creation when creating a lot of new processes. http://cr.openjdk.java.net/~robm/8034944/webrev.01/ -Rob
Re: RFR: 8034944: (process) Improve subprocess handling on Solaris
Just when I thought I beat you to it Martin. :) On 21/03/14 18:32, Martin Buchholz wrote: OK, latest webrev looks much smaller, and appears to have only a port of changes from my changes to Linux from a few years ago. (please confirm). If so, Looks Good To Me. Yup, everything that is in this webrev comes from your port. (though conversely not everything from your fix was ported over) And yep, this is a sideport. While looking at this, I think bumping the thread stack size by a factor of 2 (32k = 64k) seems timely, since native stack requirements (native sizes, alignments and overhead) seem to increase over time. But I'd put that into a separate change. Thanks for the comment, I'll take an AI. -Rob On Fri, Mar 21, 2014 at 11:23 AM, Rob McKenna rob.mcke...@oracle.com mailto:rob.mcke...@oracle.com wrote: ..just realised I had an out of date webrev up there, I've just updated in place in case anyone is looking at it. -Rob On 21/03/14 17:43, Rob McKenna wrote: Hi folks, I'd like to push this change into JDK8 7. There is other work going on in 9 which will supersede this fix so there is likely no point in pushing to that release. This is backport of the threadpool specific changes from https://bugs.openjdk.java.net/browse/JDK-6944584 along with some cosmetic changes in order to keep the code somewhat similar. In a nutshell a new process reaper thread was spawned for every Process created by the JDK. This fix runs these reaper threads in a thread pool to save on thread creation when creating a lot of new processes. http://cr.openjdk.java.net/~robm/8034944/webrev.01/ http://cr.openjdk.java.net/%7Erobm/8034944/webrev.01/ -Rob
Re: RFR (L) 8037210: Get rid of char-based descriptions 'J' of basic types
On Mar 21, 2014, at 8:49 AM, Vladimir Ivanov vladimir.x.iva...@oracle.com
wrote:
Thanks for the feedback.
What do you think about the following:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/
That looks nice. Strong typing; who woulda' thunk it. :-)
The uses of .ordinal() are the extra cost relative to using just small
integers. They seem totally reasonable in the code.
I suggest either wrapping ordinal as something like id or else changing
temp names like id to ord, to reinforce the use of a common name.
Don't make the enum class public. And especially don't make the mutable arrays
public:
+public static final BasicType[] ALL_TYPES = { L_TYPE, I_TYPE, J_TYPE,
F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE, J_TYPE,
F_TYPE, D_TYPE };
— John
P.S. That would only be safe if we had (what we don't yet) a notion of frozen
arrays like:
+public static final BasicType final[] ALL_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType final[] ARG_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE };
Re: RFR (L) 8037210: Get rid of char-based descriptions 'J' of basic types
On 03/21/2014 07:54 PM, John Rose wrote:
On Mar 21, 2014, at 8:49 AM, Vladimir Ivanov
vladimir.x.iva...@oracle.com mailto:vladimir.x.iva...@oracle.com
wrote:
Thanks for the feedback.
What do you think about the following:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/
http://cr.openjdk.java.net/%7Evlivanov/8037210/webrev.01/
That looks nice. Strong typing; who woulda' thunk it. :-)
The uses of .ordinal() are the extra cost relative to using just
small integers. They seem totally reasonable in the code.
I suggest either wrapping ordinal as something like id or else
changing temp names like id to ord, to reinforce the use of a
common name.
Don't make the enum class public. And especially don't make the
mutable arrays public:
+ public static final BasicType[] ALL_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+ public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE };
— John
P.S. That would only be safe if we had (what we don't yet) a notion of
frozen arrays like:
+ public static final BasicType final[] ALL_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+ public static final BasicType final[] ARG_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE };
or
public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE, J_TYPE,
F_TYPE, D_TYPE }.frozen();
because frozen() is a dynamic property.
Rémi
JDK 9 RFR of 8038163: Build failure on Mac OS 10.9.2 (Mavericks) due to warning treated as error
Please review at your convenience: Issue: https://bugs.openjdk.java.net/browse/JDK-8038163 Patch: http://cr.openjdk.java.net/~bpb/8038163/ My only question would be whether the changed line 291 should instead be this: return (major 1 || (major == 1 minor = 2)) ? JNI_TRUE : JNI_FALSE; Thanks, Brian
Re: JDK 9 RFR of 8038163: Build failure on Mac OS 10.9.2 (Mavericks) due to warning treated as error
Looks good to me. The version with JNI_TRUE/JNI_FALSE is more correct, but I’m fine with either one. (note: this review request should have been sent to serviceability-dev). Thanks, /Staffan On 21 mar 2014, at 20:41, Brian Burkhalter brian.burkhal...@oracle.com wrote: Please review at your convenience: Issue:https://bugs.openjdk.java.net/browse/JDK-8038163 Patch:http://cr.openjdk.java.net/~bpb/8038163/ My only question would be whether the changed line 291 should instead be this: return (major 1 || (major == 1 minor = 2)) ? JNI_TRUE : JNI_FALSE; Thanks, Brian
Re: JDK 9 RFR of 8038163: Build failure on Mac OS 10.9.2 (Mavericks) due to warning treated as error
On Mar 21, 2014, at 12:44 PM, Staffan Larsen staffan.lar...@oracle.com wrote: Looks good to me. Thanks. The version with JNI_TRUE/JNI_FALSE is more correct, but I’m fine with either one. Updated to this form: http://cr.openjdk.java.net/~bpb/8038163/webrev.01/ (note: this review request should have been sent to serviceability-dev). I had no clue to which comp/sub-comp it belongs. What test would I run to verify this change (other than that if fixes the build)? Brian
Re: JDK 9 RFR of 8038163: Build failure on Mac OS 10.9.2 (Mavericks) due to warning treated as error
On 21/03/2014 19:50, Brian Burkhalter wrote: On Mar 21, 2014, at 12:44 PM, Staffan Larsenstaffan.lar...@oracle.com wrote: Looks good to me. Thanks. The version with JNI_TRUE/JNI_FALSE is more correct, but I’m fine with either one. Updated to this form: http://cr.openjdk.java.net/~bpb/8038163/webrev.01/ (note: this review request should have been sent to serviceability-dev). I had no clue to which comp/sub-comp it belongs. What test would I run to verify this change (other than that if fixes the build)? Looks good to me too. The tests to run are the Java Debug Interface (JDI) tests, they are in the jdk_jdi jtreg group. -Alan.
Re: RFR: 8034944: (process) Improve subprocess handling on Solaris
On 21/03/2014 17:43, Rob McKenna wrote: : In a nutshell a new process reaper thread was spawned for every Process created by the JDK. This fix runs these reaper threads in a thread pool to save on thread creation when creating a lot of new processes. http://cr.openjdk.java.net/~robm/8034944/webrev.01/ Thanks Rob, it's good to get this change into the Solaris implementation. I looked through Martin's original changes in JDK-6944584 and the side-port as you've called it looks good. -Alan.
Review request for 8038177 Eliminate unnecessary dependency to sun.security.action
This is the second patch to eliminate the dependencies to sun.security.action: https://bugs.openjdk.java.net/browse/JDK-8038177 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8038177/webrev.00/ Mandy
Re: AWT Dev [9] Review request: new macro for conversion to jboolean
What if we just abandon adding an IS_TRUE / TO_JBOOLEAN macro for now.
I would like to address adding (jboolean) casts to jni.h which I suspect would
help with the static analysis issues encountered.
For now just use the explicit ? JNI_TRUE : JNI_FALSE whenever a jboolean value
is needed and we can refactor the naming of the macro, if any is required,
later.
Mike
On Mar 21 2014, at 08:10 , roger riggs roger.ri...@oracle.com wrote:
Hi Sergey,
I didn't see any examples of the use case in this thread. The proposed name
seems bulky and clumsy.
In the snippet provided, it did not seem necessary to force the cast to
jboolean.
Both JNI_TRUE and JNI_FALSE are untyped constants.
The macro would just as useful (if I understand the cases) without the cast.
How useful is a simple definition as:
#define IS_TRUE(obj) ((obj) ? JNI_TRUE : JNI_FALSE)
then it would look ok to see these in sources:
return IS_TRUE(obj);
if (IS_TRUE(obj)) {}
jboolean ret = IS_TRUE(obj);
The general purpose usage matches the general C conventions for
true and false and match the JNI semantics.
Roger
On 3/19/2014 7:43 PM, Sergey Bylokhov wrote:
Thanks.
So if nobody objects, the final version will be:
#define IS_NULL(obj) ((obj) == NULL)
#define JNU_IsNull(env,obj) ((obj) == NULL)
+#define TO_JBOOLEAN(obj) (jboolean) ((obj) ? JNI_TRUE : JNI_FALSE)
On 3/20/14 12:07 AM, roger riggs wrote:
Hi,
Well... When JNU_RETURN was added, there was a long discussion about
NOT using JNU unless the JNI environment was an argument to the macro.
So, the simple name is preferred.
Roger
On 3/19/2014 4:08 PM, Phil Race wrote:
PS .. so maybe whilst you are touching this file you could do
#define JNU_CHECK_NULL CHECK_NULL
#define JNU_CHECK_NULL_RETURN CHECK_NULL_RETURN
so we could migrate to these (clearer) ones
-phil.
On 3/19/2014 1:05 PM, Phil Race wrote:
I think having it start with JNU_ is a good suggestion.
I've been wondering over the last week or so if it would not have been
better to have CHECK_NULL called JNU_CHECK_NULL to reduce collision
chances
and to make it clearer where it comes from ..
-phil.
On 3/19/2014 12:49 PM, Mike Duigou wrote:
Definitely a useful macro.
I too would prefer a name like TO_JBOOLEAN since it reveals the result
type. Also all uppercase to identify it as a macro. If we are paranoid
and want to reduce the chance of a name collision then JNU_TO_JBOOLEAN
perhaps.
I would also define the macro as:
#define JNU_TO_JBOOLEAN(obj) (jboolean) ((obj) ? JNI_TRUE : JNI_FALSE)
so that the type of the result is explicit. Unfortunately jni.h doesn't
define JNI_TRUE or false with a cast to jboolean as they probably should.
Mike
On Mar 19 2014, at 11:36 , Sergey Bylokhov sergey.bylok...@oracle.com
wrote:
Thanks Anthony!
Can somebody from the core-libs team take a look?
On 3/17/14 10:31 PM, Anthony Petrov wrote:
Personally, I'd call it to_jboolean(obj), but IS_TRUE(obj) sounds good
to me too. Either way, I'm fine with the fix.
--
best regards,
Anthony
On 3/17/2014 7:01 PM, Sergey Bylokhov wrote:
Hello.
This review request is for the new macro, which simplify conversion to
jboolean. It will be useful for fixing parfait warnings.
We have a lot of places, where we cast some type to jboolean:
BOOL = retVal;
return (jboolean) retVal;
WARNING: Expecting value of JNI primitive type jboolean: mismatched
value retVal with size 32 bits, retVal used for conversion to int8 in
return
+++ b/src/share/native/common/jni_util.hMon Mar 17 18:28:48 2014
+0400
@@ -277,6 +277,7 @@
#define IS_NULL(obj) ((obj) == NULL)
#define JNU_IsNull(env,obj) ((obj) == NULL)
+#define IS_TRUE(obj) ((obj) ? JNI_TRUE : JNI_FALSE)
I am not sure about the name, probably someone have a better
suggestion?
The fix is for jdk9/dev.
--
Best regards, Sergey.
--
Best regards, Sergey.
Re: RFR (L) 8037210: Get rid of char-based descriptions 'J' of basic types
John, thanks for the feedback.
Updated webrev:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.02
Also moved LambdaForm.testShortenSignature() into a stand-alone unit test.
Best regards,
Vladimir Ivanov
On 3/21/14 10:54 PM, John Rose wrote:
On Mar 21, 2014, at 8:49 AM, Vladimir Ivanov
vladimir.x.iva...@oracle.com mailto:vladimir.x.iva...@oracle.com wrote:
Thanks for the feedback.
What do you think about the following:
http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/
That looks nice. Strong typing; who woulda' thunk it. :-)
The uses of .ordinal() are the extra cost relative to using just small
integers. They seem totally reasonable in the code.
I suggest either wrapping ordinal as something like id or else
changing temp names like id to ord, to reinforce the use of a common
name.
Don't make the enum class public. And especially don't make the mutable
arrays public:
+public static final BasicType[] ALL_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE,
J_TYPE, F_TYPE, D_TYPE };
— John
P.S. That would only be safe if we had (what we don't yet) a notion of
frozen arrays like:
+public static final BasicType final[] ALL_TYPES = { L_TYPE,
I_TYPE, J_TYPE, F_TYPE, D_TYPE, V_TYPE };
+public static final BasicType final[] ARG_TYPES = { L_TYPE,
I_TYPE, J_TYPE, F_TYPE, D_TYPE };
___
mlvm-dev mailing list
mlvm-...@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: AWT Dev [9] Review request: new macro for conversion to jboolean
On 3/22/14 1:38 AM, Mike Duigou wrote:
What if we just abandon adding an IS_TRUE / TO_JBOOLEAN macro for now.
I would like to address adding (jboolean) casts to jni.h which I suspect would
help with the static analysis issues encountered.
The problem, which I try to solve occurs, when we try to cast some
things like BOOL(int32) to jboolean(int8) and we have a chance to invert
result from true to false.
I am not sure, that I understand how new (jboolean) cast in jni.h solve
the problem? Parfait does not warn about assignments from JNI_TRUE to
jboolean, so I think it smart enough
For now just use the explicit ? JNI_TRUE : JNI_FALSE whenever a jboolean value
is needed and we can refactor the naming of the macro, if any is required,
later.
Sure, but I don't want to copy this across the files, and want to use
macro at least. The question is where to place it.
Mike
On Mar 21 2014, at 08:10 , roger riggs roger.ri...@oracle.com wrote:
Hi Sergey,
I didn't see any examples of the use case in this thread. The proposed name
seems bulky and clumsy.
In the snippet provided, it did not seem necessary to force the cast to
jboolean.
Both JNI_TRUE and JNI_FALSE are untyped constants.
The macro would just as useful (if I understand the cases) without the cast.
How useful is a simple definition as:
#define IS_TRUE(obj) ((obj) ? JNI_TRUE : JNI_FALSE)
then it would look ok to see these in sources:
return IS_TRUE(obj);
if (IS_TRUE(obj)) {}
jboolean ret = IS_TRUE(obj);
The general purpose usage matches the general C conventions for
true and false and match the JNI semantics.
Roger
On 3/19/2014 7:43 PM, Sergey Bylokhov wrote:
Thanks.
So if nobody objects, the final version will be:
#define IS_NULL(obj) ((obj) == NULL)
#define JNU_IsNull(env,obj) ((obj) == NULL)
+#define TO_JBOOLEAN(obj) (jboolean) ((obj) ? JNI_TRUE : JNI_FALSE)
On 3/20/14 12:07 AM, roger riggs wrote:
Hi,
Well... When JNU_RETURN was added, there was a long discussion about
NOT using JNU unless the JNI environment was an argument to the macro.
So, the simple name is preferred.
Roger
On 3/19/2014 4:08 PM, Phil Race wrote:
PS .. so maybe whilst you are touching this file you could do
#define JNU_CHECK_NULL CHECK_NULL
#define JNU_CHECK_NULL_RETURN CHECK_NULL_RETURN
so we could migrate to these (clearer) ones
-phil.
On 3/19/2014 1:05 PM, Phil Race wrote:
I think having it start with JNU_ is a good suggestion.
I've been wondering over the last week or so if it would not have been
better to have CHECK_NULL called JNU_CHECK_NULL to reduce collision chances
and to make it clearer where it comes from ..
-phil.
On 3/19/2014 12:49 PM, Mike Duigou wrote:
Definitely a useful macro.
I too would prefer a name like TO_JBOOLEAN since it reveals the result type.
Also all uppercase to identify it as a macro. If we are paranoid and want to
reduce the chance of a name collision then JNU_TO_JBOOLEAN perhaps.
I would also define the macro as:
#define JNU_TO_JBOOLEAN(obj) (jboolean) ((obj) ? JNI_TRUE : JNI_FALSE)
so that the type of the result is explicit. Unfortunately jni.h doesn't define
JNI_TRUE or false with a cast to jboolean as they probably should.
Mike
On Mar 19 2014, at 11:36 , Sergey Bylokhov sergey.bylok...@oracle.com wrote:
Thanks Anthony!
Can somebody from the core-libs team take a look?
On 3/17/14 10:31 PM, Anthony Petrov wrote:
Personally, I'd call it to_jboolean(obj), but IS_TRUE(obj) sounds good to me
too. Either way, I'm fine with the fix.
--
best regards,
Anthony
On 3/17/2014 7:01 PM, Sergey Bylokhov wrote:
Hello.
This review request is for the new macro, which simplify conversion to
jboolean. It will be useful for fixing parfait warnings.
We have a lot of places, where we cast some type to jboolean:
BOOL = retVal;
return (jboolean) retVal;
WARNING: Expecting value of JNI primitive type jboolean: mismatched
value retVal with size 32 bits, retVal used for conversion to int8 in return
+++ b/src/share/native/common/jni_util.hMon Mar 17 18:28:48 2014 +0400
@@ -277,6 +277,7 @@
#define IS_NULL(obj) ((obj) == NULL)
#define JNU_IsNull(env,obj) ((obj) == NULL)
+#define IS_TRUE(obj) ((obj) ? JNI_TRUE : JNI_FALSE)
I am not sure about the name, probably someone have a better suggestion?
The fix is for jdk9/dev.
--
Best regards, Sergey.
--
Best regards, Sergey.
--
Best regards, Sergey.
Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal
On Mar 20, 2014, at 12:49 AM, Aleksey Shipilev aleksey.shipi...@oracle.com wrote: On 03/20/2014 11:06 AM, Peter Levart wrote: I was thinking about last night, for question: Why is this double-checked non-volatile-then-volatile trick not any faster than pure volatile variant even on ARM platform where volatile read should have some penalty compared to normal read?, might be in the fact that Raspberry Pi is a single-core/single-thread machine. Would anyone with JVM JIT compiler expertise care to share some insight? I suspect that on such platform, the compiler optimizes volatile accesses so that they are performed without otherwise necessary memory fences... Yes, at least C2 is known to not emit memory fences on uniprocessor machines. You need to have a multicore ARM. If you are still interested, contact me privately and I can arrange the access to my personal quad-core Cortex-A9. This is all very interesting but I would like to have closure so to speak on the patches that I posted. I think the questions still unresolved are: 1) Is a change of toString() necessary? 2) If #1 is “yes” then which version of the patch? (I think the most recent) 3) Are the other changes OK which are unrelated to toString()? Thanks, Brian
Re: StringBuilder version of java.util.regex.Matcher.append*
let's add the StringBuilder method(s), if you can provide an updated version, I can run the rest (since it's to add new api, there is an internal CCC process need to go through). -Sherman On 3/21/14 5:18 PM, Jeremy Manson wrote: So, this is all a little opaque to me. How do we make the go/no-go decision on something like this? Everyone who has chimed in seems to think it is a good idea. Jeremy On Thu, Mar 20, 2014 at 10:38 AM, Jeremy Manson jeremyman...@google.com mailto:jeremyman...@google.com wrote: Sherman, If you had released it then (which you wouldn't have been able to do, because you would have to wait another two years for Java 7), you would have found that it improved performance even with C2. It is only post-escape-analysis that the performance in C2 equalized. Anyway, I think adding the StringBuilder variant and deferring / dealing with the Appendable differently is the right approach, FWIW. Jeremy On Thu, Mar 20, 2014 at 10:25 AM, Xueming Shen xueming.s...@oracle.com mailto:xueming.s...@oracle.com wrote: 2009? I do have something similar back to 2009 :-) http://cr.openjdk.java.net/~sherman/regex_replace/webrev/ http://cr.openjdk.java.net/%7Esherman/regex_replace/webrev/ Then the ball was dropped around the discussion of whether or not the IOE should be thrown. But if we are going to/have to have explicit StringBuilder/Buffer pair anyway, then we can keep the Appendable version as private for now and deal with the StringBuilder and Appendable as two separate issues. -Sherman On 03/20/2014 09:52 AM, Jeremy Manson wrote: That's definitely an improvement - I think that when I wrote this (circa 2009), I didn't think about Appendable. I take it my argument convinced someone? :) Jeremy On Thu, Mar 20, 2014 at 1:32 AM, Peter Levartpeter.lev...@gmail.com mailto:peter.lev...@gmail.comwrote: On 03/19/2014 06:51 PM, Jeremy Manson wrote: I'm told that the diff didn't make it. I've put it in a Google drive folder... https://drive.google.com/file/d/0B_GaXa6O4K5LY3Y0aHpranM3aEU/ edit?usp=sharing Jeremy Hi Jeremy, Your factoring-out of expandReplacement() method exposed an opportunity to further optimize the code. Instead of creating intermediate StringBuilder instance for each expandReplacement() call, this method could append directly to resulting StringBuffer/StringBuilder, like in the following: http://cr.openjdk.java.net/~plevart/jdk9-dev/MatcherWithStringBuilder/ http://cr.openjdk.java.net/%7Eplevart/jdk9-dev/MatcherWithStringBuilder/ webrev.01/ Regards, Peter On Wed, Mar 19, 2014 at 10:41 AM, Jeremy Mansonjeremyman...@google.com mailto:jeremyman...@google.com wrote: Hi folks, We've had this internally for a while, and I keep meaning to bring it up here. The Matcher class has a few public methods that take StringBuffers, and we've found it useful to add similar versions that take StringBuilders. It has two benefits: - Users don't have to convert from one to the other when they want to use the method in question. The symmetry is nice. - The StringBuilder variants are faster (if lock optimizations don't kick in, which happens in the interpreter and the client compiler). For interpreted / client-compiled code, we saw something like a 25% speedup on String.replaceAll(), which calls into this code. Any interest? Diff attached. Jeremy
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
Good catch Sherman. Vinnie - what's your recommendation for this LDAP change having both encode/decode uses the platform default charset (rather than retaining the old interop issue)? It's an incompatible change and I'll file a CCC to track this. Mandy On 3/21/2014 2:23 AM, Vincent Ryan wrote: You’re right but we’ve never received a report of any charset interop. issues. Probably such a scenario has never been encountered by customers. On 21 Mar 2014, at 05:54, Xueming Shen xueming.s...@oracle.com wrote: Obj.java:#482 It appears sun.misc.BASE64Decoder.decodeBuffer(String) uses String's deprecated String.getBytes(int srcBegin, int srcEnd, byte[] dst, int dstBegin). The proposed change now uses the jvm's default charset. It might trigger incompatible behavior if the default charset is not an ASCII compatible charset. But if the Java object in LDAP was encoded with the platform default charset (as the new comment suggested), the old implementation actually did not work on platform that the default encoding is not ASCII compatible, such as the IBM ebcdic. -Sherman On 3/20/14 3:48 PM, Mandy Chung wrote: On 3/19/14 12:28 PM, Xueming Shen wrote: On 03/19/2014 11:37 AM, Mandy Chung wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy The sun.misc.BASE64En/Decoder is MIME type, so it outputs the \r\n per 76 characters during encoding, and ignores/skip \r or \n when decoding. The new Base64.getEncoder/Decoder() returns the basic Base64 coder, which it never inserts line separator when output, and throws exception for any non-base64- alphabet character, including \r and \n. The only disadvantage/incompatibility (j.u.Base64.getMimeDecoer() vs sun.misc.BASE64Decoder) of switching to j.u.Base64 MIME type en/decoder is that the Base64 Mime decoder ignores/skips any non-base64-alphabet (including \r and \n), while sun.misc.BASE64Decoder appears to simply use the init value -1 for any non-base64-alphabet character for decoding. I'm not familiar with the use scenario of ldap's Obj class, so I'm not sure if it matters (if it ever outputs/inputs 76 character data, or even it does,if the difference matters). Btw, except getMimeEncoder(int ...) all other Base64.getXXXEn/Decoder() returns singleton, so the de/encoder cache might not be necessary. Thanks Sherman. Vinnie confirms that it should retain the current behavior as there could be long-lived Java object in LDAP encoded with JDK 8 for example and then retrieved with JDK 9. Here is the updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.01/ Thanks Mandy
