Re: RFR: 8043592: The basic XML parser based on UKit fails to read XML files encoded in UTF-16BE or LE

[huizhe wang]

Hi Sherman, Lance,

Thanks for reviews.

It appears resetting InputStream is not reliable since not every 
InputStream will support reset. I've modified the code. For other 
changes, pls see inline comments.


On 5/22/2014 10:25 AM, Xueming Shen wrote:

Hi

(1) Do we really need those shift at line ln#2989/90 and 2994/95? it 
appears to me
 those bytes have been decided to be ZERO already, we are talking 
about

 mChar[0] = '<' and mChar[1] = '?' here, right?


Fixed. No need indeed.



(2) for test, maybe we should just do p.loadFromXML(in) ? that path 
should verify the

 fix as well (the real use scenario), right?


I've removed the test and updated LoadAndStoreXM instead, as Alan 
suggested, to cover UTF-16BE/LE.




(3) do we have tests for utf16 bom? if not, I would suggest to throw 
in UTF-16BE/LE-BOM

 into the charset[], just in case.


java.nio.charset states that it writes BOM when encoding in UTF-16, but 
not for BE or LE.  That is why the tests behaved differently, that is, 
detecting BOM in the case of UTF-16, but not for UTF-16BE/LE.


I added tests to manually append BOM in the case of UTF-16BE/LE to 
verify that the code is capable of handling these cases (although 
normally they won't come with BOM).


http://cr.openjdk.java.net/~joehw/jdk9/8043592/webrev/
Thanks,
Joe



thanks!
-Sherman

On 05/22/2014 09:30 AM, huizhe wang wrote:
Refer to 8042889, while verifying/testing 8042889, we noticed that 
the tiny XML parser failed on UTF-16BE or LE. The cause of the 
failure was that the parser was actually implemented to abide by the 
XML specification that required entities encoded in UTF-16 to begin 
with BOM. The test we used sent a byte array to the parser without 
BOM, thus failed.


Since it's not uncommon for a XML to not have BOM, I borrowed the 
technique used in Xerces to add an additional check for UTF-16 
encoding.  Please review.


http://cr.openjdk.java.net/~joehw/jdk9/8043592/webrev/

Thanks,
Joe

Webrev for 8043342: StringBuffer/StringBuilder crypto changes.

[Bradford Wetmore]

No additional code review necessary, this is just an FYI.

For internal reasons (i.e. we have to sign our JCE jar files), we have 
separated the JCE portion for:


8041679: Replace uses of StringBuffer with StringBuilder within the JDK

into:

8043342: Replace uses of StringBuffer with StringBuilder within crypto code

It's the exact same code, but only the four JCE files are included here.

http://cr.openjdk.java.net/~wetmore/8043342/webrev.00/

I am sponsoring this for Jamil, from an original fix [1] sponsored by 
Paul Sandoz from:


Otávio Gonçalves de Santana

Thanks,

Brad

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-May/026820.html

Re: RFR 9: 8003488 Add Process.getPid

[roger riggs]

Thanks for the feedback and recommendations; the webrev has been updated.

Webrev: http://cr.openjdk.java.net/~rriggs/webrev-getpid-8003488/

Alan, on the use of tasklist, I think a cleaner test can be written when the
Java API to inspect other processes is available.  I did not find a 
straightforward
eqivalent for $$; there is a possible hack using tasklist but it would 
be a throwaway.


Roger



On 5/22/2014 11:01 AM, Alan Bateman wrote:

On 22/05/2014 14:49, roger riggs wrote:

Hi,

The webrev has been updated to more completely describe the pid:
"The native process id is an identification number that the operating 
system assigns to the process."


Any other comments?

Webrev: http://cr.openjdk.java.net/~rriggs/webrev-getpid-8003488/
Roger and I discussed this one last week so I'm happy with the 
signature and javadoc.


I see Ivan has picked up on the HANDLE <--> jlong and jlong_to_ptr is 
normally the macro that we use for that.


A minor consistency point is that the getPid is added before the 
constructor whereas the other overrides are below the constructor.


For the test then it would be good to add the issue number to the @bug 
line.


One question on the test as I'm not familiar with output of tasklist. 
If 2+ test were to run tasklist at the same time then could it confuse 
this test? I'm just wondering if there is anything closer to echo $$.


Otherwise looks good to me.

-Alan.

Re: RFR: JDK-8042369 Remove duplicated java.time classes in build.tools.tzdb

[Xueming Shen]

Masayoshi, Roger,

OK, let's forget the fancy TarInputStream for now:-)

Here is the webrev that just updates the code to use existing java.time classes
for tzdb data building.

http://cr.openjdk.java.net/~sherman/8042369/webrev

The difference compared to the last version [1] is that it reads in individual 
files
from the file system, as the existing tool does.

Thanks!
-Sherman

[1] http://cr.openjdk.java.net/~sherman/8042369/webrev.01/

On 05/20/2014 01:52 PM, roger riggs wrote:

Hi Sherman,

Even though it has well defined content, checking in the tar.gz seems not quite 
right.
Can the tests reconstruct the tar file from the contents or directly use
the IANA data on demand from the IANA site?

From a maintenance point of view,  functions added to the JDK should be
well used and maintained and be well supported.

TzdbZoneRulesCompiler.java: +130 The error for srcFile == null still mentions 
"-srcdir".

 :+153; the commented out message should be removed


Roger


On 5/19/2014 2:12 PM, Xueming Shen wrote:

Hi,

I've not got any feedback so far, so I assume it's good:-)

