Re: Alternatives for Unsafe field access

[Jochen Theodorou]

On 09.12.2016 08:55, Jochen Theodorou wrote:

On 08.12.2016 22:10, Uwe Schindler wrote:

Hi,

You can first do standard reflection, then use setAccessible


Maybe we have luck here and it does not apply, but if the field comes
from a class of a module and is private, my current level knowledge of
jigsaw says, that setAccessible will fail with an exception. It does not
matter at all if the class with the field is exported or not. That is
because just before the JavaOne #AwkwardStrongEncapsulation made it into
jigsaw. Only way around this is to use the Lookup object that has the
correct rights from the beginning. How to get it... well... there will
be only solutions for certain types of classes.


oh, forgot... it works if the module is made open from the command line 
or declared as such as a work around


bye Jcohen

Re: Alternatives for Unsafe field access

[Jochen Theodorou]

On 08.12.2016 22:10, Uwe Schindler wrote:

Hi,

You can first do standard reflection, then use setAccessible


Maybe we have luck here and it does not apply, but if the field comes 
from a class of a module and is private, my current level knowledge of 
jigsaw says, that setAccessible will fail with an exception. It does not 
matter at all if the class with the field is exported or not. That is 
because just before the JavaOne #AwkwardStrongEncapsulation made it into 
jigsaw. Only way around this is to use the Lookup object that has the 
correct rights from the beginning. How to get it... well... there will 
be only solutions for certain types of classes.


bye Jochen

RE: RFR(M): 8170663: Fix minor issues in corelib and servicabilty coding.

[Lindenmaier, Goetz]

Hi,

I'll post a new webrev once the hs code has been propagated to 
dev.  

Best regards,
  Goetz.

> -Original Message-
> From: David Holmes [mailto:david.hol...@oracle.com]
> Sent: Donnerstag, 8. Dezember 2016 23:03
> To: daniel.daughe...@oracle.com; Lindenmaier, Goetz
> ; 'Dmitry Samersoff'
> ; Java Core Libs  d...@openjdk.java.net>; serviceability-dev (serviceability-
> d...@openjdk.java.net) 
> Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and servicabilty
> coding.
> 
> On 9/12/2016 7:31 AM, Daniel D. Daugherty wrote:
> > On 12/8/16 1:59 PM, David Holmes wrote:
> >> On 9/12/2016 12:21 AM, Lindenmaier, Goetz wrote:
> >>> Hi David,
> >>>
> >>> thanks for looking at the change.  New webrev:
> >>> http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.04/
> >>>
>  src/java.base/share/native/libjli/java.c
>  As far as I can see the existing code is working perfectly fine.
> >>> Ah, thanks for the explanation, now I got it!  Removed.
> >>>
>  Again I'm not sure what the bug is here. On AIX the output string is
>  expanded with:
>  "%s/lib/%s/jli:"
> >>> I first edited this against jdk9/hs, where the arch is gone since
> >>> 8066474,
> >>> http://hg.openjdk.java.net/jdk9/hs/jdk/rev/81508186e5bc
> >>> but the // was not adapted. Then I moved the change to jdk9/dev because
> >>> I thought I have to push it there. And yes, in that coding // would
> >>> be correct.
> >>> So I have to wait until hs is promoted ...
> >>
> >> So just based on the current file, the change proposed - to remove one
> >> / - is not correct until the arch directory is removed.
> >
> > That change is already in JDK9-hs:
> >
> > Changeset: c14f9a7b4cab
> > Author:erikj
> > Date:  2016-12-05 17:55 +0100
> > URL:   http://hg.openjdk.java.net/jdk9/hs/rev/c14f9a7b4cab
> >
> > 8066474: Remove the lib/ directory from Linux and Solaris images
> > Reviewed-by: tbell, ihse
> >
> > along with changesets in the jdk and hotspot repos along with a few
> > closed repos.
> 
> Thanks Dan. So we need to see a webrev based on latest hs code - where
> removing the extra / would be correct.
> 
> David
> -
> 
> > Dan
> >
> >
> >
> >>
> >> David
> >> -
> >>
>  The jvmpath -> jvm_newpath change wasn't really necessary - a
>  comment on
>  the strdup would have sufficed IMO.
> >>> Dmitry asked me to add it.  But I think it's ok.
> >>>
> >>> Can I consider this reviewed now?  I.e. could I push it once 8066474 is
> >>> promoted and Joe Darcy agreed?
> >>>
> >>> Best regards,
> >>>   Goetz.
> >>>
> >>>
>  -Original Message-
>  From: David Holmes [mailto:david.hol...@oracle.com]
>  Sent: Donnerstag, 8. Dezember 2016 09:14
>  To: Lindenmaier, Goetz ; 'Dmitry
> Samersoff'
>  ; Java Core Libs   d...@openjdk.java.net>; serviceability-dev (serviceability-
>  d...@openjdk.java.net) 
>  Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and
>  servicabilty
>  coding.
> 
>  Hi Goetz,
> 
>  On 8/12/2016 1:26 AM, Lindenmaier, Goetz wrote:
> > Hi Dmitry,
> >
> > yes, new_jvmpath is consistent with the other variables.
> > I also merged the ifs in SDE.c.
> >
> > new webrev:
> > http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.03/
> 
>  src/java.base/share/native/libjli/java.c
> 
>  As far as I can see the existing code is working perfectly fine.
>  Given a
>  jvm.cfg with:
> 
>  -server KNOWN
>  -client IGNORE
>  -myvm KNOWN
>  -oldvm ALIASED_TO -server
> 
>  The usage text is:
> 
>   -server   to select the "server" VM
>   -myvm to select the "myvm" VM
>   -oldvmis a synonym for the "server" VM [deprecated]
> The default VM is server,
> because you are running on a server-class machine.
> 
>  which is exactly what I would expect. Why do you think there is a bug?
> 
>  ---
> 
>  src/java.base/unix/native/libjli/java_md_solinux.c
> 
>  Again I'm not sure what the bug is here. On AIX the output string is
>  expanded with:
> 
>  "%s/lib/%s/jli:"
> 
>  so it needs to account for the extra "/lib//jli:" characters - and that
>  is exactly what the existing code does:
> 
>  + JLI_StrLen("/lib//jli:")
> 
>  The jvmpath -> jvm_newpath change wasn't really necessary - a
>  comment on
>  the strdup would have sufficed IMO.
> 
>  Thanks,
>  David
>  -
> 
> 
> 
> 
> > Best regards,
> >   Goetz.
> >
> >> -Original Message-
> >> From: Dmitry Samersoff [mailto:dmitry.samers...@oracle.com]
> >> Sent: Wednesday, December 07, 2016 2:43 PM
> >> To: Lindenmaier, Goetz ; Java Core Libs
> >> ; serviceability-dev (serviceability-
> >> d...@openjdk.java.net) 
> >> Subject: Re: RFR(M): 8170663: Fix m

Re: Alternatives for Unsafe field access

[Remi Forax]

- Mail original -
> De: "Nathan Mittler" 
> À: "Uwe Schindler" 
> Cc: "Daniel Weis" , core-libs-dev@openjdk.java.net, "Louis 
> Ryan" 
> Envoyé: Jeudi 8 Décembre 2016 23:16:36
> Objet: Re: Alternatives for Unsafe field access

> Thanks, everyone!
> 
> Just to be clear: we'll have to support older versions of generated code so
> relying on the generated message to export VarHandles wouldn't really be an
> option.  And, we will have to access private fields of another (generated)
> class from within our runtime.
> 
> I'm a bit concerned with a few potential penalties incurred when using
> VarHandles vs a single contiguous buffer:
> 
> 1) Allocation cost: Allocating a single contiguous buffer would be
> relatively cheap compared to allocating an array of VarHandles. This sort
> of thing could significantly impact the startup time for applications that
> use many message types.

yes, you have to test

> 
> 2) Storage size: There would likely be a lot more storage required per
> message type, since we would have to maintain other metadata in a buffer,
> while also maintaining an array of VarHandles instances. This could blow up
> quickly for apps using lots of messages.

for apps that using a lot of message types

> 
> 3) Runtime performance: I suspect that there will be a performance penalty
> incurred when iterating over an array of VarHandles vs a single continuous
> buffer due to cache behavior.

yes, if you iterate over the array of VarHandle but you can gather all 
varhandles as a single blob,
for each VarHandle, you can convert it to a MethodHandle and fold them together 
after having partially applied the index into the array and the private field,
if you fold them from the last index to the first one, the JIT will do only one 
bound check i believe.

This is basically equivalent to generating you own bytecode for each message 
type.

So instead of having an array of VarHandle, you will have one method handle 
that will do the serialisation.

> 
> Also, do I need to be concerned of setAccessible throwing on certain
> platforms? I've never seen it fail ... perhaps it won't be an issue?

setAccessible will work if the module or the package is declared open.

> 
> Thanks,
> Nathan

regards,
Rémi

