Re: RFR (L) 8037210: Get rid of char-based descriptions 'J' of basic types

[Christian Thalinger]

On Apr 3, 2014, at 9:44 PM, John Rose  wrote:

> On Apr 3, 2014, at 6:33 PM, Christian Thalinger 
>  wrote:
> 
>> Of course they are popular because these are the type names.  There is no 
>> type L; it’s an object.  I don’t understand why we have to use different 
>> names just because they are used in other namespaces.  This is not a C 
>> define.
> 
> They stand for JVM signatures as well as basic types.  The letters are 
> signature letters.  Can we move on from this?

Sure.  Push it.

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

Re: RFR: 8000975: (process) Merge UNIXProcess.java.bsd & UNIXProcess.java.linux (& .solaris & .aix)

[roger riggs]

Hi Peter,

I ran into one test problem when running this through its paces.

The test: test/java/lang/ProcessBuilder/SecurityManagerClinit.java
throws a package access exception while creating a lambda
due to the wacky security manager and forbidding of access to java.util.
I hacked the test to remove the limitation on java.util.

This looks good to me but a more experienced Reviewer should look at it.

Thanks, Roger



Right,

Here it is:

http://cr.openjdk.java.net/~plevart/jdk9-dev/UNIXProcess/webrev.06/



Stack dump from test:
java.lang.ExceptionInInitializerError
at 
java.lang.invoke.DirectMethodHandle.makePreparedLambdaForm(DirectMethodHandle.java:256)
at 
java.lang.invoke.DirectMethodHandle.preparedLambdaForm(DirectMethodHandle.java:221)
at 
java.lang.invoke.DirectMethodHandle.preparedLambdaForm(DirectMethodHandle.java:210)
at 
java.lang.invoke.DirectMethodHandle.make(DirectMethodHandle.java:82)
at 
java.lang.invoke.MethodHandles$Lookup.getDirectMethodCommon(MethodHandles.java:1638)
at 
java.lang.invoke.MethodHandles$Lookup.getDirectMethodNoSecurityManager(MethodHandles.java:1602)
at 
java.lang.invoke.MethodHandles$Lookup.getDirectMethodForConstant(MethodHandles.java:1778)
at 
java.lang.invoke.MethodHandles$Lookup.linkMethodHandleConstant(MethodHandles.java:1727)
at 
java.lang.invoke.MethodHandleNatives.linkMethodHandleConstant(MethodHandleNatives.java:442)

at java.lang.UNIXProcess$Platform.get(UNIXProcess.java:147)
at java.lang.UNIXProcess.(UNIXProcess.java:160)
at java.lang.ProcessImpl.start(ProcessImpl.java:130)
at java.lang.ProcessBuilder.start(ProcessBuilder.java:1023)
at java.lang.Runtime.exec(Runtime.java:620)
at java.lang.Runtime.exec(Runtime.java:485)
at SecurityManagerClinit.main(SecurityManagerClinit.java:70)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)

at java.lang.reflect.Method.invoke(Method.java:484)
at 
com.sun.javatest.regtest.MainWrapper$MainThread.run(MainWrapper.java:94)

at java.lang.Thread.run(Thread.java:744)
Caused by: java.security.AccessControlException: access denied 
("java.lang.RuntimePermission" "accessClassInPackage.java.util")
at 
java.security.AccessControlContext.checkPermission(AccessControlContext.java:457)
at 
java.security.AccessController.checkPermission(AccessController.java:884)
at 
java.lang.SecurityManager.checkPermission(SecurityManager.java:541)
at 
java.lang.SecurityManager.checkPackageAccess(SecurityManager.java:1481)

at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:305)
at java.lang.ClassLoader.loadClass(ClassLoader.java:359)
at 
sun.invoke.util.BytecodeDescriptor.parseSig(BytecodeDescriptor.java:83)
at 
sun.invoke.util.BytecodeDescriptor.parseMethod(BytecodeDescriptor.java:54)
at 
sun.invoke.util.BytecodeDescriptor.parseMethod(BytecodeDescriptor.java:41)
at 
java.lang.invoke.MethodType.fromMethodDescriptorString(MethodType.java:911)

at java.lang.invoke.MemberName.getMethodType(MemberName.java:144)
at 
java.lang.invoke.LambdaForm.computeInitialPreparedForms(LambdaForm.java:477)

at java.lang.invoke.LambdaForm.(LambdaForm.java:1641)

Re: StringBuilder version of java.util.regex.Matcher.append*

[Xueming Shen]

On 4/3/14 4:43 PM, Jeremy Manson wrote:

Good catch, thanks.

I think we should probably just go with the (equivalent to the) 
StringBuffer variant.  I'm pretty loathe to modify the StringBuilder 
directly if we are going to back that change out.


Do you want me to generate a new patch?


I can/will send out an updated webrev before push.

-Sherman



Jeremy


On Thu, Apr 3, 2014 at 2:27 PM, Xueming Shen > wrote:


On 03/25/2014 02:07 PM, Jeremy Manson wrote:

Okay.  Thanks, Sherman.  Here's an updated version.

I've diverged a bit from Peter's version.  In this version,
appendExpandedReplacement takes a StringBuilder.  The
implications is that In the StringBuilder case, it saves creating
a new StringBuilder object.  In the StringBuffer case, it creates
a new StringBuilder, but it doesn't have to acquire and release
all of those locks.


Hi Jeremy,

It appears the "optimized" StringBuilder version will cause
following test case failure,
in which the "xyz" will be copied into the result buffer, even
when the replacement
string triggers a IAE.