Anyway, I'm going a little further to throw in a TarInputStream so now we just 
build the
tzdb data directly on  the tzdata201xy.tar.gz from iana site. I'm not sure if 
it is OK to
check in the original .tar.gz file into the jdk repository though.

There are also more improvements in this version, it is much faster now. It 
might
not matter as it is only used during build time, though:-)

http://cr.openjdk.java.net/~sherman/8042369/webrev

-Sherman

Btw, I'm still trying to sell the idea of checking in a second tzdb provider 
implementation
into the jdk, which directly loads in the tzdb data by using the iana original 
source data,
with the assumption that this might be useful at least in certain circumstance 
(such as
one gov pushes a tz change, and some of your big jdk customers need their apps 
run with
the new data in next 24 hrs...) in the future.

http://cr.openjdk.java.net/~sherman/tzdbProvider/webrev


On 05/04/2014 11:16 PM, Xueming Shen wrote:

Hi

Please help review the change for #8042369

Issue: https://bugs.openjdk.java.net/browse/JDK-8042369
Webrev: http://cr.openjdk.java.net/~sherman/8042369/webrev

In jdk8 we had to duplicate dozen java.time classes in build.tools to build the 
timezone data
for the new JSR310 timezone data compiler, which uses the new jdk8 java.time 
classes (these
classes don't exit in the < 8 boot jdk). JDK9 has switched to use the jdk8 as 
the boot jdk,  so
most of these duplicated classes are no longer needed, with ZoneRules as the 
only exception.
The ZoneRules is still needed to help the tzdb compiler to output the tzdb data 
in the
serialization forms of those transitions and rules. The proposed change here is 
to remove
those unneeded duplicated classes.

I also took the opportunity to re-organize/re-wrote the "builder" classes to 
have a faster, simpler
and and straightforward implementation, with the goal of migrating it into a 
second default
ZoneRulesProvider later to plug it into jdk, so jdk/jre can uses the tzdb 
source data from the
IANA directly. One of the benefits of such a provider is that the end user may 
just drop the latest
timezone data file into the jdk/jre and go, without waiting for the latest 
timezone binary bits from
Oracle.

Here is the webrev for the idea
http://cr.openjdk.java.net/~sherman/tzdbProvider/webrev/

The only disadvantage appears to be the possible "slowdown" of startup time 
because of
the reading and compiling of the 200k tzdb source data...(we need another new 
"bridge" for
 j.u.TimeZone, if we go with this direction)

Thanks!
-Sherman

Re: RFR: 8043592: The basic XML parser based on UKit fails to read XML files encoded in UTF-16BE or LE

[Xueming Shen]

Hi

(1) Do we really need those shift at line ln#2989/90 and 2994/95? it appears to 
me
 those bytes have been decided to be ZERO already, we are talking about
 mChar[0] = '<' and mChar[1] = '?' here, right?

(2) for test, maybe we should just do p.loadFromXML(in) ? that path should 
verify the
 fix as well (the real use scenario), right?

(3) do we have tests for utf16 bom? if not, I would suggest to throw in 
UTF-16BE/LE-BOM
 into the charset[], just in case.

thanks!
-Sherman

On 05/22/2014 09:30 AM, huizhe wang wrote:

Refer to 8042889, while verifying/testing 8042889, we noticed that the tiny XML 
parser failed on UTF-16BE or LE. The cause of the failure was that the parser 
was actually implemented to abide by the XML specification that required 
entities encoded in UTF-16 to begin with BOM. The test we used sent a byte 
array to the parser without BOM, thus failed.

Since it's not uncommon for a XML to not have BOM, I borrowed the technique 
used in Xerces to add an additional check for UTF-16 encoding.  Please review.

http://cr.openjdk.java.net/~joehw/jdk9/8043592/webrev/

Thanks,
Joe

Re: RFR: 8043592: The basic XML parser based on UKit fails to read XML files encoded in UTF-16BE or LE

[Lance Andersen]

Looks OK.  I would suggest removing the commented out code from the test before 
you push to the workspace

Best
Lace
On May 22, 2014, at 12:30 PM, huizhe wang  wrote:

> Refer to 8042889, while verifying/testing 8042889, we noticed that the tiny 
> XML parser failed on UTF-16BE or LE. The cause of the failure was that the 
> parser was actually implemented to abide by the XML specification that 
> required entities encoded in UTF-16 to begin with BOM. The test we used sent 
> a byte array to the parser without BOM, thus failed.
> 
> Since it's not uncommon for a XML to not have BOM, I borrowed the technique 
> used in Xerces to add an additional check for UTF-16 encoding.  Please review.
> 
> http://cr.openjdk.java.net/~joehw/jdk9/8043592/webrev/
> 
> Thanks,
> Joe



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

RFR: 8043592: The basic XML parser based on UKit fails to read XML files encoded in UTF-16BE or LE

[huizhe wang]

Refer to 8042889, while verifying/testing 8042889, we noticed that the 
tiny XML parser failed on UTF-16BE or LE. The cause of the failure was 
that the parser was actually implemented to abide by the XML 
specification that required entities encoded in UTF-16 to begin with 
BOM. The test we used sent a byte array to the parser without BOM, thus 
failed.


Since it's not uncommon for a XML to not have BOM, I borrowed the 
technique used in Xerces to add an additional check for UTF-16 
encoding.  Please review.


http://cr.openjdk.java.net/~joehw/jdk9/8043592/webrev/

Thanks,
Joe

Re: RFR 8043772: Typos in Double/Int/LongSummaryStatistics.java

[Mike Duigou]

Looks fine to me.

Mike

You are using an old version of webrev :-)