> 
> 
> On Thu, Dec 8, 2016 at 1:13 PM, Uwe Schindler  wrote:
> 
>> Hi,
>>
>> Of course, private field access is allowed to your own class if you use a
>> private lookup object. The setAccessible hack is only needed when
>> reflection would also be needed for frameworks or other "superusers".
>>
>> Uwe
>>
>> Am 8. Dezember 2016 22:10:32 MEZ schrieb Uwe Schindler <
>> uschind...@apache.org>:
>> >Hi,
>> >
>> >You can first do standard reflection, then use setAccessible and
>> >finally use the Lookup object to convert the now-accessible Field
>> >instance to a MethodHandle or VarHandle. You just have to know that
>> >way, existing since Java 7.
>> >
>> >Uwe
>> >
>> >Am 8. Dezember 2016 21:49:04 MEZ schrieb Nathan Mittler
>> >:
>> >>Hi Roger,
>> >>If I read that correctly, lookups still have to go through access
>> >>checks
>> >>which would seem to imply that they can't be used for private field
>> >>access.
>> >>Or am I misunderstanding?
>> >>
>> >>On Thu, Dec 8, 2016 at 11:51 AM, Roger Riggs 
>> >>wrote:
>> >>
>> >>> Hi Nathan,
>> >>>
>> >>> Have you looked at VarHandles?  [1]
>> >>> It is possible to use MethodsHandles.Lookup to get a VarHandle to an
>> >>> unreflected field.
>> >>>
>> >>> Roger
>> >>>
>> >>> [1] http://download.java.net/java/jdk9/docs/api/java/lang/invoke
>> >>> /MethodHandles.Lookup.html
>> >>>
>> >>>
>> >>>
>> >>> On 12/8/2016 1:03 PM, Nathan Mittler wrote:
>> >>>
>>  Hi everyone,
>> 
>>  Apologies in advance if this isn't the correct forum for this
>> >>question. My
>>  team is working on an experimental runtime for Google's protocol
>> >>buffers
>>  which is currently relying on sun.misc.Unsafe to perform various
>> >>tasks
>>  efficiently. I'm aware that the plan is to eventually remove Unsafe
>>  altogether, and I want to make sure that we still have a way to
>> >move
>>  forward with future versions of Java.
>> 
>>  The code is up on a github branch (
>>  https://github.com/google/protobuf/tree/java_experimental).
>> 
>>  For an example of the sorts of things our runtime needs to do, you
>> >>can
>>  look
>>  at the serialization code (
>>  https://github.com/google/protobuf/blob/java_experimental/
>> 
>> >>java/core/src/main/java/com/google/protobuf/
>> AbstractProto3Schema.java#L49
>>  ).
>> 
>>  The idea is that the runtime dynamically determines information
>> >>about
>>  message fields (e.g. field types, offsets) and stores it into a
>> >>single
>>  buffer.  Our main need is fast read/write access to the private
>> >>fields of
>>  our generated message classes. Java reflection would be too slow
>> >and
>> >>would
>>  require data representation as a list of objects, rather than a
>> 

JDK 9 RFR of JDK-8023898: Consolidate Map tests Collisions and InPlaceOpsCollisions into general Map-based test

[Amy Lu]

Please review this test refactoring to map test Collisions and 
InPlaceOpsCollisions.


These tests have (almost) same code for creating test data, and the test 
data can be reused cross tests. With converting to testng, test data 
creating is now done and provided by MapWithCollisionsProviders.java, 
and each test now uses explicit data provider, which makes test more clear.


Other changes include:
* Removed the redundant test Collections.testContainsKey and unnecessary 
map size check in the beginning of each test.

* Replaced 'check' with testng assertion method.
* Renaming and reformatting to make it more clear.

bug:https://bugs.openjdk.java.net/browse/JDK-8023898
webrev:http://cr.openjdk.java.net/~amlu/8023898/webrev.01/

Thanks,
Amy

Re: RFR of JDK-7195382: TEST_BUG: java/rmi/activation/CommandEnvironment/SetChildEnv.java can fail

[Hamlin Li]

Is some one available to review the patch?

Thank you
-Hamlin

On 2016/12/8 11:07, Hamlin Li wrote:

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-7195382
webrev: http://cr.openjdk.java.net/~mli/7195382/webrev.00/

what's changed?
* Use createRMIDOnEphemeralPort to avoid "port in use" exception.
* Because createRMIDOnEphemeralPort will choose different ports for 
rmid for multiple runs, it will fail subsequent ActivationGroup except 
of first run, so have to run 4 test cases in different JVM by putting 
them in rather in separate "@run main/othervm".

* Put rmid.cleanup() in finally block to avoid orphan process bug.
* delete dead code.

Thank you
-Hamlin

RFR JDK-8170961: ProblemList tools/jlink/multireleasejar/JLinkMultiReleaseJarTest.java due to JDK-8169971

[John Jiang]

Hi,
This test has failed for several times on Windows x64 platform, it 
should be included by ProblemList.

Please review the below patch.

diff -r e307a1fcbcca test/ProblemList.txt
--- a/test/ProblemList.txtFri Dec 09 02:26:48 2016 +
+++ b/test/ProblemList.txtFri Dec 09 03:00:02 2016 +
@@ -265,6 +265,8 @@
 tools/jimage/JImageListTest.java 8169713 generic-all
 tools/jimage/JImageVerifyTest.java 8169713 generic-all

+tools/jlink/multireleasejar/JLinkMultiReleaseJarTest.java 8169971 
windows-x64

+
 

 # jdk_jdi

Best regards,
John Jiang

Re: Ping - Re: RFR 8170890, Problem list tools/javadoc/CheckResourceKeys.java and tools/javadoc/ReleaseOption.java until fix for JDK-8170772

[Mandy Chung]

I suggest to hold off not to problem list these test failures.

CheckResourceKeys.java is a test bug that Jon is working on a test fix.  We 
wanted to observe if ReleaseOption.java will still fail or not.

It seems that there is some issue with ResourceBundle caching going on here.

Mandy

> On Dec 8, 2016, at 3:58 PM, Felix Yang  wrote:
> 
> Hi all,
> 
>any comment on this problem-list request?
> 
> Thanks,
> Felix
> On 2016/12/8 11:06, Felix Yang wrote:
>> Hi,
>> 
>>   please review the patch to problem-list two tier1 tests from Linux 
>> platform.
>> 
>> Bug:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8170890
>> 
>> Thanks,
>> 
>> Felix
>> 
>> 
>> diff -r 586c93260d3b test/ProblemList.txt
>> --- a/test/ProblemList.txt  Mon Dec 05 15:08:24 2016 -0800
>> +++ b/test/ProblemList.txt  Wed Dec 07 18:47:22 2016 -0800
>> @@ -31,6 +31,8 @@
>> jdk/javadoc/tool/enum/enumType/Main.java 8152313 generic-allconvert to 
>> doclet test framework
>> jdk/javadoc/tool/varArgs/Main.java 8152313generic-all convert to doclet 
>> test framework
>> jdk/javadoc/doclet/testIOException/TestIOException.java 8164597
>> windows-all
>> +tools/javadoc/CheckResourceKeys.java 8170772linux-all
>> +tools/javadoc/ReleaseOption.java 8170772linux-all
>> 
>> ### 
>> #
>> 
> 

RFR JDK-8170952: jar's usage message output need some cleanup

[Xueming Shen]

Hi,

Please help some trival usage msg clean of the jar command

issue: https://bugs.openjdk.java.net/browse/JDK-8170952
webrev: http://cr.openjdk.java.net/~sherman/8170952/



 321 static void printUsageSummary(PrintWriter out) {
 322 out.format("%s%n", Main.getMsg("main.usage.summary"));
 323 out.format("%s%n", Main.getMsg("main.usage.summary.try"));
 324 }

with the original msgs in jar.properties as



 177 main.usage.summary=\
 178 jar: You must specify one of -ctxui options.
 179 main.usage.summary.try=\
 180 Try `jar --help' for more information.

So basic in most use cases, regardless the cause of the parsing err, we always 
have
the following two lines at the bottom of the jar brief usage msgs.

jar: You must specify one of -ctxui options.
Try `jar --help' for more information.

After talked with Chris, I suggested to simply remove the first line "jar: You 
mut
specify ...", IF there is already a error msg (for the real trigger) and the 
"try
jar --help...", for example

jar --list --create foo.jar now outputs


You may not specify more than one '-cuxti' options
Try `jar --help' for more information.


There is use scenario that there is no direct err msg, the general brief usage 
is
print out. For example, if you type "jar" without anything, then output is

Usage: jar [OPTION...] [ [--release VERSION] [-C dir] files] ...
Try `jar --help' for more information.

which I think is more appropriate. I'm copy/pasting the "Usage: ..." from the
first line of  main.help.preopt. Hope it should be OK for the l10n team.

Thanks,
Sherman

Ping - Re: RFR 8170890, Problem list tools/javadoc/CheckResourceKeys.java and tools/javadoc/ReleaseOption.java until fix for JDK-8170772

[Felix Yang]

Hi all,

any comment on this problem-list request?

Thanks,
Felix
On 2016/12/8 11:06, Felix Yang wrote:

Hi,

   please review the patch to problem-list two tier1 tests from Linux 
platform.


Bug:

 https://bugs.openjdk.java.net/browse/JDK-8170890

Thanks,

Felix


diff -r 586c93260d3b test/ProblemList.txt
--- a/test/ProblemList.txt  Mon Dec 05 15:08:24 2016 -0800
+++ b/test/ProblemList.txt  Wed Dec 07 18:47:22 2016 -0800
@@ -31,6 +31,8 @@
 jdk/javadoc/tool/enum/enumType/Main.java 8152313 generic-all
convert to doclet test framework
 jdk/javadoc/tool/varArgs/Main.java 8152313generic-all convert to 
doclet test framework
 jdk/javadoc/doclet/testIOException/TestIOException.java 8164597
windows-all

+tools/javadoc/CheckResourceKeys.java 8170772linux-all
+tools/javadoc/ReleaseOption.java 8170772linux-all

 ### 


 #

Re: RFR 9: 8153882 rmid emits warning about security policy with obsolete URL

[Stuart Marks]

Hi Roger,

Change looks good. Thanks.

s'marks

On 12/7/16 1:20 PM, Roger Riggs wrote:

Please review a change to an warning message from rmid when the security policy
is incorrectly configured.
There is no reliable link to the rmid documentation that will remain accurate
across JDK versions.
The direct link is removed and expect the user to find the rmid documentation
from the doc bundle or
via a web search.

(this corrects the US english version, other locale will require translation)

Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-rmid-url-8153882/

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

Thanks, Roger

RFR(s): JDK-8170943 Collectors.partitioningBy should specify that false and true entries are always present

[Stuart Marks]

Hi all,

Please review this small spec change to the Collectors.partitioningBy() methods, 
appended below. This collector returns Map. The change 
requires the implementation always to populate the returned map with entries for 
both false and true keys.


The implementation already does this; the entries are initially populated with 
values obtained by calling the supplier of the downstream collector.


The reason for this spec change that it's useful for applications to be able to 
rely on this. The spec is currently silent about what entries will be present in 
the map. If one or the other (or both!) of the entries can be absent, callers 
would have to defend against this possibility. For example,


Map> map = stream.collect(partitioningBy(pred));
processResults(map.getOrDefault(false, List.of()),
   map.getOrDefault(true, List.of()));

With the guarantee, it's safe instead to write:

Map> map = stream.collect(partitioningBy(pred));
processResults(map.get(false), map.get(true));

In fact, this code works today, it's just that it's not specified to work. It 
should be.


Thanks,

s'marks



diff -r 7583c87dfe7c 
src/java.base/share/classes/java/util/stream/Collectors.java
--- a/src/java.base/share/classes/java/util/stream/Collectors.java	Thu Dec 08 
21:21:39 2016 +
+++ b/src/java.base/share/classes/java/util/stream/Collectors.java	Thu Dec 08 
14:39:31 2016 -0800

@@ -1268,6 +1268,8 @@
  * to a {@code Predicate}, and organizes them into a
  * {@code Map>}.
  *
+ * The returned {@code Map} always contains mappings for both
+ * {@code false} and {@code true} keys.
  * There are no guarantees on the type, mutability,
  * serializability, or thread-safety of the {@code Map} or {@code List}
  * returned.
@@ -1290,7 +1292,10 @@
  * {@code Map} whose values are the result of the downstream
  * reduction.
  *
- * There are no guarantees on the type, mutability,
+ * 
+ * The returned {@code Map} always contains mappings for both
+ * {@code false} and {@code true} keys.
+ * There are no guarantees on the type, mutability,
  * serializability, or thread-safety of the {@code Map} returned.
  *
  * @param  the type of the input elements

Re: RFR(M): 8170663: Fix minor issues in corelib and servicabilty coding.

[David Holmes]

On 9/12/2016 7:31 AM, Daniel D. Daugherty wrote:

On 12/8/16 1:59 PM, David Holmes wrote:

On 9/12/2016 12:21 AM, Lindenmaier, Goetz wrote:

Hi David,

thanks for looking at the change.  New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.04/


src/java.base/share/native/libjli/java.c
As far as I can see the existing code is working perfectly fine.

Ah, thanks for the explanation, now I got it!  Removed.


Again I'm not sure what the bug is here. On AIX the output string is
expanded with:
"%s/lib/%s/jli:"

I first edited this against jdk9/hs, where the arch is gone since
8066474,
http://hg.openjdk.java.net/jdk9/hs/jdk/rev/81508186e5bc
but the // was not adapted. Then I moved the change to jdk9/dev because
I thought I have to push it there. And yes, in that coding // would
be correct.
So I have to wait until hs is promoted ...


So just based on the current file, the change proposed - to remove one
/ - is not correct until the arch directory is removed.


That change is already in JDK9-hs:

Changeset: c14f9a7b4cab
Author:erikj
Date:  2016-12-05 17:55 +0100
URL:   http://hg.openjdk.java.net/jdk9/hs/rev/c14f9a7b4cab

8066474: Remove the lib/ directory from Linux and Solaris images
Reviewed-by: tbell, ihse

along with changesets in the jdk and hotspot repos along with a few
closed repos.


Thanks Dan. So we need to see a webrev based on latest hs code - where 
removing the extra / would be correct.


David
-


Dan





David
-


The jvmpath -> jvm_newpath change wasn't really necessary - a
comment on
the strdup would have sufficed IMO.

Dmitry asked me to add it.  But I think it's ok.

Can I consider this reviewed now?  I.e. could I push it once 8066474 is
promoted and Joe Darcy agreed?

Best regards,
  Goetz.



-Original Message-
From: David Holmes [mailto:david.hol...@oracle.com]
Sent: Donnerstag, 8. Dezember 2016 09:14
To: Lindenmaier, Goetz ; 'Dmitry Samersoff'
; Java Core Libs ; serviceability-dev (serviceability-
d...@openjdk.java.net) 
Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and
servicabilty
coding.

Hi Goetz,

On 8/12/2016 1:26 AM, Lindenmaier, Goetz wrote:

Hi Dmitry,

yes, new_jvmpath is consistent with the other variables.
I also merged the ifs in SDE.c.

new webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.03/


src/java.base/share/native/libjli/java.c

As far as I can see the existing code is working perfectly fine.
Given a
jvm.cfg with:

-server KNOWN
-client IGNORE
-myvm KNOWN
-oldvm ALIASED_TO -server

The usage text is:

 -server   to select the "server" VM
 -myvm to select the "myvm" VM
 -oldvmis a synonym for the "server" VM [deprecated]
   The default VM is server,
   because you are running on a server-class machine.

which is exactly what I would expect. Why do you think there is a bug?

---

src/java.base/unix/native/libjli/java_md_solinux.c

Again I'm not sure what the bug is here. On AIX the output string is
expanded with:

"%s/lib/%s/jli:"

so it needs to account for the extra "/lib//jli:" characters - and that
is exactly what the existing code does:

+ JLI_StrLen("/lib//jli:")

The jvmpath -> jvm_newpath change wasn't really necessary - a
comment on
the strdup would have sufficed IMO.

Thanks,
David
-





Best regards,
  Goetz.


-Original Message-
From: Dmitry Samersoff [mailto:dmitry.samers...@oracle.com]
Sent: Wednesday, December 07, 2016 2:43 PM
To: Lindenmaier, Goetz ; Java Core Libs
; serviceability-dev (serviceability-
d...@openjdk.java.net) 
Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and
servicabilty
coding.

Goetz,

SDE.c:

You might combine if at ll. 260 and 263 to one but it's just
matter of test.

 if (sti == baseStratumIndex || sti < 0) {
return; /* Java stratum - return unchanged */
 }


I'm not sure what you mean. I tried to fix it, but please
double-check the new webrev.


if cnt is <= 0 loop at l.267

for (; cnt-- > 0; ++fromEntry) {

is never run and we effectively do

 *entryCountPtr = 0;

at l.283

So if we you suspect that cnt may become negative or 0:
(your v.01 changes)

 260 if (sti < 0 && cnt > 0) {
 261 return;
 262 }

it's better to check it early.

But I'm not sure we have to care about negative/zero cnt here.

-Dmitry

On 2016-12-07 11:37, Lindenmaier, Goetz wrote:

Hi Dmitry,

thanks for looking at my change!
Updated webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.02


* src/java.base/unix/native/libjli/java_md_solinux.c
Is this line correct?
519 jvmpath = JLI_StringDup(jvmpath);


It seems pointless. Should I remove it?  (The whole file is a
horror.)


* src/jdk.jdwp.agent/share/native/libjdwp/SDE.c
It might be better to return immediately if cnt < 0,
regardless of value of sti.


I'm not sure what you mean. I tried to fix it, but please
double-check the new webrev.


* src/jdk.jdwp.agent/unix

Re: RFR(M): 8170663: Fix minor issues in corelib and servicabilty coding.

[Daniel D. Daugherty]

On 12/8/16 1:59 PM, David Holmes wrote:

On 9/12/2016 12:21 AM, Lindenmaier, Goetz wrote:

Hi David,

thanks for looking at the change.  New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.04/


src/java.base/share/native/libjli/java.c
As far as I can see the existing code is working perfectly fine.

Ah, thanks for the explanation, now I got it!  Removed.


Again I'm not sure what the bug is here. On AIX the output string is
expanded with:
"%s/lib/%s/jli:"
I first edited this against jdk9/hs, where the arch is gone since 
8066474,

http://hg.openjdk.java.net/jdk9/hs/jdk/rev/81508186e5bc
but the // was not adapted. Then I moved the change to jdk9/dev because
I thought I have to push it there. And yes, in that coding // would 
be correct.

So I have to wait until hs is promoted ...


So just based on the current file, the change proposed - to remove one 
/ - is not correct until the arch directory is removed.


That change is already in JDK9-hs:

Changeset: c14f9a7b4cab
Author:erikj
Date:  2016-12-05 17:55 +0100
URL:   http://hg.openjdk.java.net/jdk9/hs/rev/c14f9a7b4cab

8066474: Remove the lib/ directory from Linux and Solaris images
Reviewed-by: tbell, ihse

along with changesets in the jdk and hotspot repos along with a few
closed repos.

Dan





David
-

The jvmpath -> jvm_newpath change wasn't really necessary - a 
comment on

the strdup would have sufficed IMO.

Dmitry asked me to add it.  But I think it's ok.

Can I consider this reviewed now?  I.e. could I push it once 8066474 is
promoted and Joe Darcy agreed?

Best regards,
  Goetz.



-Original Message-
From: David Holmes [mailto:david.hol...@oracle.com]
Sent: Donnerstag, 8. Dezember 2016 09:14
To: Lindenmaier, Goetz ; 'Dmitry Samersoff'
; Java Core Libs ; serviceability-dev (serviceability-
d...@openjdk.java.net) 
Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and 
servicabilty

coding.

Hi Goetz,

On 8/12/2016 1:26 AM, Lindenmaier, Goetz wrote:

Hi Dmitry,

yes, new_jvmpath is consistent with the other variables.
I also merged the ifs in SDE.c.

new webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.03/


src/java.base/share/native/libjli/java.c

As far as I can see the existing code is working perfectly fine. 
Given a

jvm.cfg with:

-server KNOWN
-client IGNORE
-myvm KNOWN
-oldvm ALIASED_TO -server

The usage text is:

 -server   to select the "server" VM
 -myvm to select the "myvm" VM
 -oldvmis a synonym for the "server" VM [deprecated]
   The default VM is server,
   because you are running on a server-class machine.

which is exactly what I would expect. Why do you think there is a bug?

---

src/java.base/unix/native/libjli/java_md_solinux.c

Again I'm not sure what the bug is here. On AIX the output string is
expanded with:

"%s/lib/%s/jli:"

so it needs to account for the extra "/lib//jli:" characters - and that
is exactly what the existing code does:

+ JLI_StrLen("/lib//jli:")

The jvmpath -> jvm_newpath change wasn't really necessary - a 
comment on

the strdup would have sufficed IMO.

Thanks,
David
-





Best regards,
  Goetz.


-Original Message-
From: Dmitry Samersoff [mailto:dmitry.samers...@oracle.com]
Sent: Wednesday, December 07, 2016 2:43 PM
To: Lindenmaier, Goetz ; Java Core Libs
; serviceability-dev (serviceability-
d...@openjdk.java.net) 
Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and 
servicabilty

coding.

Goetz,

SDE.c:

You might combine if at ll. 260 and 263 to one but it's just 
matter of test.


 if (sti == baseStratumIndex || sti < 0) {
return; /* Java stratum - return unchanged */
 }


I'm not sure what you mean. I tried to fix it, but please
double-check the new webrev.


if cnt is <= 0 loop at l.267

for (; cnt-- > 0; ++fromEntry) {

is never run and we effectively do

 *entryCountPtr = 0;

at l.283

So if we you suspect that cnt may become negative or 0:
(your v.01 changes)

 260 if (sti < 0 && cnt > 0) {
 261 return;
 262 }

it's better to check it early.

But I'm not sure we have to care about negative/zero cnt here.

-Dmitry

On 2016-12-07 11:37, Lindenmaier, Goetz wrote:

Hi Dmitry,

thanks for looking at my change!
Updated webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.02


* src/java.base/unix/native/libjli/java_md_solinux.c
Is this line correct?
519 jvmpath = JLI_StringDup(jvmpath);


It seems pointless. Should I remove it?  (The whole file is a 
horror.)



* src/jdk.jdwp.agent/share/native/libjdwp/SDE.c
It might be better to return immediately if cnt < 0,
regardless of value of sti.


I'm not sure what you mean. I tried to fix it, but please
double-check the new webrev.


* src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c
It might be better to write
  arg.l_linger = (on) ? (unsigned short)value.i : 0;
and leave one copy of setsockopt() call


Yes, 

Re: Alternatives for Unsafe field access

[Uwe Schindler]

Hi,

Of course, private field access is allowed to your own class if you use a 
private lookup object. The setAccessible hack is only needed when reflection 
would also be needed for frameworks or other "superusers".

Uwe

Am 8. Dezember 2016 22:10:32 MEZ schrieb Uwe Schindler :
>Hi,
>
>You can first do standard reflection, then use setAccessible and
>finally use the Lookup object to convert the now-accessible Field
>instance to a MethodHandle or VarHandle. You just have to know that
>way, existing since Java 7.
>
>Uwe
>
>Am 8. Dezember 2016 21:49:04 MEZ schrieb Nathan Mittler
>:
>>Hi Roger,
>>If I read that correctly, lookups still have to go through access
>>checks
>>which would seem to imply that they can't be used for private field
>>access.
>>Or am I misunderstanding?
>>
>>On Thu, Dec 8, 2016 at 11:51 AM, Roger Riggs 
>>wrote:
>>
>>> Hi Nathan,
>>>
>>> Have you looked at VarHandles?  [1]
>>> It is possible to use MethodsHandles.Lookup to get a VarHandle to an
>>> unreflected field.
>>>
>>> Roger
>>>
>>> [1] http://download.java.net/java/jdk9/docs/api/java/lang/invoke
>>> /MethodHandles.Lookup.html
>>>
>>>
>>>
>>> On 12/8/2016 1:03 PM, Nathan Mittler wrote:
>>>
 Hi everyone,

 Apologies in advance if this isn't the correct forum for this
>>question. My
 team is working on an experimental runtime for Google's protocol
>>buffers
 which is currently relying on sun.misc.Unsafe to perform various
>>tasks
 efficiently. I'm aware that the plan is to eventually remove Unsafe
 altogether, and I want to make sure that we still have a way to
>move
 forward with future versions of Java.

 The code is up on a github branch (
 https://github.com/google/protobuf/tree/java_experimental).

 For an example of the sorts of things our runtime needs to do, you
>>can
 look
 at the serialization code (
 https://github.com/google/protobuf/blob/java_experimental/

>>java/core/src/main/java/com/google/protobuf/AbstractProto3Schema.java#L49
 ).

 The idea is that the runtime dynamically determines information
>>about
 message fields (e.g. field types, offsets) and stores it into a
>>single
 buffer.  Our main need is fast read/write access to the private
>>fields of
 our generated message classes. Java reflection would be too slow
>and
>>would
 require data representation as a list of objects, rather than a
>>continuous
 buffer (much less compact and cache-friendly). Security
>restrictions
>>would
 be a concern as well. At first glance at Java 9, I don't see any
 facilities
 that would help here.

 This would seem to be a fairly common use case a low-level
>>serialization
 framework, such as protobuf. Are there any thoughts on how such a
>>use case
 could be supported post-Unsafe?

 Thanks,
 Nathan

>>>
>>>
>
>--
>Uwe Schindler
>Achterdiek 19, 28357 Bremen
>https://www.thetaphi.de

Re: Alternatives for Unsafe field access

[Uwe Schindler]

Hi,

You can first do standard reflection, then use setAccessible and finally use 
the Lookup object to convert the now-accessible Field instance to a 
MethodHandle or VarHandle. You just have to know that way, existing since Java 
7.

Uwe

Am 8. Dezember 2016 21:49:04 MEZ schrieb Nathan Mittler 
:
>Hi Roger,
>If I read that correctly, lookups still have to go through access
>checks
>which would seem to imply that they can't be used for private field
>access.
>Or am I misunderstanding?
>
>On Thu, Dec 8, 2016 at 11:51 AM, Roger Riggs 
>wrote:
>
>> Hi Nathan,
>>
>> Have you looked at VarHandles?  [1]
>> It is possible to use MethodsHandles.Lookup to get a VarHandle to an
>> unreflected field.
>>
>> Roger
>>
>> [1] http://download.java.net/java/jdk9/docs/api/java/lang/invoke
>> /MethodHandles.Lookup.html
>>
>>
>>
>> On 12/8/2016 1:03 PM, Nathan Mittler wrote:
>>
>>> Hi everyone,
>>>
>>> Apologies in advance if this isn't the correct forum for this
>question. My
>>> team is working on an experimental runtime for Google's protocol
>buffers
>>> which is currently relying on sun.misc.Unsafe to perform various
>tasks
>>> efficiently. I'm aware that the plan is to eventually remove Unsafe
>>> altogether, and I want to make sure that we still have a way to move
>>> forward with future versions of Java.
>>>
>>> The code is up on a github branch (
>>> https://github.com/google/protobuf/tree/java_experimental).
>>>
>>> For an example of the sorts of things our runtime needs to do, you
>can
>>> look
>>> at the serialization code (
>>> https://github.com/google/protobuf/blob/java_experimental/
>>>
>java/core/src/main/java/com/google/protobuf/AbstractProto3Schema.java#L49
>>> ).
>>>
>>> The idea is that the runtime dynamically determines information
>about
>>> message fields (e.g. field types, offsets) and stores it into a
>single
>>> buffer.  Our main need is fast read/write access to the private
>fields of
>>> our generated message classes. Java reflection would be too slow and
>would
>>> require data representation as a list of objects, rather than a
>continuous
>>> buffer (much less compact and cache-friendly). Security restrictions
>would
>>> be a concern as well. At first glance at Java 9, I don't see any
>>> facilities
>>> that would help here.
>>>
>>> This would seem to be a fairly common use case a low-level
>serialization
>>> framework, such as protobuf. Are there any thoughts on how such a
>use case
>>> could be supported post-Unsafe?
>>>
>>> Thanks,
>>> Nathan
>>>
>>
>>

--
Uwe Schindler
Achterdiek 19, 28357 Bremen
https://www.thetaphi.de

Re: Alternatives for Unsafe field access

[John Rose]

They support private fields. Like MethodHandles the access check is performed 
at creation time not invocation time. Any class can arrange to make VHs and MHs 
on its own privates. There will also be super user access for frameworks. 

– John

> On Dec 8, 2016, at 12:49 PM, Nathan Mittler  wrote:
> 
> Hi Roger,
> If I read that correctly, lookups still have to go through access checks
> which would seem to imply that they can't be used for private field access.
> Or am I misunderstanding?
> 
>> On Thu, Dec 8, 2016 at 11:51 AM, Roger Riggs  wrote:
>> 
>> Hi Nathan,
>> 
>> Have you looked at VarHandles?  [1]
>> It is possible to use MethodsHandles.Lookup to get a VarHandle to an
>> unreflected field.
>> 
>> Roger
>> 
>> [1] http://download.java.net/java/jdk9/docs/api/java/lang/invoke
>> /MethodHandles.Lookup.html
>> 
>> 
>> 
>>> On 12/8/2016 1:03 PM, Nathan Mittler wrote:
>>> 
>>> Hi everyone,
>>> 
>>> Apologies in advance if this isn't the correct forum for this question. My
>>> team is working on an experimental runtime for Google's protocol buffers
>>> which is currently relying on sun.misc.Unsafe to perform various tasks
>>> efficiently. I'm aware that the plan is to eventually remove Unsafe
>>> altogether, and I want to make sure that we still have a way to move
>>> forward with future versions of Java.
>>> 
>>> The code is up on a github branch (
>>> https://github.com/google/protobuf/tree/java_experimental).
>>> 
>>> For an example of the sorts of things our runtime needs to do, you can
>>> look
>>> at the serialization code (
>>> https://github.com/google/protobuf/blob/java_experimental/
>>> java/core/src/main/java/com/google/protobuf/AbstractProto3Schema.java#L49
>>> ).
>>> 
>>> The idea is that the runtime dynamically determines information about
>>> message fields (e.g. field types, offsets) and stores it into a single
>>> buffer.  Our main need is fast read/write access to the private fields of
>>> our generated message classes. Java reflection would be too slow and would
>>> require data representation as a list of objects, rather than a continuous
>>> buffer (much less compact and cache-friendly). Security restrictions would
>>> be a concern as well. At first glance at Java 9, I don't see any
>>> facilities
>>> that would help here.
>>> 
>>> This would seem to be a fairly common use case a low-level serialization
>>> framework, such as protobuf. Are there any thoughts on how such a use case
>>> could be supported post-Unsafe?
>>> 
>>> Thanks,
>>> Nathan
>>> 
>> 
>> 

Re: RFR(M): 8170663: Fix minor issues in corelib and servicabilty coding.

[David Holmes]

On 9/12/2016 12:21 AM, Lindenmaier, Goetz wrote:

Hi David,

thanks for looking at the change.  New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.04/


src/java.base/share/native/libjli/java.c
As far as I can see the existing code is working perfectly fine.

Ah, thanks for the explanation, now I got it!  Removed.


Again I'm not sure what the bug is here. On AIX the output string is
expanded with:
"%s/lib/%s/jli:"

I first edited this against jdk9/hs, where the arch is gone since 8066474,
http://hg.openjdk.java.net/jdk9/hs/jdk/rev/81508186e5bc
but the // was not adapted. Then I moved the change to jdk9/dev because
I thought I have to push it there. And yes, in that coding // would be correct.
So I have to wait until hs is promoted ...


So just based on the current file, the change proposed - to remove one / 
- is not correct until the arch directory is removed.


David
-


The jvmpath -> jvm_newpath change wasn't really necessary - a comment on
the strdup would have sufficed IMO.

Dmitry asked me to add it.  But I think it's ok.

Can I consider this reviewed now?  I.e. could I push it once 8066474 is
promoted and Joe Darcy agreed?

Best regards,
  Goetz.



-Original Message-
From: David Holmes [mailto:david.hol...@oracle.com]
Sent: Donnerstag, 8. Dezember 2016 09:14
To: Lindenmaier, Goetz ; 'Dmitry Samersoff'
; Java Core Libs ; serviceability-dev (serviceability-
d...@openjdk.java.net) 
Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and servicabilty
coding.

Hi Goetz,

On 8/12/2016 1:26 AM, Lindenmaier, Goetz wrote:

Hi Dmitry,

yes, new_jvmpath is consistent with the other variables.
I also merged the ifs in SDE.c.

new webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.03/


src/java.base/share/native/libjli/java.c

As far as I can see the existing code is working perfectly fine. Given a
jvm.cfg with:

-server KNOWN
-client IGNORE
-myvm KNOWN
-oldvm ALIASED_TO -server

The usage text is:

 -server   to select the "server" VM
 -myvm to select the "myvm" VM
 -oldvmis a synonym for the "server" VM  [deprecated]
   The default VM is server,
   because you are running on a server-class machine.

which is exactly what I would expect. Why do you think there is a bug?

---

src/java.base/unix/native/libjli/java_md_solinux.c

Again I'm not sure what the bug is here. On AIX the output string is
expanded with:

"%s/lib/%s/jli:"

so it needs to account for the extra "/lib//jli:" characters - and that
is exactly what the existing code does:

+ JLI_StrLen("/lib//jli:")

The jvmpath -> jvm_newpath change wasn't really necessary - a comment on
the strdup would have sufficed IMO.

Thanks,
David
-





Best regards,
  Goetz.


-Original Message-
From: Dmitry Samersoff [mailto:dmitry.samers...@oracle.com]
Sent: Wednesday, December 07, 2016 2:43 PM
To: Lindenmaier, Goetz ; Java Core Libs
; serviceability-dev (serviceability-
d...@openjdk.java.net) 
Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and servicabilty
coding.

Goetz,

SDE.c:

You might combine if at ll. 260 and 263 to one but it's just matter of test.

 if (sti == baseStratumIndex || sti < 0) {
return; /* Java stratum - return unchanged */
 }


I'm not sure what you mean. I tried to fix it, but please
double-check the new webrev.


if cnt is <= 0 loop at l.267

for (; cnt-- > 0; ++fromEntry) {

is never run and we effectively do

 *entryCountPtr = 0;

at l.283

So if we you suspect that cnt may become negative or 0:
(your v.01 changes)

 260 if (sti < 0 && cnt > 0) {
 261 return;
 262 }

it's better to check it early.

But I'm not sure we have to care about negative/zero cnt here.

-Dmitry

On 2016-12-07 11:37, Lindenmaier, Goetz wrote:

Hi Dmitry,

thanks for looking at my change!
Updated webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.02


* src/java.base/unix/native/libjli/java_md_solinux.c
Is this line correct?
519 jvmpath = JLI_StringDup(jvmpath);


It seems pointless. Should I remove it?  (The whole file is a horror.)


* src/jdk.jdwp.agent/share/native/libjdwp/SDE.c
It might be better to return immediately if cnt < 0,
regardless of value of sti.


I'm not sure what you mean. I tried to fix it, but please
double-check the new webrev.


* src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c
It might be better to write
  arg.l_linger = (on) ? (unsigned short)value.i : 0;
and leave one copy of setsockopt() call


Yes, looks better.

Best regards,
  Goetz




-Dmitry


On 2016-12-06 16:12, Lindenmaier, Goetz wrote:

Hi,



This change fixes some minor issues found in our code scans.

I hope this correctly addresses corelib and serviceability issues.



Please review:

http://cr.openjdk.java.net/~goetz/wr16/8170663-

corlib_s11y/webrev.01/




Best regards,

  Goetz.



Changes in detail:



e_asin.c

Code scan reports missing

Re: [9] RFR 8170769: Provide a simple hexdump facility for binary data

[Paul Sandoz]

Hi,

It may take a few iterations to get the API settled.

There are other byte sources we may want to consider such as InputStream and 
ByteBuffer.

Comments below.

Paul.

> On 7 Dec 2016, at 08:32, Vincent Ryan  wrote:
> 
> A hexdump facility has been available for many, many years via an unsupported 
> class: sun.misc.HexDumpEncoder.
> Although that class was always unsupported, it was still accessible. That 
> accessibility changes with Jigsaw so I’m proposing
> a very simple replacement in a new and supported class: java.util.HexDump.
> 
> Thanks.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8170769
> Webrev: http://cr.openjdk.java.net/~vinnie/8170769/webrev.00/
> 

  55 if (bytes == null) {
  56 throw new NullPointerException();
  57 }

Objects.requireNonNull


  78  * @throws IllegalArgumentException if {@code hexString} has an odd 
number
  79  * of digits
  80  * @throws NullPointerException if {@code hexString} is {@code null}
  81  */
  82 public static byte[] fromHexString(String hexString) {

This should accept a CharSequence, plus also have bounded range equivalent.

Also throws IAE if the hexString contains non-convertible characters.


 106 /**
 107  * Generates a stream of hexadecimal strings, in 16-byte chunks,
 108  * from the contents of a binary buffer.
 109  *
 110  * @param bytes a binary buffer
 111  * @return a stream of hexadecimal strings
 112  * @throws NullPointerException if {@code bytes} is {@code null}
 113  */
 114 public static Stream dumpToStream(byte[] bytes) {
 115 if (bytes == null) {
 116 throw new NullPointerException();
 117 }
 118 return StreamSupport.stream(
 119 Spliterators.spliteratorUnknownSize(
 120 new HexDumpIterator(bytes),
 121 Spliterator.ORDERED | Spliterator.NONNULL),
 122 false);
 123 }

I suspect there is no need for a HexDumpIterator and you can do:

  return IntStream.range(0, roundUp(bytes.length, 16)).mapToObject(…);

and in the map function take care of the trailing bytes based on the byte[] 
length. That’s potentially simple enough there might be no need for a stream  
method. Where it gets more complicated is if the source is of unknown size such 
as InputStream, where the stream returning method probably has more value.

Ideally what we want to return is Stream<[Long, String]>, then it’s really easy 
for others for format, but alas the current form would be ugly and inefficient. 
So i suspect the dump method has some legs but it’s real value may be for a 
fully streaming solution.


 140 public static String dump(byte[] bytes) {
 141 if (bytes == null) {
 142 throw new NullPointerException();
 143 }
 144
 145 int[] count = { -16 };
 146 return dumpToStream(bytes).collect(Collector.of(
 147 StringBuilder::new,
 148 (stringBuilder, hexString) ->
 149 stringBuilder
 150 .append(String.format("%08x  %s%n",
 151 (count[0] += CHUNK_LENGTH),
 152 explode(hexString))),
 153 StringBuilder::append,
 154 StringBuilder::toString));
 155 }

The encoding of the count and exploding could also append directly into the 
main string builder, reducing memory churn.

And i think you can also start from IntStream.range(0, roundUp(bytes.length, 
16)) avoiding the count state.


 192 if (b < ' ' || b > '~') {
 193 sb.append(".”);

Use ‘.’


 194 } else {
 195 sb.append(new String(new byte[]{ b }));

Cast the byte to a char instead.


 196 }

Re: Alternatives for Unsafe field access

[Roger Riggs]

Hi Nathan,

Have you looked at VarHandles?  [1]
It is possible to use MethodsHandles.Lookup to get a VarHandle to an 
unreflected field.


Roger

[1] 
http://download.java.net/java/jdk9/docs/api/java/lang/invoke/MethodHandles.Lookup.html



On 12/8/2016 1:03 PM, Nathan Mittler wrote:

Hi everyone,

Apologies in advance if this isn't the correct forum for this question. My
team is working on an experimental runtime for Google's protocol buffers
which is currently relying on sun.misc.Unsafe to perform various tasks
efficiently. I'm aware that the plan is to eventually remove Unsafe
altogether, and I want to make sure that we still have a way to move
forward with future versions of Java.

The code is up on a github branch (
https://github.com/google/protobuf/tree/java_experimental).

For an example of the sorts of things our runtime needs to do, you can look
at the serialization code (
https://github.com/google/protobuf/blob/java_experimental/java/core/src/main/java/com/google/protobuf/AbstractProto3Schema.java#L49
).

The idea is that the runtime dynamically determines information about
message fields (e.g. field types, offsets) and stores it into a single
buffer.  Our main need is fast read/write access to the private fields of
our generated message classes. Java reflection would be too slow and would
require data representation as a list of objects, rather than a continuous
buffer (much less compact and cache-friendly). Security restrictions would
be a concern as well. At first glance at Java 9, I don't see any facilities
that would help here.

This would seem to be a fairly common use case a low-level serialization
framework, such as protobuf. Are there any thoughts on how such a use case
could be supported post-Unsafe?

Thanks,
Nathan

Re: [9] RFR 8170769: Provide a simple hexdump facility for binary data

[Alan Bateman]

On 07/12/2016 16:32, Vincent Ryan wrote:


A hexdump facility has been available for many, many years via an unsupported 
class: sun.misc.HexDumpEncoder.
Although that class was always unsupported, it was still accessible. That 
accessibility changes with Jigsaw so I’m proposing
a very simple replacement in a new and supported class: java.util.HexDump.

Thanks.

Bug: https://bugs.openjdk.java.net/browse/JDK-8170769
Webrev: http://cr.openjdk.java.net/~vinnie/8170769/webrev.00/
I'm happy to see a proposal for an API in Java SE for this, it's always 
sad to bump into code that is using a utility method exposed in 
java.xml.bind or the JDK internal hex dumper.


I've skimmed the API. The class name and package look okay. I assume you 
can make HexDump final (I realize the ctor is private).


The toHexString/fromHexString methods are really useful but I think the 
javadoc needs to be fleshed out a bit. For example, fromHexString 
doesn't make it clear whether the A-F need to be in a specific case, the 
IAE description doesn't make it clear that it is thrown when there is 
invalid input. The description of toHexString can be clearer on the 
length of the returned string, the reader might wonder if there is a 
"0x" prepended for example.


I like dump(byte[]) but I could imagine calls to "customize" the 
returned string in many ways. After reading dumpToStream then I wonder 
if it might be better to drop dump and let users do their own layout. If 
you keep it then the javadoc needs work - the reader will have many 
questions. What is "non-printable", what is the character in the output 
that splits the lines, how is the last line handled when the input is 
not a multiple of 16 bytes. Given dumpToStream then I wonder if might be 
better to drop this method.


dumpToStream(byte[]) is interesting too but again needs a lot more 
javadoc. I assume the spliterator will be replaced eventually as the 
size is known so you can do a good implementation.


It would be useful to put in some @see references from methods like 
Integer.toHexString.


I don't have comments on the implementation/test at this time.

-Alan.

Re: RFR: 8054214: JapaneseEra.getDisplayName doesn't return names if it's an additional era

[Naoto Sato]

Okutsu-san,

In JapaneseEra::getDisplayName, Should there be Objects.requireNonNull() 
check for "style"? The current impl seems to return abbreviated era if 
null is passed as "style".


Naoto

On 12/8/16 12:55 AM, Masayoshi Okutsu wrote:

Hi,

Please review the fix for JDK-8054214. It was necessary to override
Era::getDisplayName to get era names from the specified property. This
one fixes getAbbreviation() as well.

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

Webrev:
http://cr.openjdk.java.net/~okutsu/9/8054214/webrev.00/

Thanks,
Masayoshi

Alternatives for Unsafe field access

[Nathan Mittler]

Hi everyone,

Apologies in advance if this isn't the correct forum for this question. My
team is working on an experimental runtime for Google's protocol buffers
which is currently relying on sun.misc.Unsafe to perform various tasks
efficiently. I'm aware that the plan is to eventually remove Unsafe
altogether, and I want to make sure that we still have a way to move
forward with future versions of Java.

The code is up on a github branch (
https://github.com/google/protobuf/tree/java_experimental).

For an example of the sorts of things our runtime needs to do, you can look
at the serialization code (
https://github.com/google/protobuf/blob/java_experimental/java/core/src/main/java/com/google/protobuf/AbstractProto3Schema.java#L49
).

The idea is that the runtime dynamically determines information about
message fields (e.g. field types, offsets) and stores it into a single
buffer.  Our main need is fast read/write access to the private fields of
our generated message classes. Java reflection would be too slow and would
require data representation as a list of objects, rather than a continuous
buffer (much less compact and cache-friendly). Security restrictions would
be a concern as well. At first glance at Java 9, I don't see any facilities
that would help here.

This would seem to be a fairly common use case a low-level serialization
framework, such as protobuf. Are there any thoughts on how such a use case
could be supported post-Unsafe?

Thanks,
Nathan

Re: RFR: 8170595: Optimize Class.isAnonymousClass, isLocalClass, and isMemberClass

[Claes Redestad]

On 2016-12-08 18:17, Mandy Chung wrote:

On Dec 8, 2016, at 3:06 AM, Claes Redestad  wrote:

http://cr.openjdk.java.net/~redestad/8170595/webrev.04/

Looks good.


Thanks!


  Can you update the synoposis of this issue to include isLocalClass and 
isMemberClass?


Done.

/Claes



Mandy

Re: RFR: 8170595: Optimize Class.isAnonymousClass

[Mandy Chung]

> On Dec 8, 2016, at 3:06 AM, Claes Redestad  wrote:
> 
> http://cr.openjdk.java.net/~redestad/8170595/webrev.04/

Looks good.  Can you update the synoposis of this issue to include isLocalClass 
and isMemberClass?

Mandy

Re: RFR 8170348: Appendable.appendN(char, int) method to append multiple copies of char

[Paul Sandoz]

> On 8 Dec 2016, at 01:27, Claes Redestad  wrote:
> 
> 
> 
> On 2016-12-08 10:01, Ulf Zibis wrote:
>> Am 08.12.2016 um 09:28 schrieb Peter Levart:
>>> 
>>> On 12/07/2016 11:28 PM, Roger Riggs wrote:
 AbstractStringBuilder:
   I agree with Claes' comment suggesting that IAE for negative lengths is 
 a pain
   and defining it to append 0 would be natural in many use cases.
>>> 
>>> OTOH, inserting a simple Math.max(n, 0) instead of n where n could get 
>>> negative would achieve the same without complicating the expression too 
>>> much. Java standard APIs have a tradition of being explicit rather than 
>>> having implicit hidden logic which surely shortens many usecases, but makes 
>>> them harder to read and understand for casual readers not intimately 
>>> familiar with such API. The logic to treat negative lengths as 0 is 
>>> implicit and not universally correct.
>> +1
>> If we would treat negative values as 0, we loose a chance, where programmers 
>> could become aware about possible errors in the logic of their program.
> 
> Right, the two of you have convinced me that some exceptional explicitness is 
> the better choice in this (and likely most) cases.
> 

+1 i think this is the correct approach.

Paul.

Re: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed intermittently

[Felix Yang]

Langer and Dmitry,

   thanks for the review. Pushed with typo fixed

-Felix
On 2016/12/8 20:21, Langer, Christoph wrote:

Hi Felix,

looks good also for me.

Small typo:
85 throw new RuntimeException("Mistmatch between default"

Mistmatch -> Mismatch :)

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Felix
Yang
Sent: Donnerstag, 8. Dezember 2016 11:35
To: Dmitry Samersoff ; core-libs-
d...@openjdk.java.net; net-...@openjdk.java.net; CHRIS.HEGARTY

Subject: Re: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed
intermittently

Hi Dmitry,

 I tested your suggested "icann.org" and it indeed works well. Please
review the updated webrev:

http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.02/

Thanks,
Felix
On 2016/12/6 19:16, Dmitry Samersoff wrote:

Felix,

1. I'm not sure that javaweb.sfbay.sun.com is the best domain name for
this test. Could we use different one (e.g. icann.org)

2. This test run JTREG -> Test VM -> Another VM. Could we optimize
process usage?

It might be better to create a jtreg "same vm" process that

 1. launch another process with -Djava.net.preferIPv4Stack=true
 that do A and PRT lookup in one run.

 2. Read results of process above, do PTR lookup with default
 settings and compare results.

-Dmitry


On 2016-12-06 12:06, Felix Yang wrote:

Hi,

 please review the following patch. It generally coverts codes from
shell to plain java.

Bug:

  https://bugs.openjdk.java.net/browse/JDK-8169115

Webrev:

  http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.00/

Thanks,

Felix

RE: RFR(M): 8170663: Fix minor issues in corelib and servicabilty coding.

[Lindenmaier, Goetz]

Hi Joe, 

could you please have a look at my change to e_asin.c:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.04/src/java.base/share/native/libfdlibm/e_asin.c.udiff.html

The change conserves the situation, I just added {} and comments.
I would appreciate to clean this up as it's hard to understand and our 
code scan does not like it the way it is.

We had talked about this in my thread where I learned to read this code :)
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-December/045165.html

Best regards,
  Goetz.


> -Original Message-
> From: Martin Buchholz [mailto:marti...@google.com]
> Sent: Mittwoch, 7. Dezember 2016 18:09
> To: Lindenmaier, Goetz ; Joe Darcy
> 
> Cc: Dmitry Samersoff ; Java Core Libs  libs-...@openjdk.java.net>; serviceability-dev (serviceability-
> d...@openjdk.java.net) 
> Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and servicabilty
> coding.
> 
> This changes fdlibm, which has historically been untouched.  Don't commit
> without Joe Darcy's approval.
> 
> On Wed, Dec 7, 2016 at 7:26 AM, Lindenmaier, Goetz
> mailto:goetz.lindenma...@sap.com> > wrote:
> 
> 
>   Hi Dmitry,
> 
>   yes, new_jvmpath is consistent with the other variables.
>   I also merged the ifs in SDE.c.
> 
>   new webrev:
>   http://cr.openjdk.java.net/~goetz/wr16/8170663-
> corlib_s11y/webrev.03/  corlib_s11y/webrev.03/>
> 
>   Best regards,
> Goetz.
> 
>   > -Original Message-
>   > From: Dmitry Samersoff [mailto:dmitry.samers...@oracle.com
>  ]
>   > Sent: Wednesday, December 07, 2016 2:43 PM
>   > To: Lindenmaier, Goetz   >; Java Core Libs
>   > mailto:core-libs-
> d...@openjdk.java.net> >; serviceability-dev (serviceability-
>   > d...@openjdk.java.net  )
> mailto:serviceability-
> d...@openjdk.java.net> >
>   > Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and
> servicabilty
>   > coding.
>   >
> 
>   > Goetz,
>   >
>   > SDE.c:
>   >
>   > You might combine if at ll. 260 and 263 to one but it's just matter of
> test.
>   >
>   >  if (sti == baseStratumIndex || sti < 0) {
>   > return; /* Java stratum - return unchanged */
>   >  }
>   >
>   > > I'm not sure what you mean. I tried to fix it, but please
>   > > double-check the new webrev.
>   >
>   > if cnt is <= 0 loop at l.267
>   >
>   > for (; cnt-- > 0; ++fromEntry) {
>   >
>   > is never run and we effectively do
>   >
>   >  *entryCountPtr = 0;
>   >
>   > at l.283
>   >
>   > So if we you suspect that cnt may become negative or 0:
>   > (your v.01 changes)
>   >
>   >  260 if (sti < 0 && cnt > 0) {
>   >  261 return;
>   >  262 }
>   >
>   > it's better to check it early.
>   >
>   > But I'm not sure we have to care about negative/zero cnt here.
>   >
>   > -Dmitry
>   >
>   > On 2016-12-07 11:37, Lindenmaier, Goetz wrote:
>   > > Hi Dmitry,
>   > >
>   > > thanks for looking at my change!
>   > > Updated webrev:
>   > > http://cr.openjdk.java.net/~goetz/wr16/8170663-
> corlib_s11y/webrev.02  corlib_s11y/webrev.02>
>   > >
>   > >> * src/java.base/unix/native/libjli/java_md_solinux.c
>   > >> Is this line correct?
>   > >> 519 jvmpath = JLI_StringDup(jvmpath);
>   > >
>   > > It seems pointless. Should I remove it?  (The whole file is a 
> horror.)
>   > >
>   > >> * src/jdk.jdwp.agent/share/native/libjdwp/SDE.c
>   > >> It might be better to return immediately if cnt < 0,
>   > >> regardless of value of sti.
>   > >
>   > > I'm not sure what you mean. I tried to fix it, but please
>   > > double-check the new webrev.
>   > >
>   > >> * src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c
>   > >> It might be better to write
>   > >>   arg.l_linger = (on) ? (unsigned short)value.i : 0;
>   > >> and leave one copy of setsockopt() call
>   > >
>   > > Yes, looks better.
>   > >
>   > > Best regards,
>   > >   Goetz
>   > >
>   > >
>   > >>
>   > >> -Dmitry
>   > >>
>   > >>
>   > >> On 2016-12-06 16:12, Lindenmaier, Goetz wrote:
>   > >>> Hi,
>   > >>>
>   > >>>
>   > >>>
>   > >>> This change fixes some minor issues found in our code scans.
>   > >>>
>   > >>> I hope this correctly addresses corelib and serviceability issues.
>   > >>>
>   > >>>
>   > >>>
>   > >>> Please review:
>   > >>>
>   > >>> http://cr.openjdk.java.net/~goetz/wr16/8170663-
> 
>   > corlib_s11y/webrev.01/
>   

RE: RFR(M): 8170663: Fix minor issues in corelib and servicabilty coding.

[Lindenmaier, Goetz]

Hi David, 

thanks for looking at the change.  New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.04/

> src/java.base/share/native/libjli/java.c
> As far as I can see the existing code is working perfectly fine.
Ah, thanks for the explanation, now I got it!  Removed.

> Again I'm not sure what the bug is here. On AIX the output string is
> expanded with:
> "%s/lib/%s/jli:"
I first edited this against jdk9/hs, where the arch is gone since 8066474,
http://hg.openjdk.java.net/jdk9/hs/jdk/rev/81508186e5bc
but the // was not adapted. Then I moved the change to jdk9/dev because 
I thought I have to push it there. And yes, in that coding // would be correct.
So I have to wait until hs is promoted ...

> The jvmpath -> jvm_newpath change wasn't really necessary - a comment on
> the strdup would have sufficed IMO.
Dmitry asked me to add it.  But I think it's ok.

Can I consider this reviewed now?  I.e. could I push it once 8066474 is 
promoted and Joe Darcy agreed?

Best regards,
  Goetz.


> -Original Message-
> From: David Holmes [mailto:david.hol...@oracle.com]
> Sent: Donnerstag, 8. Dezember 2016 09:14
> To: Lindenmaier, Goetz ; 'Dmitry Samersoff'
> ; Java Core Libs  d...@openjdk.java.net>; serviceability-dev (serviceability-
> d...@openjdk.java.net) 
> Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and servicabilty
> coding.
> 
> Hi Goetz,
> 
> On 8/12/2016 1:26 AM, Lindenmaier, Goetz wrote:
> > Hi Dmitry,
> >
> > yes, new_jvmpath is consistent with the other variables.
> > I also merged the ifs in SDE.c.
> >
> > new webrev:
> > http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.03/
> 
> src/java.base/share/native/libjli/java.c
> 
> As far as I can see the existing code is working perfectly fine. Given a
> jvm.cfg with:
> 
> -server KNOWN
> -client IGNORE
> -myvm KNOWN
> -oldvm ALIASED_TO -server
> 
> The usage text is:
> 
>  -server   to select the "server" VM
>  -myvm to select the "myvm" VM
>  -oldvmis a synonym for the "server" VM  [deprecated]
>The default VM is server,
>because you are running on a server-class machine.
> 
> which is exactly what I would expect. Why do you think there is a bug?
> 
> ---
> 
> src/java.base/unix/native/libjli/java_md_solinux.c
> 
> Again I'm not sure what the bug is here. On AIX the output string is
> expanded with:
> 
> "%s/lib/%s/jli:"
> 
> so it needs to account for the extra "/lib//jli:" characters - and that
> is exactly what the existing code does:
> 
> + JLI_StrLen("/lib//jli:")
> 
> The jvmpath -> jvm_newpath change wasn't really necessary - a comment on
> the strdup would have sufficed IMO.
> 
> Thanks,
> David
> -
> 
> 
> 
> 
> > Best regards,
> >   Goetz.
> >
> >> -Original Message-
> >> From: Dmitry Samersoff [mailto:dmitry.samers...@oracle.com]
> >> Sent: Wednesday, December 07, 2016 2:43 PM
> >> To: Lindenmaier, Goetz ; Java Core Libs
> >> ; serviceability-dev (serviceability-
> >> d...@openjdk.java.net) 
> >> Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and servicabilty
> >> coding.
> >>
> >> Goetz,
> >>
> >> SDE.c:
> >>
> >> You might combine if at ll. 260 and 263 to one but it's just matter of 
> >> test.
> >>
> >>  if (sti == baseStratumIndex || sti < 0) {
> >> return; /* Java stratum - return unchanged */
> >>  }
> >>
> >>> I'm not sure what you mean. I tried to fix it, but please
> >>> double-check the new webrev.
> >>
> >> if cnt is <= 0 loop at l.267
> >>
> >> for (; cnt-- > 0; ++fromEntry) {
> >>
> >> is never run and we effectively do
> >>
> >>  *entryCountPtr = 0;
> >>
> >> at l.283
> >>
> >> So if we you suspect that cnt may become negative or 0:
> >> (your v.01 changes)
> >>
> >>  260 if (sti < 0 && cnt > 0) {
> >>  261 return;
> >>  262 }
> >>
> >> it's better to check it early.
> >>
> >> But I'm not sure we have to care about negative/zero cnt here.
> >>
> >> -Dmitry
> >>
> >> On 2016-12-07 11:37, Lindenmaier, Goetz wrote:
> >>> Hi Dmitry,
> >>>
> >>> thanks for looking at my change!
> >>> Updated webrev:
> >>> http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.02
> >>>
>  * src/java.base/unix/native/libjli/java_md_solinux.c
>  Is this line correct?
>  519 jvmpath = JLI_StringDup(jvmpath);
> >>>
> >>> It seems pointless. Should I remove it?  (The whole file is a horror.)
> >>>
>  * src/jdk.jdwp.agent/share/native/libjdwp/SDE.c
>  It might be better to return immediately if cnt < 0,
>  regardless of value of sti.
> >>>
> >>> I'm not sure what you mean. I tried to fix it, but please
> >>> double-check the new webrev.
> >>>
>  * src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c
>  It might be better to write
>    arg.l_linger = (on) ? (unsigned short)value.i : 0;
>  and leave one copy of setsockopt() call
> >>>
> >>> Yes, looks better.
> >>>
> >>> Best regards,
> >>>   Goetz
> >>>
> >

RE: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed intermittently

[Langer, Christoph]

Hi Felix,

looks good also for me.

Small typo:
85 throw new RuntimeException("Mistmatch between default"

Mistmatch -> Mismatch :)

Best regards
Christoph 

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Felix
> Yang
> Sent: Donnerstag, 8. Dezember 2016 11:35
> To: Dmitry Samersoff ; core-libs-
> d...@openjdk.java.net; net-...@openjdk.java.net; CHRIS.HEGARTY
> 
> Subject: Re: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed
> intermittently
> 
> Hi Dmitry,
> 
> I tested your suggested "icann.org" and it indeed works well. Please
> review the updated webrev:
> 
> http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.02/
> 
> Thanks,
> Felix
> On 2016/12/6 19:16, Dmitry Samersoff wrote:
> > Felix,
> >
> > 1. I'm not sure that javaweb.sfbay.sun.com is the best domain name for
> > this test. Could we use different one (e.g. icann.org)
> >
> > 2. This test run JTREG -> Test VM -> Another VM. Could we optimize
> > process usage?
> >
> > It might be better to create a jtreg "same vm" process that
> >
> > 1. launch another process with -Djava.net.preferIPv4Stack=true
> > that do A and PRT lookup in one run.
> >
> > 2. Read results of process above, do PTR lookup with default
> > settings and compare results.
> >
> > -Dmitry
> >
> >
> > On 2016-12-06 12:06, Felix Yang wrote:
> >> Hi,
> >>
> >> please review the following patch. It generally coverts codes from
> >> shell to plain java.
> >>
> >> Bug:
> >>
> >>  https://bugs.openjdk.java.net/browse/JDK-8169115
> >>
> >> Webrev:
> >>
> >>  http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.00/
> >>
> >> Thanks,
> >>
> >> Felix
> >>
> >

Re: [9] RFR 8170769: Provide a simple hexdump facility for binary data

[Vincent Ryan]

> On 8 Dec 2016, at 12:10, Vincent Ryan  wrote:
> 
> 
>> On 8 Dec 2016, at 07:22, David Holmes  wrote:
>> 
>> On 8/12/2016 2:32 AM, Vincent Ryan wrote:
>>> A hexdump facility has been available for many, many years via an 
>>> unsupported class: sun.misc.HexDumpEncoder.
>>> Although that class was always unsupported, it was still accessible. That 
>>> accessibility changes with Jigsaw so I’m proposing
>>> a very simple replacement in a new and supported class: java.util.HexDump.
>>> 
>>> Thanks.
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8170769
>>> Webrev: http://cr.openjdk.java.net/~vinnie/8170769/webrev.00/
>> 
>> googling for "hexdump java" reveals quite a few hits - including 
>> javax.xml.bind.DatatypeConverter.
> 
> DatatypeConverter is no longer directly accessible in JDK 9 as it is in the 
> java.xml.bind module.
> So an ‘—add-modules’ flag would be required.
> 
> 
>> 
>> Does this sun.* class really meet the acceptance criteria for inclusion in 
>> java.util? That bar is intentionally set very high.
> 
> We considered other locations for these methods (including java.lang.String 
> !) but settled on java.util.HexString
s/HexString/HexDump

> as a good balance between visibility and controversy.
> 
> 
>> 
>> Just wondering ...
>> 
>> David
>> 
> 

Re: [9] RFR 8170769: Provide a simple hexdump facility for binary data

[Vincent Ryan]

> On 8 Dec 2016, at 07:22, David Holmes  wrote:
> 
> On 8/12/2016 2:32 AM, Vincent Ryan wrote:
>> A hexdump facility has been available for many, many years via an 
>> unsupported class: sun.misc.HexDumpEncoder.
>> Although that class was always unsupported, it was still accessible. That 
>> accessibility changes with Jigsaw so I’m proposing
>> a very simple replacement in a new and supported class: java.util.HexDump.
>> 
>> Thanks.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8170769
>> Webrev: http://cr.openjdk.java.net/~vinnie/8170769/webrev.00/
> 
> googling for "hexdump java" reveals quite a few hits - including 
> javax.xml.bind.DatatypeConverter.

DatatypeConverter is no longer directly accessible in JDK 9 as it is in the 
java.xml.bind module.
So an ‘—add-modules’ flag would be required.


> 
> Does this sun.* class really meet the acceptance criteria for inclusion in 
> java.util? That bar is intentionally set very high.

We considered other locations for these methods (including java.lang.String !) 
but settled on java.util.HexString
as a good balance between visibility and controversy.


> 
> Just wondering ...
> 
> David
> 

Re: RFR: 8054214: JapaneseEra.getDisplayName doesn't return names if it's an additional era

[Jonathan Bluett-Duncan]

Hi Masayoshi,

I'm not a reviewer, but there's something in JapaneseEra.java.cdiff.html

which
isn't terribly clear to me, so I wonder if you could clarify it for me.

In the method `getDisplayName(TextStyle style, Locale locale)`, it's not
clear to me why the call to `Objects.requireNonNull(locale, "locale");` is
within the if-statement rather than on the first line. Is this because
`Era.super::getDisplayName` already has a null check of its own?

Kind regards,
Jonathan

On 8 December 2016 at 08:55, Masayoshi Okutsu 
wrote:

> Hi,
>
> Please review the fix for JDK-8054214. It was necessary to override
> Era::getDisplayName to get era names from the specified property. This one
> fixes getAbbreviation() as well.
>
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8054214
>
> Webrev:
> http://cr.openjdk.java.net/~okutsu/9/8054214/webrev.00/
>
> Thanks,
> Masayoshi
>
>

Re: [9] RFR 8170769: Provide a simple hexdump facility for binary data

[Vincent Ryan]

> On 8 Dec 2016, at 10:41, Ulf Zibis  wrote:
> 
> Hi,
> 
> I would prefer a "normal" class instead a convolut of static methods. Via a 
> normal constructor, we could pass some custom parameters e.g. 
> capital/uppercase letters for "abcdef", prefix a header line, width of the 
> index counter, bytes per line, i.e. have all the parameters, you have 
> hardcoded, variable.

The dumpToStream method is designed to cater for all forms of customisation.
You can see in the implementation that the dump method simply calls 
dumpToStream with a custom collector.

I think that approach is more flexible than a constructor taking a fixed set of 
parameters.


> 
> Additionally I would like to see a method with variable start and end:
> 
> String dump(byte[] bytes, int start, int length)
> 

That makes sense, I will add that. And  matching one for dumpToStream ?


> 
> -Ulf
> 
> Am 07.12.2016 um 17:32 schrieb Vincent Ryan:
>> A hexdump facility has been available for many, many years via an 
>> unsupported class: sun.misc.HexDumpEncoder.
>> Although that class was always unsupported, it was still accessible. That 
>> accessibility changes with Jigsaw so I’m proposing
>> a very simple replacement in a new and supported class: java.util.HexDump.
>> 
>> Thanks.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8170769
>> Webrev: http://cr.openjdk.java.net/~vinnie/8170769/webrev.00/
>> 
>> 
> 

Re: RFR: 8170595: Optimize Class.isAnonymousClass

[Claes Redestad]

On 2016-12-08 04:30, Mandy Chung wrote:

On Dec 2, 2016, at 6:29 AM, Claes Redestad  wrote:

:
http://cr.openjdk.java.net/~redestad/8170595/webrev.03/

This brings significant improvements to some variants:

Good to see the significant improvements.

I think what isLocalOrAnonymousClass needs probably is something like this:

   private boolean hasEnclosingMethodInfo() {
   Object[] enclosingInfo = getEnclosingMethod0();
   if (enclosingInfo != null) {
   EnclosingMethodInfo.validate(enclosingInfo);
   }
   return enclosingInfo != null;
   }

Then EnclosingMethodInfo doesn’t seem the need to change.  The overall change 
would be simpler.


Adding hasEnclosingMethodInfo has some descriptive value, but we'd still
need to change EnclosingMethodInfo a bit to separate the validation from
the constructor.



As Joe suggests, this worths adding test cases for these methods, if not 
present.


I was also concerned about the lack of explicit tests for this under 
jdk/test, but found what
seemed like comprehensive tests in langtools, EnclosingMethodTest.java 
in particular.


However, it seems adding a straightforward, minimal test inspired by 
other tests under
java/lang/Class wouldn't hurt to ensure completeness and some more local 
coverage, so

I made an attempt at this:

http://cr.openjdk.java.net/~redestad/8170595/webrev.04/

Thanks!

/Claes



Mandy

Re: [9] RFR 8170769: Provide a simple hexdump facility for binary data

[Ulf Zibis]

Hi,

I would prefer a "normal" class instead a convolut of static methods. Via a normal constructor, we 
could pass some custom parameters e.g. capital/uppercase letters for "abcdef", prefix a header line, 
width of the index counter, bytes per line, i.e. have all the parameters, you have hardcoded, variable.


Additionally I would like to see a method with variable start and end:

String dump(byte[] bytes, int start, int length)


-Ulf

Am 07.12.2016 um 17:32 schrieb Vincent Ryan:

A hexdump facility has been available for many, many years via an unsupported 
class: sun.misc.HexDumpEncoder.
Although that class was always unsupported, it was still accessible. That 
accessibility changes with Jigsaw so I’m proposing
a very simple replacement in a new and supported class: java.util.HexDump.

Thanks.

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

Re: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed intermittently

[Dmitry Samersoff]

Felix,

Changes looks good to me. Thank you for doing it.

-Dmitry


On 2016-12-08 13:35, Felix Yang wrote:
> Hi Dmitry,
> 
>I tested your suggested "icann.org" and it indeed works well. Please
> review the updated webrev:
> 
> http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.02/
> 
> Thanks,
> Felix
> On 2016/12/6 19:16, Dmitry Samersoff wrote:
>> Felix,
>>
>> 1. I'm not sure that javaweb.sfbay.sun.com is the best domain name for
>> this test. Could we use different one (e.g. icann.org)
>>
>> 2. This test run JTREG -> Test VM -> Another VM. Could we optimize
>> process usage?
>>
>> It might be better to create a jtreg "same vm" process that
>>
>> 1. launch another process with -Djava.net.preferIPv4Stack=true
>> that do A and PRT lookup in one run.
>>
>> 2. Read results of process above, do PTR lookup with default
>> settings and compare results.
>>
>> -Dmitry
>>
>>
>> On 2016-12-06 12:06, Felix Yang wrote:
>>> Hi,
>>>
>>> please review the following patch. It generally coverts codes from
>>> shell to plain java.
>>>
>>> Bug:
>>>
>>>  https://bugs.openjdk.java.net/browse/JDK-8169115
>>>
>>> Webrev:
>>>
>>>  http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.00/
>>>
>>> Thanks,
>>>
>>> Felix
>>>
>>
> 


-- 
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: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed intermittently

[Felix Yang]

Hi Dmitry,

   I tested your suggested "icann.org" and it indeed works well. Please 
review the updated webrev:


http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.02/

Thanks,
Felix
On 2016/12/6 19:16, Dmitry Samersoff wrote:

Felix,

1. I'm not sure that javaweb.sfbay.sun.com is the best domain name for
this test. Could we use different one (e.g. icann.org)

2. This test run JTREG -> Test VM -> Another VM. Could we optimize
process usage?

It might be better to create a jtreg "same vm" process that

1. launch another process with -Djava.net.preferIPv4Stack=true
that do A and PRT lookup in one run.

2. Read results of process above, do PTR lookup with default
settings and compare results.

-Dmitry


On 2016-12-06 12:06, Felix Yang wrote:

Hi,

please review the following patch. It generally coverts codes from
shell to plain java.

Bug:

 https://bugs.openjdk.java.net/browse/JDK-8169115

Webrev:

 http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.00/

Thanks,

Felix

Re: [9] RFR 8170769: Provide a simple hexdump facility for binary data

[Vincent Ryan]

> On 8 Dec 2016, at 09:34, Volker Simonis  wrote:
> 
> Hi Vincent,
> 
> the bug is closed and can't be looked at outside Oracle.
> Can you please make it visible for everybody.

Sorry. I’ve just corrected that.

> 
> Also, this is a change of the public API so it requires a CCC 
> request/approval.

I’ve a CCC prepared which I will submit if there is consensus on the proposed 
API.


> 
> Finally, this seems to be clearly a feature and not a bug so it
> requires an FC extension request [1]

Right. I’ve begun that too.


> 
> Thanks,
> Volker
> 
> [1] http://openjdk.java.net/projects/jdk9/fc-extension-process
> 
> On Wed, Dec 7, 2016 at 5:32 PM, Vincent Ryan  
> wrote:
>> A hexdump facility has been available for many, many years via an 
>> unsupported class: sun.misc.HexDumpEncoder.
>> Although that class was always unsupported, it was still accessible. That 
>> accessibility changes with Jigsaw so I’m proposing
>> a very simple replacement in a new and supported class: java.util.HexDump.
>> 
>> Thanks.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8170769
>> Webrev: http://cr.openjdk.java.net/~vinnie/8170769/webrev.00/
>> 

Re: [9] RFR 8170769: Provide a simple hexdump facility for binary data

[Volker Simonis]

Hi Vincent,

the bug is closed and can't be looked at outside Oracle.
Can you please make it visible for everybody.

Also, this is a change of the public API so it requires a CCC request/approval.

Finally, this seems to be clearly a feature and not a bug so it
requires an FC extension request [1]

Thanks,
Volker

[1] http://openjdk.java.net/projects/jdk9/fc-extension-process

On Wed, Dec 7, 2016 at 5:32 PM, Vincent Ryan  wrote:
> A hexdump facility has been available for many, many years via an unsupported 
> class: sun.misc.HexDumpEncoder.
> Although that class was always unsupported, it was still accessible. That 
> accessibility changes with Jigsaw so I’m proposing
> a very simple replacement in a new and supported class: java.util.HexDump.
>
> Thanks.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8170769
> Webrev: http://cr.openjdk.java.net/~vinnie/8170769/webrev.00/
>

Re: RFR 8170348: Appendable.appendN(char, int) method to append multiple copies of char

[Claes Redestad]

On 2016-12-08 10:01, Ulf Zibis wrote:

Am 08.12.2016 um 09:28 schrieb Peter Levart:


On 12/07/2016 11:28 PM, Roger Riggs wrote:

AbstractStringBuilder:
   I agree with Claes' comment suggesting that IAE for negative 
lengths is a pain
   and defining it to append 0 would be natural in many use cases. 


OTOH, inserting a simple Math.max(n, 0) instead of n where n could 
get negative would achieve the same without complicating the 
expression too much. Java standard APIs have a tradition of being 
explicit rather than having implicit hidden logic which surely 
shortens many usecases, but makes them harder to read and understand 
for casual readers not intimately familiar with such API. The logic 
to treat negative lengths as 0 is implicit and not universally correct.

+1
If we would treat negative values as 0, we loose a chance, where 
programmers could become aware about possible errors in the logic of 
their program.


Right, the two of you have convinced me that some exceptional 
explicitness is the better choice in this (and likely most) cases.


Thanks!

/Claes


-Ulf

Re: RFR 8170348: Appendable.appendN(char, int) method to append multiple copies of char

[Ulf Zibis]

Am 08.12.2016 um 09:28 schrieb Peter Levart:


On 12/07/2016 11:28 PM, Roger Riggs wrote:

AbstractStringBuilder:
   I agree with Claes' comment suggesting that IAE for negative lengths is a 
pain
   and defining it to append 0 would be natural in many use cases. 


OTOH, inserting a simple Math.max(n, 0) instead of n where n could get negative would achieve the 
same without complicating the expression too much. Java standard APIs have a tradition of being 
explicit rather than having implicit hidden logic which surely shortens many usecases, but makes 
them harder to read and understand for casual readers not intimately familiar with such API. The 
logic to treat negative lengths as 0 is implicit and not universally correct.

+1
If we would treat negative values as 0, we loose a chance, where programmers could become aware 
about possible errors in the logic of their program.


-Ulf

RFR: 8054214: JapaneseEra.getDisplayName doesn't return names if it's an additional era

[Masayoshi Okutsu]

Hi,

Please review the fix for JDK-8054214. It was necessary to override 
Era::getDisplayName to get era names from the specified property. This 
one fixes getAbbreviation() as well.


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

Webrev:
http://cr.openjdk.java.net/~okutsu/9/8054214/webrev.00/

Thanks,
Masayoshi

Re: RFR 8170348: Appendable.appendN(char, int) method to append multiple copies of char

[Peter Levart]

On 12/07/2016 11:28 PM, Roger Riggs wrote:

AbstractStringBuilder:
   I agree with Claes' comment suggesting that IAE for negative 
lengths is a pain
   and defining it to append 0 would be natural in many use cases. 


OTOH, inserting a simple Math.max(n, 0) instead of n where n could get 
negative would achieve the same without complicating the expression too 
much. Java standard APIs have a tradition of being explicit rather than 
having implicit hidden logic which surely shortens many usecases, but 
makes them harder to read and understand for casual readers not 
intimately familiar with such API. The logic to treat negative lengths 
as 0 is implicit and not universally correct.


Regards, Peter

Re: RFR(M): 8170663: Fix minor issues in corelib and servicabilty coding.

[David Holmes]

Hi Goetz,

On 8/12/2016 1:26 AM, Lindenmaier, Goetz wrote:

Hi Dmitry,

yes, new_jvmpath is consistent with the other variables.
I also merged the ifs in SDE.c.

new webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.03/


src/java.base/share/native/libjli/java.c

As far as I can see the existing code is working perfectly fine. Given a 
jvm.cfg with:


-server KNOWN
-client IGNORE
-myvm KNOWN
-oldvm ALIASED_TO -server

The usage text is:

-server   to select the "server" VM
-myvm to select the "myvm" VM
-oldvmis a synonym for the "server" VM  [deprecated]
  The default VM is server,
  because you are running on a server-class machine.

which is exactly what I would expect. Why do you think there is a bug?

---

src/java.base/unix/native/libjli/java_md_solinux.c

Again I'm not sure what the bug is here. On AIX the output string is 
expanded with:


"%s/lib/%s/jli:"

so it needs to account for the extra "/lib//jli:" characters - and that 
is exactly what the existing code does:


+ JLI_StrLen("/lib//jli:")

The jvmpath -> jvm_newpath change wasn't really necessary - a comment on 
the strdup would have sufficed IMO.


Thanks,
David
-





Best regards,
  Goetz.


-Original Message-
From: Dmitry Samersoff [mailto:dmitry.samers...@oracle.com]
Sent: Wednesday, December 07, 2016 2:43 PM
To: Lindenmaier, Goetz ; Java Core Libs
; serviceability-dev (serviceability-
d...@openjdk.java.net) 
Subject: Re: RFR(M): 8170663: Fix minor issues in corelib and servicabilty
coding.

Goetz,

SDE.c:

You might combine if at ll. 260 and 263 to one but it's just matter of test.

 if (sti == baseStratumIndex || sti < 0) {
return; /* Java stratum - return unchanged */
 }


I'm not sure what you mean. I tried to fix it, but please
double-check the new webrev.


if cnt is <= 0 loop at l.267

for (; cnt-- > 0; ++fromEntry) {

is never run and we effectively do

 *entryCountPtr = 0;

at l.283

So if we you suspect that cnt may become negative or 0:
(your v.01 changes)

 260 if (sti < 0 && cnt > 0) {
 261 return;
 262 }

it's better to check it early.

But I'm not sure we have to care about negative/zero cnt here.

-Dmitry

On 2016-12-07 11:37, Lindenmaier, Goetz wrote:

Hi Dmitry,

thanks for looking at my change!
Updated webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170663-corlib_s11y/webrev.02


* src/java.base/unix/native/libjli/java_md_solinux.c
Is this line correct?
519 jvmpath = JLI_StringDup(jvmpath);


It seems pointless. Should I remove it?  (The whole file is a horror.)


* src/jdk.jdwp.agent/share/native/libjdwp/SDE.c
It might be better to return immediately if cnt < 0,
regardless of value of sti.


I'm not sure what you mean. I tried to fix it, but please
double-check the new webrev.


* src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c
It might be better to write
  arg.l_linger = (on) ? (unsigned short)value.i : 0;
and leave one copy of setsockopt() call


Yes, looks better.

Best regards,
  Goetz




-Dmitry


On 2016-12-06 16:12, Lindenmaier, Goetz wrote:

Hi,



This change fixes some minor issues found in our code scans.

I hope this correctly addresses corelib and serviceability issues.



Please review:

http://cr.openjdk.java.net/~goetz/wr16/8170663-

corlib_s11y/webrev.01/




Best regards,

  Goetz.



Changes in detail:



e_asin.c

Code scan reports missing {}.


The code "if (huge+x>one) {" is only there to set the inexact flag of
the processor.
It's a way to avoid the C compiler to optimize the code away. It is
always true,
so the parenthesis of the outer else don't matter.

Although this basically just adds the {} I would like to submit this to

avoid anybody else spends more the 30sec on understanding these

if statements.


k_standard.c

exc.retval is returned below and thus should always be initialized.


imageDecompressor.cpp

Wrong destructor is used.  Reported also by David CARLIER


java.c

in line 1865 'name' was used, it should be 'alias'.


java_md_solinux.c

"//" in path is useless. Further down a free is missing.


SDE.c

Call to stratumTableIndex can return negative value if defaultStratumId
== null.


socket_md.c

arg.l_linger is passed to setsockopt uninitialized. Its use is hidden in
the macros.


unpack.cpp

n.slice should not get negative argument for end, which is passed from
dollar1.
But dollar1 can get negative where it is set to the result of
lastIndexOf(DOLLAR_MIN, DOLLAR_MAX, n, dollar2 - 1).




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



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