// Check nothing has been appended into the output buffer if
// the replacement string triggers IllegalArgumentException.
Pattern p = Pattern.compile("(abc)");
Matcher m = p.matcher("abcd");
StringBuilder result = new StringBuilder();
try {
m.appendReplacement(result, ("xyz$g"));
} catch (IllegalArgumentException iae) {
if (result.length() != 0)
System.err.println(" FAILED");

}

We may have to either catch the IAE and reset the sb, or create
a new sb, as the StringBuffer does.

-Sherman





I also noticed a redundant cast to (int), which I removed.

Jeremy


On Fri, Mar 21, 2014 at 7:34 PM, Xueming Shen
mailto:xueming.s...@oracle.com>> wrote:

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
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
mailto:xueming.s...@oracle.com>> wrote:

2009? I do have something similar back to 2009 :-)

http://cr.openjdk.java.net/~sherman/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
Levartmailto:peter.lev...@gmail.com>>wrote:

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 op

Re: JDK-8036003: Add variable not to separate debug information.

[Dmitry Samersoff]

Magnus,

Not, we are not doing it now.

But we should consider doing it in a future and therefore keep it in
mind when designing option to choose debug symbol mode.

-Dmitry

On 2014-03-24 15:18, Magnus Ihse Bursie wrote:
> On 2014-03-21 10:36, Dmitry Samersoff wrote:
>>
>> (c) Compression mode:
>>
>> 1. none
>> 2. per-section compression, SHF_GNU_COMPRESSED [1]
>> 3. zip entire file
>>
> 
> Is 2 something we're doing? I couldn't find any references to it in the
> code. Or is it something we're planning to do?
> 
> /Magnus


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.

Re: String.indexOf optimization

[Remi Forax]

On 04/04/2014 05:18 PM, Alan Bateman wrote:

On 04/04/2014 15:49, Zoltan Sziladi wrote:

Hi,

I am new to this mailing list so please forgive me if this has been
discussed before.

I was looking at the implementation of String.indexOf and I see that
it uses the O(n^2) naive implementation. I have been trying to find
out why it does not use some kind of a modern, sophisticated O(n)
algorithm but I have no clear answer as of now.

My guess is that the average case should be quite good for this
algorithm because in practice the partial matches are actually quite
rare, so it should work well... usually. Also, I saw that this code
was last touched about 6 years ago, so maybe it was just left like
this?
My concern is actually the worst case scenario. If we compared two
long strings with lots of partial matches, then this would perform
quite poorly. Wouldn't it be worth having an O(n) implementation here
then? Modern O(n) pattern matching algorithms don't use much extra
space either.
The Collections.sort method also uses an algorithm that prepares for
worst case. Maybe a highly optimized quicksort could outperform the
current mergesort implementation but I suppose one of the design
principles behind that was also to prepare for the worst case. (Even
though a nicely implemented quicksort has an expected average case
runtime of O(nlogn) regardless of the input).

If anyone knows why it is implemented this way or if it were possible
to change the implementation, I'd be happy to hear your opinion.
Thanks!


Here's a previous thread where someone else asked about String.indexOf

http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/018062.html 



If you are interested in this topic and can do better then go ahead, 
I'm sure there would be a lot of people here would be interested in 
space and time numbers.


-Alan.


You may also want to test against the couple Pattern + Matcher,
which use BoyerMoore (at least the last time i have read the source)

Rémi

Re: [9] Review request for 8039172: Tidy warnings cleanup for java.net, java.math, java.time, java.rmi

[alexander stepanov]

Thanks; the line in net-properties.html was splitted.

Regards,
Alexander

On 04.04.2014 19:25, Alan Bateman wrote:

On 04/04/2014 12:53, alexander stepanov wrote:

Hello Lance,

Thank you for the note; the summaries were removed, please see the 
updated webrev:

http://cr.openjdk.java.net/~yan/8039172/webrev.01/
I looked through the updated webrev and it looks okay to me. One 
suggestion for net-properties.html is to split line 38 as it's very 
long compared to all the rest.


-Alan.

Re: RFR: 8039256 Add sun/jvmstat/monitor/MonitoredVm/CR6672135.java to ProblemList.txt

[Alan Bateman]

On 04/04/2014 12:33, Staffan Larsen wrote:

Please review the change below to add a test to ProblemList.txt. For details, 
see https://bugs.openjdk.java.net/browse/JDK-8033104

Thanks,
/Staffan



diff --git a/test/ProblemList.txt b/test/ProblemList.txt
--- a/test/ProblemList.txt
+++ b/test/ProblemList.txt
@@ -273,4 +273,7 @@
  # 8031482
  sun/tools/jcmd/TestJcmdSanity.javawindows-all

+# 8033104
+sun/jvmstat/monitor/MonitoredVm/CR6672135.java generic-all
+
  
Excluding this test until the issue running with java.io.tmpdir set is 
resolved seem okay to me.


-Alan

Re: [9] Review request for 8039172: Tidy warnings cleanup for java.net, java.math, java.time, java.rmi

[Alan Bateman]

On 04/04/2014 12:53, alexander stepanov wrote:

Hello Lance,

Thank you for the note; the summaries were removed, please see the 
updated webrev:

http://cr.openjdk.java.net/~yan/8039172/webrev.01/
I looked through the updated webrev and it looks okay to me. One 
suggestion for net-properties.html is to split line 38 as it's very long 
compared to all the rest.


-Alan.

Re: RFR: 8038306: (tz) Support tzdata2014b