wget http://hg.openjdk.java.net/code-tools/webrev/raw-file/tip/webrev.ksh


On May 22 2014, at 08:36 , Ivan Gerasimov  wrote:

> Hello!
> 
> Some functions were renamed with the fix for JDK-8015318.
> A few of them kept their old names in the comments.
> Would you please review this trivial fix?
> 
> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8043772
> WEBREV: http://cr.openjdk.java.net/~igerasim/8043772/0/webrev/
> 
> Sincerely yours,
> Ivan

Re: RFR 9: 8003488 Add Process.getPid

[roger riggs]

Hi,

Thanks for the rationale.

Using the natural terminology familiar to developers seems most useful.
Mac, Windows, and Linux refer to these values a process id or pid
as the handle for a Process; it appears in the tools like ps and tasklist.
The javadoc can explain anything more specific or system dependent.

Roger


On 5/22/2014 11:55 AM, David M. Lloyd wrote:

On 05/22/2014 10:44 AM, Alan Bateman wrote:

On 22/05/2014 16:34, David M. Lloyd wrote:


I guess this is a little late, and minor, but the "jstack" tool uses
the acronym "nid" for this purpose, which I believe is mapped to the
same concept (on Linux it is anyway).

I think either this terminology should be unified on the jstack side,
or else the method should be called "getNativeId" or "getNid" or
something.

Are you thinking of "vmid" by any chance? If so then that term comes
from jvmstat. It uses String for the identifier because it can support
inspecting the counters of remote VMs (pid@host for example). If I
recall correctly then jstack allows for a String too, mostly because it
has its roots as a SA tool where it can connect to a remote VM when
jsadebugd is running.


Nah I'm thinking of this:


$ jstack 30044
2014-05-22 10:39:55
Full thread dump Java HotSpot(TM) 64-Bit Server VM (24.51-b03 mixed 
mode):

[...]
"RMI TCP Accept-0" daemon prio=10 tid=0x7ff884143000 nid=0x756e

The "nid" here is the hex-encoded process ID of the thread.  But 
instead of saying "process ID" or whatever, it's "native ID".  As 
stated earlier though, on Linux the thread and process IDs share a 
namespace so it's not always true that these are the *same* thing; 
however, it illustrates that in general, threads have a "native ID", 
reserving "thread ID" as an internal JVM concept.  I would think the 
same would apply to processes, especially given that "process ID" is a 
first-order concept on some (but maybe not all) OSes, and even OSes 
without that first-order PID concept might still have some other 
meaningful numeric mapping for the native ID of a Process.

Re: RFR 9: 8003488 Add Process.getPid

[David M. Lloyd]

On 05/22/2014 10:44 AM, Alan Bateman wrote:

On 22/05/2014 16:34, David M. Lloyd wrote:


I guess this is a little late, and minor, but the "jstack" tool uses
the acronym "nid" for this purpose, which I believe is mapped to the
same concept (on Linux it is anyway).

I think either this terminology should be unified on the jstack side,
or else the method should be called "getNativeId" or "getNid" or
something.

Are you thinking of "vmid" by any chance? If so then that term comes
from jvmstat. It uses String for the identifier because it can support
inspecting the counters of remote VMs (pid@host for example). If I
recall correctly then jstack allows for a String too, mostly because it
has its roots as a SA tool where it can connect to a remote VM when
jsadebugd is running.


Nah I'm thinking of this:


$ jstack 30044
2014-05-22 10:39:55
Full thread dump Java HotSpot(TM) 64-Bit Server VM (24.51-b03 mixed mode):
[...]
"RMI TCP Accept-0" daemon prio=10 tid=0x7ff884143000 nid=0x756e 



The "nid" here is the hex-encoded process ID of the thread.  But instead 
of saying "process ID" or whatever, it's "native ID".  As stated earlier 
though, on Linux the thread and process IDs share a namespace so it's 
not always true that these are the *same* thing; however, it illustrates 
that in general, threads have a "native ID", reserving "thread ID" as an 
internal JVM concept.  I would think the same would apply to processes, 
especially given that "process ID" is a first-order concept on some (but 
maybe not all) OSes, and even OSes without that first-order PID concept 
might still have some other meaningful numeric mapping for the native ID 
of a Process.


--
- DML

Re: RFR 8043772: Typos in Double/Int/LongSummaryStatistics.java

[roger riggs]

Hi Ivan,

Looks fine, Roger

On 5/22/2014 11:36 AM, Ivan Gerasimov wrote:

Hello!

Some functions were renamed with the fix for JDK-8015318.
A few of them kept their old names in the comments.
Would you please review this trivial fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8043772
WEBREV: http://cr.openjdk.java.net/~igerasim/8043772/0/webrev/

Sincerely yours,
Ivan

Re: RFR 9: 8003488 Add Process.getPid

[David M. Lloyd]

On 05/22/2014 10:34 AM, David M. Lloyd wrote:

On 05/22/2014 08:49 AM, roger riggs wrote:

Hi,

The webrev has been updated to more completely describe the pid:
"The native process id is an identification number that the operating
system assigns to the process."

Any other comments?

Webrev: http://cr.openjdk.java.net/~rriggs/webrev-getpid-8003488/

Issue: https://bugs.openjdk.java.net/browse/JDK-8003488


I guess this is a little late, and minor, but the "jstack" tool uses the
acronym "nid" for this purpose, which I believe is mapped to the same
concept (on Linux it is anyway).