[Aleksej Efimov]

Masayoshi,
Thank you for the review. The "TST" is already used by "Asia/Taipei" in 
"zh_TW".  I'll leave the naming as it is, because I suppose that we 
probably should see an official short name abbreviation for this time 
zone in near future.


-Aleksej
On 04/04/2014 06:25 PM, Masayoshi Okutsu wrote:
Another option would be "Troll Station Time" and "TST". But your 
invention is fine with me.


Thanks,
Masayoshi

On 4/4/2014 9:19 PM, Aleksej Efimov wrote:

Masayoshi,

The new webrev with proposed generic names for Antarctica/Troll can 
be found here: http://cr.openjdk.java.net/~aefimov/8038306/9/webrev.01


Thank you,
Aleksej

On 04/03/2014 06:29 PM, Aleksej Efimov wrote:

Masayoshi,
How about "Troll Time" and "ATT" for generic long and short names 
across all locales? The "TT" is used as generic name for 
"Asia/Taipei" in "zh_TW" locale, because of that I propose "ATT" (A 
- for Antractica) - it's not used anywhere.


Thanks,
Aleksej

On 04/03/2014 06:21 PM, Masayoshi Okutsu wrote:

Hi Aleksej,

Sorry, but I forgot about the generic names. "Coordinated Universal 
Time" and "UTC"shouldn't be the generic names. You will need to 
"invent" the names, something like "Troll Time".


Thanks,
Masayoshi

On 4/2/2014 7:55 PM, Aleksej Efimov wrote:

Hello,

Can I have a review for the latest (2014b) TZ data integration to 
JDK9. The webrev can be located here [1].


The following set of tests were executed without failures:
test/sun/util/calendar test/java/util/Calendar 
test/sun/util/resources/TimeZone test/sun/util/calendar 
test/java/util/TimeZone test/java/time test/java/util/Formatter 
test/closed/java/util/Calendar\ test/closed/java/util/TimeZone


Thank you,
Aleksej

[1] Webrev: http://cr.openjdk.java.net/~aefimov/8038306/9/webrev.00/
[2] BUG: https://bugs.openjdk.java.net/browse/JDK-8038306

Re: String.indexOf optimization

[Alan Bateman]

On 04/04/2014 15:49, Zoltan Sziladi wrote:

Hi,

I am new to this mailing list so please forgive me if this has been
discussed before.

I was looking at the implementation of String.indexOf and I see that
it uses the O(n^2) naive implementation. I have been trying to find
out why it does not use some kind of a modern, sophisticated O(n)
algorithm but I have no clear answer as of now.

My guess is that the average case should be quite good for this
algorithm because in practice the partial matches are actually quite
rare, so it should work well... usually. Also, I saw that this code
was last touched about 6 years ago, so maybe it was just left like
this?
My concern is actually the worst case scenario. If we compared two
long strings with lots of partial matches, then this would perform
quite poorly. Wouldn't it be worth having an O(n) implementation here
then? Modern O(n) pattern matching algorithms don't use much extra
space either.
The Collections.sort method also uses an algorithm that prepares for
worst case. Maybe a highly optimized quicksort could outperform the
current mergesort implementation but I suppose one of the design
principles behind that was also to prepare for the worst case. (Even
though a nicely implemented quicksort has an expected average case
runtime of O(nlogn) regardless of the input).

If anyone knows why it is implemented this way or if it were possible
to change the implementation, I'd be happy to hear your opinion.
Thanks!


Here's a previous thread where someone else asked about String.indexOf

http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/018062.html

If you are interested in this topic and can do better then go ahead, I'm 
sure there would be a lot of people here would be interested in space 
and time numbers.


-Alan.

Re: JDK-8036003: Add variable not to separate debug information.

[Yasumasa Suenaga]

Hi all,


This should fix it:
http://pkgs.fedoraproject.org/cgit/java-1.8.0-openjdk.git/commit/?id=1734315551d634b7467f28788d90595b467ea5eb


I updated OpenJDK8 to java-1.8.0-openjdk-1.8.0.0-0.34.b132.fc20 .
However, debuginfo files are not contained ELF sections for debugging.
(I checked libjvm.so.debug and libnio.so.debug with "readelf -S")

According to SPEC file of OpenJDK8, following options are passed to "make":
-
make \
SCTP_WERROR= \
DEBUG_BINARIES=true \
FULL_DEBUG_SYMBOLS=0 \
STRIP_POLICY=none \
ALT_OBJCOPY=/does_not_exist \
LOG=trace \
all
-

I ran "grep" with DEBUG_BINARIES in jdk makefiles, however I could not 
find it.

At least, DEBUG_BINARIES does not affect to jdk sources, and also does not
affect to hotspot sources.


I've succeeded to make binaries which are contained debuginfo as following:

http://mail.openjdk.java.net/pipermail/build-dev/2014-March/012037.html
$ make images STRIP_POLICY=no_strip POST_STRIP_CMD=""


I guess that we should run "make" above options to avoid this issue in 
currently.



Thanks,

Yasumasa


On 03/04/2014 04:01 AM, Omair Majid wrote:

* Andrew Haley  [2014-03-03 04:43]:

On 02/28/2014 09:18 AM, Yasumasa Suenaga wrote:

For example, OpenJDK8 in Fedora20 ships libjvm.so and libjvm.debuginfo .
libjvm.debuginfo is generated in OpenJDK's makefiles, however it does not
contain debug information. Actual debug information is shipped by OpenJDK
debuginfo package.

That's a bug in Fedora's build.  We should fix it.