I think either this terminology should be unified on the jstack side, or
else the method should be called "getNativeId" or "getNid" or something.


Refining that thought - on Linux, process IDs and thread IDs share a 
namespace, but maybe that isn't true on all platforms.  Still calling it 
"getPid" seems very UNIX-specific.  Process.getNativeId() seems "safer" 
or more future-proof to me, somehow.


--
- DML

Re: RFR 9: 8003488 Add Process.getPid

[roger riggs]

Hi,

jstack -help uses "pid";  where are looking?

Roger

On 5/22/2014 11:34 AM, David M. Lloyd wrote:

On 05/22/2014 08:49 AM, roger riggs wrote:

Hi,

The webrev has been updated to more completely describe the pid:
"The native process id is an identification number that the operating
system assigns to the process."

Any other comments?

Webrev: http://cr.openjdk.java.net/~rriggs/webrev-getpid-8003488/

Issue: https://bugs.openjdk.java.net/browse/JDK-8003488


I guess this is a little late, and minor, but the "jstack" tool uses 
the acronym "nid" for this purpose, which I believe is mapped to the 
same concept (on Linux it is anyway).


I think either this terminology should be unified on the jstack side, 
or else the method should be called "getNativeId" or "getNid" or 
something.

Re: RFR 9: 8003488 Add Process.getPid

[Alan Bateman]

On 22/05/2014 16:34, David M. Lloyd wrote:


I guess this is a little late, and minor, but the "jstack" tool uses 
the acronym "nid" for this purpose, which I believe is mapped to the 
same concept (on Linux it is anyway).


I think either this terminology should be unified on the jstack side, 
or else the method should be called "getNativeId" or "getNid" or 
something.
Are you thinking of "vmid" by any chance? If so then that term comes 
from jvmstat. It uses String for the identifier because it can support 
inspecting the counters of remote VMs (pid@host for example). If I 
recall correctly then jstack allows for a String too, mostly because it 
has its roots as a SA tool where it can connect to a remote VM when 
jsadebugd is running.


-Alan.

RFR 8043772: Typos in Double/Int/LongSummaryStatistics.java

[Ivan Gerasimov]

Hello!

Some functions were renamed with the fix for JDK-8015318.
A few of them kept their old names in the comments.
Would you please review this trivial fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8043772
WEBREV: http://cr.openjdk.java.net/~igerasim/8043772/0/webrev/

Sincerely yours,
Ivan

Re: RFR 9: 8003488 Add Process.getPid

[David M. Lloyd]

On 05/22/2014 08:49 AM, roger riggs wrote:

Hi,

The webrev has been updated to more completely describe the pid:
"The native process id is an identification number that the operating
system assigns to the process."

Any other comments?

Webrev: http://cr.openjdk.java.net/~rriggs/webrev-getpid-8003488/

Issue: https://bugs.openjdk.java.net/browse/JDK-8003488


I guess this is a little late, and minor, but the "jstack" tool uses the 
acronym "nid" for this purpose, which I believe is mapped to the same 
concept (on Linux it is anyway).


I think either this terminology should be unified on the jstack side, or 
else the method should be called "getNativeId" or "getNid" or something.



--
- DML

Re: RFR 9: 8003488 Add Process.getPid

[Alan Bateman]

On 22/05/2014 14:49, roger riggs wrote:

Hi,

The webrev has been updated to more completely describe the pid:
"The native process id is an identification number that the operating 
system assigns to the process."


Any other comments?

Webrev: http://cr.openjdk.java.net/~rriggs/webrev-getpid-8003488/ 

Roger and I discussed this one last week so I'm happy with the signature 
and javadoc.


I see Ivan has picked up on the HANDLE <--> jlong and jlong_to_ptr is 
normally the macro that we use for that.


A minor consistency point is that the getPid is added before the 
constructor whereas the other overrides are below the constructor.


For the test then it would be good to add the issue number to the @bug line.

One question on the test as I'm not familiar with output of tasklist. If 
2+ test were to run tasklist at the same time then could it confuse this 
test? I'm just wondering if there is anything closer to echo $$.


Otherwise looks good to me.

-Alan.

Re: RFR 9: 8003488 Add Process.getPid

[Ivan Gerasimov]

Hi Roger!

On 22.05.2014 17:49, roger riggs wrote:

Hi,

The webrev has been updated to more completely describe the pid:
"The native process id is an identification number that the operating 
system assigns to the process."


Any other comments?

I assume it compiles fine, but in 
src/windows/native/java/lang/ProcessImpl_md.c:


258 DWORD pid = GetProcessId(handle);

GetProcessId requires an argument of type HANDLE, which is defined as void*.
Wouldn't it be better to explicitly cast jlong to HANDLE?

Sincerely yours,
Ivan

Webrev: http://cr.openjdk.java.net/~rriggs/webrev-getpid-8003488/ 


Issue: https://bugs.openjdk.java.net/browse/JDK-8003488

Thanks, Roger

Re: RFR - 8042857: 14 stuck threads waiting for notification on LDAPRequest

[Vincent Ryan]

Fix looks fine Rob.
Thanks.

On 16 May 2014, at 17:00, Rob McKenna  wrote:

> Hi folks,
> 
> A simple fix to eliminate a possible infinite loop when an Ldap Connection 
> cannot contact the server.
> 
> http://cr.openjdk.java.net/~robm/8042857/webrev.01/
> 
>-Rob
> 

Re: RFR 9: 8003488 Add Process.getPid