This should fix it:
http://pkgs.fedoraproject.org/cgit/java-1.8.0-openjdk.git/commit/?id=1734315551d634b7467f28788d90595b467ea5eb

Thanks,
Omair

String.indexOf optimization

[Zoltan Sziladi]

Hi,

I am new to this mailing list so please forgive me if this has been
discussed before.

I was looking at the implementation of String.indexOf and I see that
it uses the O(n^2) naive implementation. I have been trying to find
out why it does not use some kind of a modern, sophisticated O(n)
algorithm but I have no clear answer as of now.

My guess is that the average case should be quite good for this
algorithm because in practice the partial matches are actually quite
rare, so it should work well... usually. Also, I saw that this code
was last touched about 6 years ago, so maybe it was just left like
this?
My concern is actually the worst case scenario. If we compared two
long strings with lots of partial matches, then this would perform
quite poorly. Wouldn't it be worth having an O(n) implementation here
then? Modern O(n) pattern matching algorithms don't use much extra
space either.
The Collections.sort method also uses an algorithm that prepares for
worst case. Maybe a highly optimized quicksort could outperform the
current mergesort implementation but I suppose one of the design
principles behind that was also to prepare for the worst case. (Even
though a nicely implemented quicksort has an expected average case
runtime of O(nlogn) regardless of the input).

If anyone knows why it is implemented this way or if it were possible
to change the implementation, I'd be happy to hear your opinion.
Thanks!

Regards,
Zoltan

Re: RFR: 8038306: (tz) Support tzdata2014b

[Masayoshi Okutsu]

Another option would be "Troll Station Time" and "TST". But your 
invention is fine with me.


Thanks,
Masayoshi

On 4/4/2014 9:19 PM, Aleksej Efimov wrote:

Masayoshi,

The new webrev with proposed generic names for Antarctica/Troll can be 
found here: http://cr.openjdk.java.net/~aefimov/8038306/9/webrev.01


Thank you,
Aleksej

On 04/03/2014 06:29 PM, Aleksej Efimov wrote:

Masayoshi,
How about "Troll Time" and "ATT" for generic long and short names 
across all locales? The "TT" is used as generic name for 
"Asia/Taipei" in "zh_TW" locale, because of that I propose "ATT" (A - 
for Antractica) - it's not used anywhere.


Thanks,
Aleksej

On 04/03/2014 06:21 PM, Masayoshi Okutsu wrote:

Hi Aleksej,

Sorry, but I forgot about the generic names. "Coordinated Universal 
Time" and "UTC"shouldn't be the generic names. You will need to 
"invent" the names, something like "Troll Time".


Thanks,
Masayoshi

On 4/2/2014 7:55 PM, Aleksej Efimov wrote:

Hello,

Can I have a review for the latest (2014b) TZ data integration to 
JDK9. The webrev can be located here [1].


The following set of tests were executed without failures:
test/sun/util/calendar test/java/util/Calendar 
test/sun/util/resources/TimeZone test/sun/util/calendar 
test/java/util/TimeZone test/java/time test/java/util/Formatter 
test/closed/java/util/Calendar\ test/closed/java/util/TimeZone


Thank you,
Aleksej

[1] Webrev: http://cr.openjdk.java.net/~aefimov/8038306/9/webrev.00/
[2] BUG: https://bugs.openjdk.java.net/browse/JDK-8038306

Re: Improve timezone mapping for AIX platform

[Volker Simonis]

Hi Jonathan,

sorry, but I found a few more issues:

- please use strncpy instead of strcpy in TimeZone_md.c:798 otherwise
somebody could easily crash the VM by specifying a TZ with more than
100 characters.

- you can delete the lines 871-872 because the variables are actually
not used and you can also remove the handling for "timezone == 0"
because that is actually done in getGMTOffsetID() anyway.

- could you please avoid the usage of strtok. It is not intended for
usage in a multithreaded environment (see for example "man strtok" on
AIX). strtok_r is probably overkill, but you could use for example
strchr.

did you check the build on Windows?

Thank you and best regards,
Volker