[Paul Benedict]

Looks good. Don't forget @since 1.9 in the javadoc


Cheers,
Paul


On Thu, May 22, 2014 at 8:49 AM, roger riggs  wrote:

> Hi,
>
> The webrev has been updated to more completely describe the pid:
> "The native process id is an identification number that the operating
> system assigns to the process."
>
> Any other comments?
>
> Webrev: http://cr.openjdk.java.net/~rriggs/webrev-getpid-8003488/ <
> http://cr.openjdk.java.net/%7Erriggs/webrev-getpid-8003488/>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8003488
>
> Thanks, Roger
>
>

Re: RFR 9: 8003488 Add Process.getPid

[roger riggs]

Hi,

The webrev has been updated to more completely describe the pid:
"The native process id is an identification number that the operating 
system assigns to the process."


Any other comments?

Webrev: http://cr.openjdk.java.net/~rriggs/webrev-getpid-8003488/ 


Issue: https://bugs.openjdk.java.net/browse/JDK-8003488

Thanks, Roger

Re: RFR JDK-8042887: Remove serialver -show, this tool does not need a GUI

[Chris Hegarty]

It is up to you. Consider it reviewed, from my perspective, either way.

-Chris.

On 22 May 2014, at 14:47, Pavel Rappo  wrote:

> Yeah, I've done that before. But finally decided to exclude this change and 
> several more unrelated to the issue. I believe from the 'hg' history 
> perspective it will look cleaner. I can file a separate bug for a tiny 
> refactoring of this tool if you want, but being realistic I don't think 
> anybody needs it. Here they are:
> 
> @@ -169,11 +66,11 @@
> 
> */
>static String serialSyntax(String classname) throws ClassNotFoundException 
> {
>String ret = null;
>boolean classFound = false;
> 
> 
> -// If using old style of qualifyling inner classes with '$'s.
> +// If using old style of qualifying inner classes with '$'s.
> 
>if (classname.indexOf('$') != -1) {
>ret = resolveClass(classname);
>} else {
>/* Try to resolve the fully qualified name and if that fails, start
> * replacing the '.'s with '$'s starting from the last '.', until
> 
> @@ -276,25 +165,23 @@
> 
>} catch (IOException ioe) {
>System.err.println(Res.getText("error.parsing.classpath", envcp));
>System.exit(3);
>}
> 
> 
> -if (!show) {
> 
>/*
> 
> - * Check if there are any class names specified, if it is not a
> - * invocation with the -show option.
> + * Check if there are any class names specified
> 
> */
>if (i == args.length) {
>usage();
>System.exit(1);
>}
> 
>/*
> * The rest of the parameters are classnames.
> */
>boolean exitFlag = false;
> 
> -for (i = i; i < args.length; i++ ) {
> +for (; i < args.length; i++) {
> 
>try {
>String syntax = serialSyntax(args[i]);
>if (syntax != null)
>System.out.println(args[i] + ":" + syntax);
>else {
> 
> @@ -184,17 +81,17 @@
> 
>classFound = true;
>} catch (ClassNotFoundException e) {
>/* Class not found so far */
>}
>if (!classFound) {
> 
> -StringBuffer workBuffer = new StringBuffer(classname);
> -String workName = workBuffer.toString();
> +StringBuilder workBuilder = new StringBuilder(classname);
> +String workName = workBuilder.toString();
> 
>int i;
>while ((i = workName.lastIndexOf('.')) != -1 && !classFound) {
> 
> -workBuffer.setCharAt(i, '$');
> +workBuilder.setCharAt(i, '$');
> 
>try {
> 
> -workName = workBuffer.toString();
> +workName = workBuilder.toString();
> 
>ret = resolveClass(workName);
>classFound = true;
>} catch (ClassNotFoundException e) {
>/* Continue searching */
>}
> @@ -423,49 +237,26 @@
> 
> * get and format message string from resource
> *
> * @param key selects message from resource
> */
>static String getText(String key) {
> 
> -return getText(key, (String)null);
> +return getText(key, null);
> 
>}
> 
>/**
> * get and format message string from resource
> *
> * @param key selects message from resource
> * @param a1 first argument
> */
>static String getText(String key, String a1) {
> 
> -return getText(key, a1, null);
> -}
> -
> -/**
> - * get and format message string from resource
> - *
> - * @param key selects message from resource
> - * @param a1 first argument
> - * @param a2 second argument
> - */
> -static String getText(String key, String a1, String a2) {
> -return getText(key, a1, a2, null);
> -}
> -
> -/**
> - * get and format message string from resource
> - *
> - * @param key selects message from resource
> - * @param a1 first argument
> - * @param a2 second argument
> - * @param a3 third argument
> - */
> -static String getText(String key, String a1, String a2, String a3) {
> 
>if (messageRB == null) {
>initResource();
>}
>try {
>String message = messageRB.getString(key);
> 
> -return MessageFormat.format(message, a1, a2, a3);
> +return MessageFormat.format(message, a1);
> 
>} catch (MissingResourceException e) {
>throw new Error("Fatal: Resource for serialver is broken. There is 
> no " + key + " key in resource.");
>}
>}
> }
> 
> 
> -Pavel
> 
> On 22 May 2014, at 14:32, Chris Hegarty  wrote:
> 
>> This looks good to me.
>> 
>> Trivially, I think you could remove the 3 a

Re: RFR JDK-8042887: Remove serialver -show, this tool does not need a GUI

[Pavel Rappo]

Yeah, I've done that before. But finally decided to exclude this change and 
several more unrelated to the issue. I believe from the 'hg' history 
perspective it will look cleaner. I can file a separate bug for a tiny 
refactoring of this tool if you want, but being realistic I don't think anybody 
needs it. Here they are:

@@ -169,11 +66,11 @@

 */
static String serialSyntax(String classname) throws ClassNotFoundException {
String ret = null;
boolean classFound = false;


-// If using old style of qualifyling inner classes with '$'s.
+// If using old style of qualifying inner classes with '$'s.

if (classname.indexOf('$') != -1) {
ret = resolveClass(classname);
} else {
/* Try to resolve the fully qualified name and if that fails, start
 * replacing the '.'s with '$'s starting from the last '.', until

@@ -276,25 +165,23 @@

} catch (IOException ioe) {
System.err.println(Res.getText("error.parsing.classpath", envcp));
System.exit(3);
}


-if (!show) {

/*

- * Check if there are any class names specified, if it is not a
- * invocation with the -show option.
+ * Check if there are any class names specified

 */
if (i == args.length) {
usage();
System.exit(1);
}

/*
 * The rest of the parameters are classnames.
 */
boolean exitFlag = false;

-for (i = i; i < args.length; i++ ) {
+for (; i < args.length; i++) {

try {
String syntax = serialSyntax(args[i]);
if (syntax != null)
System.out.println(args[i] + ":" + syntax);
else {

@@ -184,17 +81,17 @@

classFound = true;
} catch (ClassNotFoundException e) {
/* Class not found so far */
}
if (!classFound) {

-StringBuffer workBuffer = new StringBuffer(classname);
-String workName = workBuffer.toString();
+StringBuilder workBuilder = new StringBuilder(classname);
+String workName = workBuilder.toString();

int i;
while ((i = workName.lastIndexOf('.')) != -1 && !classFound) {

-workBuffer.setCharAt(i, '$');
+workBuilder.setCharAt(i, '$');

try {

-workName = workBuffer.toString();
+workName = workBuilder.toString();

ret = resolveClass(workName);
classFound = true;
} catch (ClassNotFoundException e) {
/* Continue searching */
}
@@ -423,49 +237,26 @@

 * get and format message string from resource
 *
 * @param key selects message from resource
 */
static String getText(String key) {

-return getText(key, (String)null);
+return getText(key, null);

}

/**
 * get and format message string from resource
 *
 * @param key selects message from resource
 * @param a1 first argument
 */
static String getText(String key, String a1) {

-return getText(key, a1, null);
-}
-
-/**
- * get and format message string from resource
- *
- * @param key selects message from resource
- * @param a1 first argument
- * @param a2 second argument
- */
-static String getText(String key, String a1, String a2) {
-return getText(key, a1, a2, null);
-}
-
-/**
- * get and format message string from resource
- *
- * @param key selects message from resource
- * @param a1 first argument
- * @param a2 second argument
- * @param a3 third argument
- */
-static String getText(String key, String a1, String a2, String a3) {

if (messageRB == null) {
initResource();
}
try {
String message = messageRB.getString(key);

-return MessageFormat.format(message, a1, a2, a3);
+return MessageFormat.format(message, a1);

} catch (MissingResourceException e) {
throw new Error("Fatal: Resource for serialver is broken. There is 
no " + key + " key in resource.");
}
}
}


-Pavel

On 22 May 2014, at 14:32, Chris Hegarty  wrote:

> This looks good to me.
> 
> Trivially, I think you could remove the 3 and 4 arg Res.getText methods, as I 
> don’t see them being used.
> 
> -Chris.
> 
> On 22 May 2014, at 09:47, Pavel Rappo  wrote:
> 
>> Hi everyone,
>> 
>> could you please review my change for JDK-8042887?
>> 
>> http://cr.openjdk.java.net/~alanb/8042887/webrev/
>> 
>> I also created following issues for appropriate docs/localization updates:
>> 
>> https://bugs.openjdk.java.ne

Re: RFR JDK-8042887: Remove serialver -show, this tool does not need a GUI

[Chris Hegarty]

This looks good to me.

Trivially, I think you could remove the 3 and 4 arg Res.getText methods, as I 
don’t see them being used.

-Chris.

On 22 May 2014, at 09:47, Pavel Rappo  wrote:

> Hi everyone,
> 
> could you please review my change for JDK-8042887?
> 
> http://cr.openjdk.java.net/~alanb/8042887/webrev/
> 
> I also created following issues for appropriate docs/localization updates:
> 
> https://bugs.openjdk.java.net/browse/JDK-8043613
> https://bugs.openjdk.java.net/browse/JDK-8043620
> 
> Thanks
> -Pavel

Re: RFR JDK-8042887: Remove serialver -show, this tool does not need a GUI

[Alan Bateman]

On 22/05/2014 09:47, Pavel Rappo wrote:

Hi everyone,

could you please review my change for JDK-8042887?

http://cr.openjdk.java.net/~alanb/8042887/webrev
Thanks Pavel, good to see this legacy option going away. The change 
looks good to me and I'm happy to sponsor it for you.


-Alan.

Re: RFR(S): 8043520: Serviceability tests using @library failing with java.lang.NoClassDefFoundError

[Yekaterina Kantserova]

Thanks Erik!

Staffan, could you please be my sponsor and push the change?

// Katja



On 05/22/2014 11:02 AM, Erik Gahlin wrote:

Looks good!

Erik

Yekaterina Kantserova skrev 2014-05-20 15:48:

Thanks Staffan!

New webrev can be found here: 
http://cr.openjdk.java.net/~ykantser/8043520/webrev.01/


// Katja



On 05/20/2014 03:07 PM, Staffan Larsen wrote:

test/sun/management/jmxremote/bootstrap/PasswordFilePermissionTest.java
   “Dummy” is being built twice.

Otherwise good!

/Staffan


On 20 maj 2014, at 14:24, Yekaterina Kantserova 
 wrote:



Staffan, Alan,

could you please review the following fix.

Bug: https://bugs.openjdk.java.net/browse/JDK-8043520
Webrev: http://cr.openjdk.java.net/~ykantser/8043520/webrev.00/

I've missed somehow several tests in 
http://cr.openjdk.java.net/~ykantser/8034960/webrev.01/ which 
existed in http://cr.openjdk.java.net/~ykantser/8034960/webrev.00/.


Thanks,
Katja







# HG changeset patch
# User ykantser
# Date 1400749536 -7200
# Node ID b81e5cefbee48c41350343e94ed807d4ee6c5292
# Parent  5b45a5efe417d0420ad29c2467247740a84064d2
8043520: Serviceability tests using @library failing with java.lang.NoClassDefFoundError
Reviewed-by: sla, egahlin

diff --git a/test/com/sun/jdi/BadHandshakeTest.java b/test/com/sun/jdi/BadHandshakeTest.java
--- a/test/com/sun/jdi/BadHandshakeTest.java
+++ b/test/com/sun/jdi/BadHandshakeTest.java
@@ -26,7 +26,7 @@
  * @summary Check that a bad handshake doesn't cause a debuggee to abort
  * @library /lib/testlibrary
  *
- * @build VMConnection BadHandshakeTest Exit0
+ * @build jdk.testlibrary.* VMConnection BadHandshakeTest Exit0
  * @run main BadHandshakeTest
  *
  */
diff --git a/test/com/sun/jdi/ExclusiveBind.java b/test/com/sun/jdi/ExclusiveBind.java
--- a/test/com/sun/jdi/ExclusiveBind.java
+++ b/test/com/sun/jdi/ExclusiveBind.java
@@ -27,8 +27,7 @@
  *  at the same time.
  * @library /lib/testlibrary
  *
- * @build jdk.testlibrary.ProcessTools jdk.testlibrary.JDKToolLauncher jdk.testlibrary.Utils
- * @build VMConnection ExclusiveBind HelloWorld
+ * @build jdk.testlibrary.* VMConnection ExclusiveBind HelloWorld
  * @run main ExclusiveBind
  */
 import java.net.ServerSocket;
diff --git a/test/com/sun/tools/attach/TempDirTest.java b/test/com/sun/tools/attach/TempDirTest.java
--- a/test/com/sun/tools/attach/TempDirTest.java
+++ b/test/com/sun/tools/attach/TempDirTest.java
@@ -38,7 +38,7 @@
  * @bug 8033104
  * @summary Test to make sure attach and jvmstat works correctly when java.io.tmpdir is set
  * @library /lib/testlibrary
- * @run build Application Shutdown RunnerUtil
+ * @run build jdk.testlibrary.* Application Shutdown RunnerUtil
  * @run main TempDirTest
  */
 
diff --git a/test/java/lang/instrument/DaemonThread/TestDaemonThread.java b/test/java/lang/instrument/DaemonThread/TestDaemonThread.java
--- a/test/java/lang/instrument/DaemonThread/TestDaemonThread.java
+++ b/test/java/lang/instrument/DaemonThread/TestDaemonThread.java
@@ -26,7 +26,7 @@
  * @summary Assert in java.lang.instrument agents during shutdown when classloading occurs after shutdown
  * @library /lib/testlibrary
  *
- * @build DummyAgent DummyClass TestDaemonThreadLauncher TestDaemonThread
+ * @build jdk.testlibrary.* DummyAgent DummyClass TestDaemonThreadLauncher TestDaemonThread
  * @run shell ../MakeJAR3.sh DummyAgent
  * @run main TestDaemonThreadLauncher /timeout=240
  *
diff --git a/test/java/lang/management/MemoryMXBean/ResetPeakMemoryUsage.java b/test/java/lang/management/MemoryMXBean/ResetPeakMemoryUsage.java
--- a/test/java/lang/management/MemoryMXBean/ResetPeakMemoryUsage.java
+++ b/test/java/lang/management/MemoryMXBean/ResetPeakMemoryUsage.java
@@ -33,7 +33,7 @@
  * @author  Mandy Chung
  *
  * @library /lib/testlibrary/
- * @build ResetPeakMemoryUsage MemoryUtil RunUtil
+ * @build jdk.testlibrary.* ResetPeakMemoryUsage MemoryUtil RunUtil
  * @run main ResetPeakMemoryUsage
  */
 
diff --git a/test/sun/management/jdp/JdpDefaultsTest.java b/test/sun/management/jdp/JdpDefaultsTest.java
--- a/test/sun/management/jdp/JdpDefaultsTest.java
+++ b/test/sun/management/jdp/JdpDefaultsTest.java
@@ -28,7 +28,7 @@
  * @test JdpDefaultsTest
  * @summary Assert that we can read JDP packets from a multicast socket connection, on default IP and port.
  * @library /lib/testlibrary
- * @build ClientConnection JdpTestUtil JdpTestCase JdpOnTestCase DynamicLauncher
+ * @build jdk.testlibrary.* ClientConnection JdpTestUtil JdpTestCase JdpOnTestCase DynamicLauncher
  * @run main JdpDefaultsTest
  */
 
diff --git a/test/sun/management/jdp/JdpOffTest.java b/test/sun/management/jdp/JdpOffTest.java
--- a/test/sun/management/jdp/JdpOffTest.java
+++ b/test/sun/management/jdp/JdpOffTest.java
@@ -29,7 +29,7 @@
  * @test JdpOffTest.java
  * @summary Assert that no JDP packets are sent to the default address and port.
  * @library /lib/testlibrary
- * @build ClientConnection JdpTestUtil JdpTestCase JdpOffTestCase DynamicLauncher
+ * @build jdk.testlibrary.* ClientConnecti

Re: RFR(S): 8043520: Serviceability tests using @library failing with java.lang.NoClassDefFoundError

[Erik Gahlin]

Looks good!

Erik

Yekaterina Kantserova skrev 2014-05-20 15:48:

Thanks Staffan!

New webrev can be found here: 
http://cr.openjdk.java.net/~ykantser/8043520/webrev.01/


// Katja



On 05/20/2014 03:07 PM, Staffan Larsen wrote:

test/sun/management/jmxremote/bootstrap/PasswordFilePermissionTest.java
   “Dummy” is being built twice.

Otherwise good!

/Staffan


On 20 maj 2014, at 14:24, Yekaterina Kantserova 
 wrote:



Staffan, Alan,

could you please review the following fix.

Bug: https://bugs.openjdk.java.net/browse/JDK-8043520
Webrev: http://cr.openjdk.java.net/~ykantser/8043520/webrev.00/

I've missed somehow several tests in 
http://cr.openjdk.java.net/~ykantser/8034960/webrev.01/ which 
existed in http://cr.openjdk.java.net/~ykantser/8034960/webrev.00/.


Thanks,
Katja

RFR JDK-8042887: Remove serialver -show, this tool does not need a GUI

[Pavel Rappo]

Hi everyone,

could you please review my change for JDK-8042887?

http://cr.openjdk.java.net/~alanb/8042887/webrev/

I also created following issues for appropriate docs/localization updates:

https://bugs.openjdk.java.net/browse/JDK-8043613
https://bugs.openjdk.java.net/browse/JDK-8043620

Thanks
-Pavel

RE: RFR: Faster ZipFile.getEntry()/entries()

[Jeroen Frijters]

Hi Sherman,

As a (minor) data point, IKVM.NET has been using a pure Java ZipFile 
implementation since day one (based on the GNU Classpath version) and other 
than a few compat bugs in the early days people have never complained about it.

For obvious reasons, I'd certainly prefer the pure Java version (to minimize 
the amount of work I have to do ;-)), but I've also always thought that it was 
quite a hard sell that the native zip code was faster than pure Java code, 
given the overhead of JNI and the cost of native memory interop/pinning.

Regards,
Jeroen

> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> Behalf Of Xueming Shen
> Sent: Wednesday, May 21, 2014 23:19
> To: core-libs-dev@openjdk.java.net
> Subject: Re: RFR: Faster ZipFile.getEntry()/entries()
> 
> Hi,
> 
> This one didn't make into jdk8. Here is an updated webrev for the jdk9
> repo
> 
> http://cr.openjdk.java.net/~sherman/zipfile_j/webrev
> 
> And a pure java version of j.u.ZipFile is also available at
> 
> http://cr.openjdk.java.net/~sherman/zipfile_jj/webrev/
> 
> We do have incident reports and requests that suggest a pure Java
> version of j.u.ZipFile might be preferred, especially to eliminate the
> possibility of jvm crash at native level, mostly triggered by the mmap
> usage and/or use scenario that the target zip/jar file is being
> overwritten while reading.
> 
> And java implementation also brings in the benefits of better memory
> usage (all memory allocated in java heap), no more expensive jni
> invocations...
> 
> Opinion/comments are appreciated.
> 
> Thanks!
> -Sherman
> 
> 
> On 09/05/2013 04:16 PM, Xueming Shen wrote:
> > Hi,
> >
> > The change proposed here is to bring the zip entry handing code from
> > the native level to the java level. This effectively solves the
> > performance issues of ZipFile.getEntry and entries() that is caused by
> > multiple jni invocation steps to generate one single ZipEntry (see
> > ZipFile.getZipEntry()). A simple non-scientific benchmark test of
> > simply iterating the ZipFile via the Enumeration from entries() on
> rt.jar/charsets.jar suggests a 50%+ speed boost.
> >
> > http://cr.openjdk.java.net/~sherman/zipfile_j/webrev
> >
> > Couple notes:
> >
> > (1) Ideally it might be desired to go further to bring all the native
> > code of ZipFile to java level (which should help completely remove
> > that mmap crash issue, have better file and memory management... ),
> > but it is suggested that it might be better to limit the scope of the
> change at this late release circle.
> >
> > (2) JavaFile.read0() is the version that uses
> > "getPrimitiveArrayCritical" to read file bits into the java array
> > directly (instead of using a stack buffer and then copy into the java
> > array), which appears to be 5% faster. But I can't make up my mind of
> > which one would be better. Given (1) the trouble we had before in
> > De/Infalter code (when the getPrimitiveArrayCritical is being heavily
> used), (2) FileInputStream uses the same "copy" approach, I'm staying
> with the "copy" appraoch, but option appreciated.
> >
> > (3) We will have to keep the native implementation (zip_util.c) for
> > the vm directly access.
> >
> > Thanks!
> > -Sherman
> >