On Fri, Apr 4, 2014 at 10:18 AM, Jonathan Lu  wrote:
> Hi Volker, Masayoshi,
>
> I made another  patch which fixed the problems mentioned in last mail,
>
> http://cr.openjdk.java.net/~luchsh/JDK-8034220.v3/
>
> Could you pls help to take a look?
>
> Many thanks
> Jonathan
>
>
>
> On Thu, Apr 3, 2014 at 12:34 AM, Jonathan Lu 
> wrote:
>>
>> Hi Volker,
>>
>>
>> On 2014年04月02日 23:07, Volker Simonis wrote:
>>
>> Hi Jonathan,
>>
>> thanks for updating the change. Please find my comments inline:
>>
>> On Tue, Apr 1, 2014 at 4:52 PM, Jonathan Lu 
>> wrote:
>>
>> Hi Volker, Masayoshi,
>>
>> Thanks a lot for your review, here's the updated patch, could you pls take
>> a
>> look?
>>
>> http://cr.openjdk.java.net/~luchsh/JDK-8034220.v2/
>>
>>
>> On Thu, Mar 27, 2014 at 1:48 AM, Volker Simonis 
>> wrote:
>>
>> Hi Jonathan,
>>
>> thanks for doing this change. Please find some comments below:
>>
>> 1. please update the copyright year to 2014 in every file you touch
>>
>> Updated in new patch.
>>
>> 2. in CopyFiles.gmk you can simplify the change by joining the windows
>> and aix cases because on Windows OPENJDK_TARGET_OS is the same like
>> OPENJDK_TARGET_OS_API_DIR. So you can write:
>>
>> ifneq ($(findstring $(OPENJDK_TARGET_OS), windows aix), )
>>
>>   TZMAPPINGS_SRC := $(JDK_TOPDIR)/src/$(OPENJDK_TARGET_OS)/lib
>>
>>   $(LIBDIR)/tzmappings: $(TZMAPPINGS_SRC)/tzmappings
>> $(call install-file)
>>
>>   COPY_FILES += $(LIBDIR)/tzmappings
>>
>> endif
>>
>> I've tried that approach before, but OPENJDK_TARGET_OS_API_DIR is
>> 'solaris'
>> as I observed on my AIX box,
>> solaris/lib is not the directory where tzmappings was stored for AIX.
>> So I'm keeping this change, please fix me if I was wrong about the config.
>>
>> Yes, but my point was to actually use OPENJDK_TARGET_OS for the
>> construction of  TZMAPPINGS_SRC as shown in the code snippet above.
>> OPENJDK_TARGET_OS is "windows" on Windows platforms and "aix" on AIX
>> and that should be just enough here.
>>
>>
>> That's right, let me try that  and also build/test on Windows platform.
>>
>>
>> 3. regarding the changes proposed by Masayoshi: I agree that we should
>> put the AIX timezone mapping code in its own function, but I don't
>> think that getPlatformTimeZoneID() would be the right place. As far as
>> I understand, getPlatformTimeZoneID() is used to get a platform time
>> zone id if the environment variable "TZ" is not set. I don't know a
>> way how this could be done on AIX (@Jonathan: maybe you know?). If
>> there would be a way to get the time zone from some system files or
>> so, then we should put this code into the AIX version of
>> getPlatformTimeZoneID().
>>
>> I guess we may try to use /etc/envrionment file, which is basic setting
>> for
>> all processes,
>> see
>>
>> http://publib.boulder.ibm.com/infocenter/aix/v7r1/index.jsp?topic=%2Fcom.ibm.aix.files%2Fdoc%2Faixfiles%2Fenvironment.htm
>> The implementation of  getPlatformTimeZoneID() has been updated.
>>
>> That's good!
>>
>> But the current AIX code contributed by Jonathan, actually uses the
>> time zone id from the "TZ" environment variable and just maps it to a
>> Java compatible time zone id. So I'd suggest to refactor this code
>> into a function like for example "static const char*
>> mapPlatformToJavaTimzone(const char* tz)" and call that from
>> findJavaTZ_md(). I think that would make the code easier to
>> understand.
>>
>> Agree, and have split the code into a separate static method to do the
>> mapping work.
>>
>> Good. But there's still one error in findJavaTZ_md():
>>
>>  706 #ifdef _AIX
>>  707 javatz = mapPlatformToJavaTimezone(java_home_dir, tz);
>>  708 #endif
>>
>> This should read:
>>
>>  706 #ifdef _AIX
>>  707 javatz = mapPlatformToJavaTimezone(java_home_dir, javatz);
>>  708 #endif
>>
>> because in line 703 you free 'tz' via its alias 'freetz' if 'tz' came
>> from getPlatformTimeZoneID().
>>
>>
>> Right, but with the second approach, there may be a minor memory leak
>> here,
>> as javatz was not freed before being overwritten on AIX. will post another
>> patch on this soon.
>>
>>
>> With the above fixed I'll push this one we get one more review from a
>> reviewer (I'm currently not an official reviewer).
>>
>> Regards,
>> Volker
>>
>>
>> @Masayoshi: what do you think, would y

RE: RFR: 8039256 Add sun/jvmstat/monitor/MonitoredVm/CR6672135.java to ProblemList.txt

[Markus Grönlund]

Looks good.

/Markus

-Original Message-
From: Staffan Larsen 
Sent: den 4 april 2014 13:33
To: serviceability-...@openjdk.java.net serviceability-...@openjdk.java.net; 
core-libs-dev Libs
Subject: RFR: 8039256 Add sun/jvmstat/monitor/MonitoredVm/CR6672135.java to 
ProblemList.txt

Please review the change below to add a test to ProblemList.txt. For details, 
see https://bugs.openjdk.java.net/browse/JDK-8033104

Thanks,
/Staffan



diff --git a/test/ProblemList.txt b/test/ProblemList.txt
--- a/test/ProblemList.txt
+++ b/test/ProblemList.txt
@@ -273,4 +273,7 @@
 # 8031482
 sun/tools/jcmd/TestJcmdSanity.java windows-all

+# 8033104
+sun/jvmstat/monitor/MonitoredVm/CR6672135.java generic-all
+
 

Re: [9] Review request for 8039172: Tidy warnings cleanup for java.net, java.math, java.time, java.rmi

[Lance @ Oracle]

Looks fine


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

On Apr 4, 2014, at 7:53 AM, alexander stepanov 
 wrote:

> Hello Lance,
> 
> Thank you for the note; the summaries were removed, please see the updated 
> webrev:
> http://cr.openjdk.java.net/~yan/8039172/webrev.01/
> 
> Regards,
> Alexander
> 
> On 04.04.2014 15:16, Lance @ Oracle wrote:
>> Looks ok but have one concern/question as to why you added the summary 
>> attribute to the table tag as it has been deprecated and believe it is not 
>> supported in HTML 5.
>> 
>> I would probably not include it
>> 
>> Best
>> Lance
>> 
>> Lance Andersen| 
>> Principal Member of Technical Staff | +1.781.442.2037 
>> Oracle Java Engineering
>> 1 Network Drive 
>> Burlington, MA 01803 
>> lance.ander...@oracle.com 
>> Sent from my iPad
>> 
>> On Apr 4, 2014, at 4:55 AM, alexander stepanov 
>> mailto:alexander.v.stepa...@oracle.com>> 
>> wrote:
>> 
>>> Hello,
>>> 
>>> Could you please review the fix for the following bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8039172
>>> 
>>> Webrev corresponding:
>>> http://cr.openjdk.java.net/~yan/8039172/webrev.00/ 
>>> 
>>> 
>>> Just a minor cleanup of javadoc to avoid tidy warnings; no other code 
>>> affected.
>>> 
>>> Thanks.
>>> 
>>> Regards,
>>> Alexander
> 

Re: RFR: 8038306: (tz) Support tzdata2014b

[Aleksej Efimov]

Masayoshi,

The new webrev with proposed generic names for Antarctica/Troll can be 
found here: http://cr.openjdk.java.net/~aefimov/8038306/9/webrev.01


Thank you,
Aleksej

On 04/03/2014 06:29 PM, Aleksej Efimov wrote:

Masayoshi,
How about "Troll Time" and "ATT" for generic long and short names 
across all locales? The "TT" is used as generic name for "Asia/Taipei" 
in "zh_TW" locale, because of that I propose "ATT" (A - for 
Antractica) - it's not used anywhere.


Thanks,
Aleksej

On 04/03/2014 06:21 PM, Masayoshi Okutsu wrote:

Hi Aleksej,

Sorry, but I forgot about the generic names. "Coordinated Universal 
Time" and "UTC"shouldn't be the generic names. You will need to 
"invent" the names, something like "Troll Time".


Thanks,
Masayoshi

On 4/2/2014 7:55 PM, Aleksej Efimov wrote:

Hello,

Can I have a review for the latest (2014b) TZ data integration to 
JDK9. The webrev can be located here [1].


The following set of tests were executed without failures:
test/sun/util/calendar test/java/util/Calendar 
test/sun/util/resources/TimeZone test/sun/util/calendar 
test/java/util/TimeZone test/java/time test/java/util/Formatter 
test/closed/java/util/Calendar\ test/closed/java/util/TimeZone


Thank you,
Aleksej

[1] Webrev: http://cr.openjdk.java.net/~aefimov/8038306/9/webrev.00/
[2] BUG: https://bugs.openjdk.java.net/browse/JDK-8038306

Re: [9] Review request for 8039172: Tidy warnings cleanup for java.net, java.math, java.time, java.rmi

[alexander stepanov]

Hello Lance,

Thank you for the note; the summaries were removed, please see the 
updated webrev:

http://cr.openjdk.java.net/~yan/8039172/webrev.01/

Regards,
Alexander

On 04.04.2014 15:16, Lance @ Oracle wrote:
Looks ok but have one concern/question as to why you added the summary 
attribute to the table tag as it has been deprecated and believe it is 
not supported in HTML 5.


I would probably not include it

Best
Lance

Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037 


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

On Apr 4, 2014, at 4:55 AM, alexander stepanov 
> wrote:



Hello,

Could you please review the fix for the following bug:
https://bugs.openjdk.java.net/browse/JDK-8039172

Webrev corresponding:
http://cr.openjdk.java.net/~yan/8039172/webrev.00/ 



Just a minor cleanup of javadoc to avoid tidy warnings; no other code 
affected.


Thanks.

Regards,
Alexander

Review of MH/LF patches in the review queue

[Paul Sandoz]

Hi,

Reviews of two patches in the queue.

Paul.


http://cr.openjdk.java.net/~vlivanov/8037209/webrev.02/

Looks good, though I don't claim to understand all the nuances around casts and 
JVM/bytecode correctness. Minor stuff below.

InvokerByteCodeGenerator:
--

Unused method:

static boolean match(MemberName member, MethodHandle fn) {
if (member == null || fn == null)  return false;
return member.equals(fn.internalMemberName());
}


MethodHandleImpl
--

If you are gonna remove the weakly typed wrapper methods for array access, you 
might as well remove USE_WEAKLY_TYPED_ARRAY_ACCESSORS and it's use?

- // Weakly typed wrappers of Object[] accessors:
- static Object  getElementL(Objecta, int i){ return 
getElementL((Object[])a, i); }
- static voidsetElementL(Objecta, int i, Object  x) {
setElementL((Object[]) a, i, x); }
- static Object  getElementL(Object   arrayClass, Object a, int i)  
   { return getElementL((Class) arrayClass, (Object[])a, i); }
- static voidsetElementL(Object   arrayClass, Object a, int i, 
Object x)   {setElementL((Class) arrayClass, (Object[])a, i, x); }
- 

Or otherwise for expediency just leave it in until the array improvements patch 
follows? IMHO better to be consistent either way.


VerifyType
--

Typo in docs:

 * 
 * The primitive type 'void' does not interconvert with any other type,
 * even though it is legal to drop any type from the stack and "return 
void".
 * The stack effects, though are difference between void and any other type,
 * so it is safer to report a non-trivial conversion.

s/difference/different



http://cr.openjdk.java.net/~vlivanov/8038261/webrev.01/

To re-iterate this is a nice improvement over the previous approach.

InvokerByteCodeGenerator
--

For this specialization:

if (defc == ArrayAccessor.class &&
match(member, ArrayAccessor.OBJECT_ARRAY_GETTER)) {
mv.visitInsn(Opcodes.AALOAD);
} else if (defc == ArrayAccessor.class &&
match(member, ArrayAccessor.OBJECT_ARRAY_SETTER)) {
mv.visitInsn(Opcodes.AASTORE);
} else if (member.isMethod()) {

IIUC all stuff on the stack is correctly placed for the substitution of the 
invokestatic instruction to ArrayAccessor.set/getElementL with an aastore/load 
instruction. This makes we wonder if there is a more general approach for 
direct or direct-like MHs to be visited and provide their own snippets of ASM 
producing code.

Is it worthwhile introducing such direct coupling for a specific case, as that 
tends to increase the inter-connective complexity of the code. How much 
performance gain is achieved?


The last two re-assigments are never used and are reassigned again at the top 
of the loop:

// Update cached form name's info in case an intrinsic spanning 
multiple names was encountered.
name = lambdaForm.names[i];
member = name.function.member();
rtype = name.function.methodType().returnType();

RFR: 8039256 Add sun/jvmstat/monitor/MonitoredVm/CR6672135.java to ProblemList.txt

[Staffan Larsen]

Please review the change below to add a test to ProblemList.txt. For details, 
see https://bugs.openjdk.java.net/browse/JDK-8033104

Thanks,
/Staffan



diff --git a/test/ProblemList.txt b/test/ProblemList.txt
--- a/test/ProblemList.txt
+++ b/test/ProblemList.txt
@@ -273,4 +273,7 @@
 # 8031482
 sun/tools/jcmd/TestJcmdSanity.java windows-all

+# 8033104
+sun/jvmstat/monitor/MonitoredVm/CR6672135.java generic-all
+
 

Re: [9] Review request for 8039172: Tidy warnings cleanup for java.net, java.math, java.time, java.rmi

[Lance @ Oracle]

Looks ok but have one concern/question as to why you added the summary 
attribute to the table tag as it has been deprecated and believe it is not 
supported in HTML 5.

I would probably not include it

Best
Lance

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

On Apr 4, 2014, at 4:55 AM, alexander stepanov 
 wrote:

> Hello,
> 
> Could you please review the fix for the following bug:
> https://bugs.openjdk.java.net/browse/JDK-8039172
> 
> Webrev corresponding:
> http://cr.openjdk.java.net/~yan/8039172/webrev.00/
> 
> Just a minor cleanup of javadoc to avoid tidy warnings; no other code 
> affected.
> 
> Thanks.
> 
> Regards,
> Alexander

Re: 8020860: cluster Hashtable/Vector field updates for better transactional memory behaviour

[Paul Sandoz]

On Apr 4, 2014, at 1:42 AM, Mike Duigou  wrote:
>> 
>> I could live with that change in behaviour, but this change completely 
>> breaks the fail-fast semantics of the iterators in some cases! If you don't 
>> update modCount until after the change is complete, the iterator may access 
>> the updated state and not throw CME!.
> 
> For Vector I don't see this. The Iterator accesses to the data structures is 
> always done with the Vector.this lock held. The re-ordering would only be 
> observable to another thread if it is reading the Vector fields without 
> holding the lock. I am not sure we should worry about that case.
> 

Agreed, i don't see how that can happen.


> For Hashtable Iterator there is no synchronization on the owning Hashtable 
> except during the remove() method. It is unclear why the Hashtable iterators 
> were not written in the same way as Vector.

Dunno.


> It seems like there would be massive disruption to adding synchronization to 
> Hashtable's itertors. Are the Hashtable iterators actually fast-fail?

They are fail fast only from within the same thread when the control is 
inverted via iterator (like that for non-synchronized HashMap etc), otherwise 
it is necessary to explicitly synchronize on the iterator, much like that for 
Collections.synchronized* methods, see the implementation:

public Set keySet() {
if (keySet == null)
keySet = Collections.synchronizedSet(new KeySet(), this);
return keySet;
}

The documentation for keySet etc. states:

 * reflected in the set, and vice-versa.  If the map is modified
 * while an iteration over the set is in progress (except through
 * the iterator's own remove operation), the results of
 * the iteration are undefined.  The set supports element removal,


The documentation on the enumeration methods does not say anything.

We should probably update the documentation to additionally say something like 
that on Collections.synchronized* methods.


> Without synchronization this is not guaranteed since the writes may not be 
> visible and Hashtable iterator failure behaviour is already likely to vary 
> between platforms/architectures. With RTM it's presumed that the writes will 
> NOT be visible until the transaction completes. This implies that the failure 
> mode from Hashtable iterators is likely to change just by turning RTM locking 
> on whether we make this code change or not. :-( 
> 

>> I think this change is misguided.
> 
> I think we are fine for Vector, but Hashtable gives me concerns even in it's 
> current state.
> 

I don't think the current situation is made any worse by your changes.

The are some subtle changes with regards parameter checking and throwing 
exceptions, but that does not seems to be very important behaviour to preserve.

Paul.

[9] Review request for 8039172: Tidy warnings cleanup for java.net, java.math, java.time, java.rmi

[alexander stepanov]

Hello,

Could you please review the fix for the following bug:
https://bugs.openjdk.java.net/browse/JDK-8039172

Webrev corresponding:
http://cr.openjdk.java.net/~yan/8039172/webrev.00/

Just a minor cleanup of javadoc to avoid tidy warnings; no other code 
affected.


Thanks.

Regards,
Alexander

Re: Improve timezone mapping for AIX platform

[Jonathan Lu]

Hi Volker, Masayoshi,

I made another  patch which fixed the problems mentioned in last mail,

http://cr.openjdk.java.net/~luchsh/JDK-8034220.v3/

Could you pls help to take a look?

Many thanks
Jonathan



On Thu, Apr 3, 2014 at 12:34 AM, Jonathan Lu wrote:

>  Hi Volker,
>
>
> On 2014年04月02日 23:07, Volker Simonis wrote:
>
> Hi Jonathan,
>
> thanks for updating the change. Please find my comments inline:
>
> On Tue, Apr 1, 2014 at 4:52 PM, Jonathan Lu  
>  wrote:
>
>  Hi Volker, Masayoshi,
>
> Thanks a lot for your review, here's the updated patch, could you pls take a
> look?
> http://cr.openjdk.java.net/~luchsh/JDK-8034220.v2/
>
>
> On Thu, Mar 27, 2014 at 1:48 AM, Volker Simonis  
> 
> wrote:
>
>  Hi Jonathan,
>
> thanks for doing this change. Please find some comments below:
>
> 1. please update the copyright year to 2014 in every file you touch
>
>  Updated in new patch.
>
>
>  2. in CopyFiles.gmk you can simplify the change by joining the windows
> and aix cases because on Windows OPENJDK_TARGET_OS is the same like
> OPENJDK_TARGET_OS_API_DIR. So you can write:
>
> ifneq ($(findstring $(OPENJDK_TARGET_OS), windows aix), )
>
>   TZMAPPINGS_SRC := $(JDK_TOPDIR)/src/$(OPENJDK_TARGET_OS)/lib
>
>   $(LIBDIR)/tzmappings: $(TZMAPPINGS_SRC)/tzmappings
> $(call install-file)
>
>   COPY_FILES += $(LIBDIR)/tzmappings
>
> endif
>
>  I've tried that approach before, but OPENJDK_TARGET_OS_API_DIR is 'solaris'
> as I observed on my AIX box,
> solaris/lib is not the directory where tzmappings was stored for AIX.
> So I'm keeping this change, please fix me if I was wrong about the config.
>
>
>  Yes, but my point was to actually use OPENJDK_TARGET_OS for the
> construction of  TZMAPPINGS_SRC as shown in the code snippet above.
> OPENJDK_TARGET_OS is "windows" on Windows platforms and "aix" on AIX
> and that should be just enough here.
>
>
> That's right, let me try that  and also build/test on Windows platform.
>
>
>   3. regarding the changes proposed by Masayoshi: I agree that we should
> put the AIX timezone mapping code in its own function, but I don't
> think that getPlatformTimeZoneID() would be the right place. As far as
> I understand, getPlatformTimeZoneID() is used to get a platform time
> zone id if the environment variable "TZ" is not set. I don't know a
> way how this could be done on AIX (@Jonathan: maybe you know?). If
> there would be a way to get the time zone from some system files or
> so, then we should put this code into the AIX version of
> getPlatformTimeZoneID().
>
>  I guess we may try to use /etc/envrionment file, which is basic setting for
> all processes,
> seehttp://publib.boulder.ibm.com/infocenter/aix/v7r1/index.jsp?topic=%2Fcom.ibm.aix.files%2Fdoc%2Faixfiles%2Fenvironment.htm
> The implementation of  getPlatformTimeZoneID() has been updated.
>
>
>  That's good!
>
>
>  But the current AIX code contributed by Jonathan, actually uses the
> time zone id from the "TZ" environment variable and just maps it to a
> Java compatible time zone id. So I'd suggest to refactor this code
> into a function like for example "static const char*
> mapPlatformToJavaTimzone(const char* tz)" and call that from
> findJavaTZ_md(). I think that would make the code easier to
> understand.
>
>  Agree, and have split the code into a separate static method to do the
> mapping work.
>
>
>  Good. But there's still one error in findJavaTZ_md():
>
>  706 #ifdef _AIX
>  707 javatz = mapPlatformToJavaTimezone(java_home_dir, tz);
>  708 #endif
>
> This should read:
>
>  706 #ifdef _AIX
>  707 javatz = mapPlatformToJavaTimezone(java_home_dir, javatz);
>  708 #endif
>
> because in line 703 you free 'tz' via its alias 'freetz' if 'tz' came
> from getPlatformTimeZoneID().
>
>
> Right, but with the second approach, there may be a minor memory leak here,
> as javatz was not freed before being overwritten on AIX. will post another
> patch on this soon.
>
>
>  With the above fixed I'll push this one we get one more review from a
> reviewer (I'm currently not an official reviewer).
>
> Regards,
> Volker
>
>
>
>  @Masayoshi: what do you think, would you agree with such a solution.
>
> 4. I agree with Masayoshi that you should use the existing
> getGMTOffsetID()
>
>  Updated in new patch to eliminate duplication.
>
>
>  5. Please notice that your change breaks all Unix builds except AIX
> because of:
>
>  759 }
>  760 tzerr:
>  761 if (javatz == NULL) {
>  762 time_t offset;
> ...
>  782 }
>  783 #endif
>
> I think that should read:
>
>  759
>  760 tzerr:
>  761 if (javatz == NULL) {
>  762 time_t offset;
> ...
>  782 }
>  783 #endif
>  784 }
>
> Refactoring the code in an extra function will make that error more
> evident anyway.
>
> But please always build at least on one different platform (i.e.
> Linux) to prevent such errors in the future.
>
>  My fault, sorry for the failure, should take no chance after any manual
> alteration.
>
>
>  Regards,
> Volk