Re: ParallelStream Vs Stream Digest, Vol 113, Issue 94

2016-09-29 Thread Louis Wasserman
You should absolutely not assume parallel streams are faster than
sequential streams.
http://gee.cs.oswego.edu/dl/html/StreamParallelGuidance.html is pretty much
the iconic document on that subject, and explains circumstances under which
parallelism is good, and when it is likely to be harmful.

On Thu, Sep 29, 2016 at 10:07 PM Prakhar Makhija  wrote:

> The application makes a hit to a core object over and over again. I have to
> copy this object, i.e. make a clone of it using the Cloneable interface, so
> that the original object cannot be modified. But since the references of
> the old object and clone object would be intact, inside the clone method I
> am explicitly copying the List Map and Set using parrallelStream/stream.
>
> The hardware is i3 processor with 8GB RAM and 1TB hard disk.
>
> So you mean to say, Parallel Stream is good for large data set?
> On Sep 30, 2016 10:08 AM, "David Holmes"  wrote:
>
> > On 30/09/2016 2:24 PM, Prakhar Makhija wrote:
> >
> >> Hi everyone,
> >>
> >> I have started using both Stream and ParallelStream, for Set List and
> >> Entry
> >> of Map.
> >>
> >> What I can't understand is why Stream is taking lesser time than
> >> ParallelStream.
> >>
> >> Shouldnt ParallelStream be giving better performance than Stream in
> terms
> >> of Time Complexity?
> >>
> >
> > Depends on the data set size and your hardware, and what exactly you are
> > trying to do in parallel.
> >
> > David
> >
>


Re: ParallelStream Vs Stream Digest, Vol 113, Issue 94

2016-09-29 Thread David Holmes

On 30/09/2016 3:07 PM, Prakhar Makhija wrote:

The application makes a hit to a core object over and over again. I have
to copy this object, i.e. make a clone of it using the Cloneable
interface, so that the original object cannot be modified. But since the
references of the old object and clone object would be intact, inside
the clone method I am explicitly copying the List Map and Set using
parrallelStream/stream.

The hardware is i3 processor with 8GB RAM and 1TB hard disk.


You only have two cores on an i3, with hyper-threading so you have very 
limited speedup potential to begin with



So you mean to say, Parallel Stream is good for large data set?


What I mean is that if the data set is too small then the overhead of 
parallelizing the work will outweigh the work itself.


David
-


On Sep 30, 2016 10:08 AM, "David Holmes" mailto:david.hol...@oracle.com>> wrote:

On 30/09/2016 2:24 PM, Prakhar Makhija wrote:

Hi everyone,

I have started using both Stream and ParallelStream, for Set
List and Entry
of Map.

What I can't understand is why Stream is taking lesser time than
ParallelStream.

Shouldnt ParallelStream be giving better performance than Stream
in terms
of Time Complexity?


Depends on the data set size and your hardware, and what exactly you
are trying to do in parallel.

David



Re: ParallelStream Vs Stream Digest, Vol 113, Issue 94

2016-09-29 Thread Prakhar Makhija
The application makes a hit to a core object over and over again. I have to
copy this object, i.e. make a clone of it using the Cloneable interface, so
that the original object cannot be modified. But since the references of
the old object and clone object would be intact, inside the clone method I
am explicitly copying the List Map and Set using parrallelStream/stream.

The hardware is i3 processor with 8GB RAM and 1TB hard disk.

So you mean to say, Parallel Stream is good for large data set?
On Sep 30, 2016 10:08 AM, "David Holmes"  wrote:

> On 30/09/2016 2:24 PM, Prakhar Makhija wrote:
>
>> Hi everyone,
>>
>> I have started using both Stream and ParallelStream, for Set List and
>> Entry
>> of Map.
>>
>> What I can't understand is why Stream is taking lesser time than
>> ParallelStream.
>>
>> Shouldnt ParallelStream be giving better performance than Stream in terms
>> of Time Complexity?
>>
>
> Depends on the data set size and your hardware, and what exactly you are
> trying to do in parallel.
>
> David
>


Re: ParallelStream Vs Stream Digest, Vol 113, Issue 94

2016-09-29 Thread David Holmes

On 30/09/2016 2:24 PM, Prakhar Makhija wrote:

Hi everyone,

I have started using both Stream and ParallelStream, for Set List and Entry
of Map.

What I can't understand is why Stream is taking lesser time than
ParallelStream.

Shouldnt ParallelStream be giving better performance than Stream in terms
of Time Complexity?


Depends on the data set size and your hardware, and what exactly you are 
trying to do in parallel.


David


ParallelStream Vs Stream Digest, Vol 113, Issue 94

2016-09-29 Thread Prakhar Makhija
Hi everyone,

I have started using both Stream and ParallelStream, for Set List and Entry
of Map.

What I can't understand is why Stream is taking lesser time than
ParallelStream.

Shouldnt ParallelStream be giving better performance than Stream in terms
of Time Complexity?
On Sep 30, 2016 12:53 AM,  wrote:

> Send core-libs-dev mailing list submissions to
> core-libs-dev@openjdk.java.net
>
> To subscribe or unsubscribe via the World Wide Web, visit
> http://mail.openjdk.java.net/mailman/listinfo/core-libs-dev
> or, via email, send a message with subject or body 'help' to
> core-libs-dev-requ...@openjdk.java.net
>
> You can reach the person managing the list at
> core-libs-dev-ow...@openjdk.java.net
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of core-libs-dev digest..."
>
>
> Today's Topics:
>
>1. Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build
>   (Kumar Srinivasan)
>2. Proposal to introduce method "Instrumentation.getInstance()"
>   toinstrument the current VM (Rafael Winterhalter)
>3. Re: RFR: JEP draft for Linux/s3990x port (Vladimir Kozlov)
>4. Re: Proposal to introduce method
>   "Instrumentation.getInstance()" toinstrument the current VM
>   (Aleksey Shipilev)
>5. Re: Proposal to introduce method
>   "Instrumentation.getInstance()" toinstrument the current VM
>   (Alan Bateman)
>6. Re: [9] RFR of JDK-8085192: java/rmi/activation/Activatable
>   tests fail intermittently due to "Port already in use" (Chris
> Hegarty)
>7. Re: Proposal to introduce method
>   "Instrumentation.getInstance()"   to  instrument the current VM
>   (Remi Forax)
>
>
> --
>
> Message: 1
> Date: Thu, 29 Sep 2016 10:48:59 -0700
> From: Kumar Srinivasan 
> To: Alan Burlison ,   Volker Simonis
> 
> Cc: "Lindenmaier, Goetz" ,   build-dev
> ,   "ppc-aix-port-...@openjdk.java.net
> "
> ,Chris Bensen
> ,  core-libs-dev
> 
> Subject: Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build
> Message-ID: <57ed540b.9010...@oracle.com>
> Content-Type: text/plain; charset=UTF-8; format=flowed
>
>
> +1.
>
> Kumar
>
> > On 29/09/2016 16:25, Erik Joelsson wrote:
> >
> >> Volker's comment above was directed at the suggestion of taking the
> >> problematic AIX specific code out of the OpenJDK repositories and create
> >> a separate library with a separate lifecycle somewhere else that OpenJDK
> >> for AIX would then need to depend on. Volker was instead proposing what
> >> you describe.
> >
> > Ah right, in which case we are in violent agreement ;-)
> >
>
>
>
> --
>
> Message: 2
> Date: Thu, 29 Sep 2016 19:50:18 +0200
> From: Rafael Winterhalter 
> To: core-libs-dev@openjdk.java.net
> Subject: Proposal to introduce method "Instrumentation.getInstance()"
> to  instrument the current VM
> Message-ID:
>  com>
> Content-Type: text/plain; charset=UTF-8
>
> Hello,
>
> I want to propose adding a method to the instrumentation API to receive an
> instance of the current VM's instrumentation API. Currently, many
> applications "self-attach" to gain such access. Unfortunately, this only
> works on JDK-VMs but I believe that this approach will grow in popularity
> with Java 9 where the Attach API is also part of any regular VM.
>
> Currently, such self-attachment is a rather flaky procedure:
>
> 1. Locate the "tools.jar" relatively to the Java Home.
> 2. Create a new URLClassLoader for this jar.
> 3. Locate the VirtualMachine type.
> 4. Parse the process Id from the JMX ManagementBean.
> 5. Load an agent that stores the instrumentation instance in a public
> field.
> 6. Locate this type from the class loader and read the field to receive the
> instance.
>
> I maintain a library offering an API for such self-attach and we can do
> some amazing things with it. For example, the Mockito library (ca. 400k
> downloads/month) now allows for mocking of final methods and types by
> inlining the mocking logic into a class what avoids class creation to
> create mocks by a subclass with virtual method overrides. Or cache
> libraries can call the objectSize method to limit memory usage by a given
> number in byte.
>
> Due to the need of publicly exposing the instrumentation API in a public
> field when using the above approach, this is however also a security risk
> and the procedure is also less stable as it should be as it needs I/O. In
> Java 9, this improves as there is an API for reading the current process id
> and for accessing the classes of tools.jar but the situation is still far
> from ideal. Within any library that uses for example EhCache, the instance
> can be stolen by any application on the class path by simple (public)
> reflection what then allows instrumenting the security manager to gain all
> privileges.
>
> 

Re: [9] RFR of JDK-8085192: java/rmi/activation/Activatable tests fail intermittently due to "Port already in use"

2016-09-29 Thread Hamlin Li

Hi Joe, Roger,

Modified based on your latest comments, please check the new webrev: 
http://cr.openjdk.java.net/~mli/8085192/webrev.02/

At the same time, I think Chris's idea is great.

Thank you
-Hamlin

On 2016/9/30 7:14, Joseph D. Darcy wrote:
If Hamlin's approach is taken in the end, I think having a smaller 
retry count (5 or 10 rather than 20) would be more appropriate to 
avoid making the worst-case running time longer than necessary.


Cheers,

-Joe

On 9/29/2016 6:55 AM, Roger Riggs wrote:

Hi Hamlin,

One more suggested improvement.   Instead of two copy/paste copies of 
the launch with options code,
it would cleaner to create a separate RMID.launch(String[] options) 
method that would be passed the extra arguments.

Use it in forceLogSnapshot.java and ShutdownGracefully.java.

The (current) no-arg version could call the version with args with an 
empty array(or null).

There would be only 1 copy of the launch algorithm.

Roger







Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-29 Thread Stuart Marks

Hi Jonathan, all,

I've started to look at this changeset. I'm looking at the one Patrick Reinhart 
posted a couple weeks ago (! sorry):


http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.01/

I looked at only a few files and I'm already starting to have questions. Not 
because anybody did anything wrong, but because there are some subtle issues 
that hadn't been raised.


1) ModuleFinder.java

 static ModuleFinder compose(ModuleFinder... finders) {
- final List finderList = Arrays.asList(finders);
- finderList.forEach(Objects::requireNonNull);
+ final List finderList = List.of(finders);

 return new ModuleFinder() { ...

It's important to note that Arrays.asList() wraps a list around an array, 
whereas List.of() has copying semantics. Initially I thought that copying here 
is unnecessary, but after further analysis I've come to believe it's necessary. 
The compose() method is public in the ModuleFinder interface, and the list is 
captured by the ModuleFinder anonymous inner class below, which is returned to 
the caller. Therefore, if Arrays.asList() is used, the caller can mutate the 
array argument and cause the ModuleFinder instance to misbehave. This is a 
TOCTOU problem. Oddly, TOCTOU wasn't discussed with respect to this change, 
though it was with another.


In any case switching to the copying semantics of List.of() in this case is 
correct and prevents TOCTOU problems.


2) CookieManager.java

The old code created a HashMap and wrapped it in a Collections.unmodifiableMap() 
and returned it to the caller. The new code returns some instance created by 
Map.of(). This is a public API. While both versions conform to the specification 
(it says an "immutable" Map is returned) certain behavioral differences are 
visible. In particular, the different objects will serialize differently. It's a 
Map>, and it looks like the values are instances of 
ArrayList, so the whole thing will be serializable. It's conceivable that 
applications might take this thing and serialize it. This potentially exposes 
applications to serial interoperability problems if they wanted to exchange 
serial data containing this object between Java 8 and 9.


My hunch is that this problem is fairly unlikely to occur, but I think it would 
be good idea to do some searching to see how often this API is used. If it's 
used rarely, or the typical usages don't involve storing the result somewhere 
(e.g., iterating over the values), then we can proceed.


I use grepcode.com for code searches. I'd be interested in hearing about 
alternative code search engines as well.


3) FileTreeIterator.java

Here's the one where a comment was added describing the copying semantics of 
List.of in order to prevent TOCTOU problems. The options array is passed from 
the outside and thus can be mutated by the caller. But the only time it's used 
is within FileTreeWalker, and there it's iterated over once and never used 
again. This differs from case (1) above where (in the old code) the array 
reference is captured by a long-lived object.


Anyway, in this case, I don't think the copying is necessary, so the old 
Arrays.asList() code is probably fine.


4) Not a specific change, but I saw mention upthread of some change that caused 
one of the java.time tests to fail, something about 'resolverFields' containing 
null. Which change was this, and what's its status in the current webrev?


I note that Stephen Colebourne said that the changes seem "reasonable" but I 
don't know if he saw the mention of the java.time test failure, or whether the 
changeset he reviewed included a change that would have caused it.


* * *

I still need to look at the other changes. If we end up getting hung up on one 
or two, it might make sense to break up the changeset and push part of it. It 
might also make sense to split out the java.time changes, since they're a 
logical chunk, and they potentially have different reviewers. But no need to do 
anything on that just yet.


s'marks




On 9/15/16 12:36 PM, Jonathan Bluett-Duncan wrote:

Hi all,

Stuart, thank you very much for your thorough response.

Regarding serializability concerns, I've just checked my changes and all 
non-test code in the JDK which calls it, and it doesn't seem to me that they 
affect any fields in `Serializable` classes or the return values of methods 
which either return instances of `Serializable` classes or whose javadoc 
mention that the returned object is serializable. So I'm somewhat certain that 
my changes are serialization-compatible, but only somewhat, because I'm not 
that familiar with the intricacies of serialization...


Considering how busy Stuart is, would anyone else be happy to sponsor this 
change?

Kind regards,
Jonathan

On 15 September 2016 at 18:20, Stuart Marks > wrote:


Hi all,

Unfortunately I don't have time to work on any of this at the moment,
because of JavaOne preparation, and JavaOne next week.

Jonat

Re: [9] RFR of JDK-8085192: java/rmi/activation/Activatable tests fail intermittently due to "Port already in use"

2016-09-29 Thread Joseph D. Darcy
If Hamlin's approach is taken in the end, I think having a smaller retry 
count (5 or 10 rather than 20) would be more appropriate to avoid making 
the worst-case running time longer than necessary.


Cheers,

-Joe

On 9/29/2016 6:55 AM, Roger Riggs wrote:

Hi Hamlin,

One more suggested improvement.   Instead of two copy/paste copies of 
the launch with options code,
it would cleaner to create a separate RMID.launch(String[] options) 
method that would be passed the extra arguments.

Use it in forceLogSnapshot.java and ShutdownGracefully.java.

The (current) no-arg version could call the version with args with an 
empty array(or null).

There would be only 1 copy of the launch algorithm.

Roger





Re: [9] RFR of JDK-8085192: java/rmi/activation/Activatable tests fail intermittently due to "Port already in use"

2016-09-29 Thread Joseph D. Darcy

Hello,

Without commenting on the particulars, I'm happy to see work being done 
to address this issue in running the RMI tests. A fix here should 
greatly increase the reliability of the JDK test suite!


Thanks,

-Joe

On 9/29/2016 12:09 PM, Chris Hegarty wrote:

On 29 Sep 2016, at 16:25, Chris Hegarty  wrote:

I have asked Hamlin to hold off on this for a day or so.  I have an
alternative proposal that eliminates the free port anti-pattern.

It is possible to use the inheritedChannel mechanism to have the rmid
process create the server channel on an ephemeral port and report it
back to the test, i.e. remove the free port pattern.

http://cr.openjdk.java.net/~chegar/8085192_webrev/

1) The port number is reported from rmid to the test over stdout.

2) All tests pass except CheckAnnotations.java, which looks for stderr
 ( see 3 below ). I think the stderr check can be removed, and the
 just check stdout.

3)  rmid, when using inheritChannel, redirects stderr to a tmp file, so
  it is not possible to get the processes stderr over the processes
  stderr pipe. I did’t find that this was an issue when testing ( other
  than having to clear out /tmp )

4) This could be expanded to other tests, outside of activation, to
  remove more usages of free port.

This is not yet complete, I just want to share the idea to see if it is a
runner, or not.

-Chris.





Re: RFR: 8165944 jar utility doesn't process more than one -C argument

2016-09-29 Thread Steve Drach
We discovered that the last webrev subtly changed the behavior of jar tool with 
respect to the JDK 8 jar tool, so that was fixed, along with some more 
simplification, and additional test cases were added to demonstrate consistent 
behavior across releases.  Here is the newest webrev.  

http://cr.openjdk.java.net/~sdrach/8165944/webrev.06/ 



> On Sep 27, 2016, at 12:31 PM, Steve Drach  wrote:
> 
> After a discussion with Paul Sandoz, I’ve simplified and, hopefully, thus 
> clarified the changeset.  The new webrev is
> 
> http://cr.openjdk.java.net/~sdrach/8165944/webrev.01/ 
> 
> 
>> On Sep 26, 2016, at 12:31 PM, Steve Drach > > wrote:
>> 
>> Hi,
>> 
>> Please review these changes to the jar tool to fix a capability regression I 
>> introduced in an earlier revision.  The issue is that this
>> 
>> $ jar -cf test.jar -C test1 . -C test2 .
>> 
>> only puts the files under test1 in the jar and ignores the files under 
>> test2.  The DoubleCs test verified the problem and the solution.
>> 
>> issue: https://bugs.openjdk.java.net/browse/JDK-8165944 
>>  
>> > >
>> webrev: http://cr.openjdk.java.net/~sdrach/8165944/webrev.00/ 
>>  
>> > >
>> 
>> Thanks,
>> Steve
>> 
> 



Re: Proposal to introduce method "Instrumentation.getInstance()" to instrument the current VM

2016-09-29 Thread Rafael Winterhalter
It would be perfectly fine, in my opinion, to throw an unsupported
operation exception, if the feature was unsupported. The method would
primarily be used by testing code and tools. In Mockito, we simply do not
offer inline mocks (but subclass mocks) if the runtime attachment fails.
EhCache or JOL also fall back to not offering the related feature. Adding
the method would simply add convenience and security to something that is
already done by many libraries and tools out there.

Also, all major VMs do support it as of today as far as I know. (The
exception being J9 which currently has an unfortunate bug that triggers an
internal assertion error upon retransformation.)

Am 29.09.2016 9:22 nachm. schrieb "Remi Forax" :

>
>
> On September 29, 2016 9:06:12 PM GMT+02:00, Alan Bateman <
> alan.bate...@oracle.com> wrote:
> >On 29/09/2016 18:50, Rafael Winterhalter wrote:
> >
> >> :
> >>
> >> Therefore I want to propose adding a static method to the
> >Instrumentation
> >> interface:
> >>
> >> interface Instrumentation {
> >>static Instrumentation getInstance(boolean redefine, boolean
> >retransform,
> >> boolean nativePrefix) {
> >>  // security manager check
> >>  return getInstance0(redefine, retransform, nativePrefix);
> >>}
> >> }
> >>
>
> I've a code that also does that for implementing a Repl like JShell.
>
> >The original intention, back in JSR 163, was that java.lang.instrument
> >be for instrumentation agents, not applications. This is why the API
> >deliberately does not include a method to get the Instrumentation
> >object
> >along the lines you have propose. Sure, you can leak it from an agent
> >started on the command line or attached at runtime but that is not the
> >original intention. So I think introducing this method is a very
> >significant change that would require a lot of consideration (previous
> >requests to provide a way for applications to get the Instrumentation
> >object without an agent in the picture were resisted).
> >
> >Separately, introducing this method creates a complication for runtimes
> >
> >that do not have JVM TI support (the JPLIS agent is based on JVM TI).
> >As
> >things stand then it's possible to create a runtime that does not have
> >a
> >means to start agents from the command-line (the spec allows this) and
> >so there is no need to have the implementation support for this API. So
> >
> >if a method like this is every introduced then it would either have to
> >be optional or it would require changes to the JVM TI spec to make it
> >non-optional (or is based on an alternative JPLIS implementation that
> >doesn't use JVM TI).
>
> Having it optional seams fine.
> Perhaps it has to be activated by a command line argument too.
>
> >
> >-Alan
>
> Rémi
>
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.
>


Re: Proposal to introduce method "Instrumentation.getInstance()" to instrument the current VM

2016-09-29 Thread Remi Forax


On September 29, 2016 9:06:12 PM GMT+02:00, Alan Bateman 
 wrote:
>On 29/09/2016 18:50, Rafael Winterhalter wrote:
>
>> :
>>
>> Therefore I want to propose adding a static method to the
>Instrumentation
>> interface:
>>
>> interface Instrumentation {
>>static Instrumentation getInstance(boolean redefine, boolean
>retransform,
>> boolean nativePrefix) {
>>  // security manager check
>>  return getInstance0(redefine, retransform, nativePrefix);
>>}
>> }
>>

I've a code that also does that for implementing a Repl like JShell.

>The original intention, back in JSR 163, was that java.lang.instrument 
>be for instrumentation agents, not applications. This is why the API 
>deliberately does not include a method to get the Instrumentation
>object 
>along the lines you have propose. Sure, you can leak it from an agent 
>started on the command line or attached at runtime but that is not the 
>original intention. So I think introducing this method is a very 
>significant change that would require a lot of consideration (previous 
>requests to provide a way for applications to get the Instrumentation 
>object without an agent in the picture were resisted).
>
>Separately, introducing this method creates a complication for runtimes
>
>that do not have JVM TI support (the JPLIS agent is based on JVM TI).
>As 
>things stand then it's possible to create a runtime that does not have
>a 
>means to start agents from the command-line (the spec allows this) and 
>so there is no need to have the implementation support for this API. So
>
>if a method like this is every introduced then it would either have to 
>be optional or it would require changes to the JVM TI spec to make it 
>non-optional (or is based on an alternative JPLIS implementation that 
>doesn't use JVM TI).

Having it optional seams fine.
Perhaps it has to be activated by a command line argument too.

>
>-Alan

Rémi 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


Re: [9] RFR of JDK-8085192: java/rmi/activation/Activatable tests fail intermittently due to "Port already in use"

2016-09-29 Thread Chris Hegarty
On 29 Sep 2016, at 16:25, Chris Hegarty  wrote:
> 
> I have asked Hamlin to hold off on this for a day or so.  I have an 
> alternative proposal that eliminates the free port anti-pattern.

It is possible to use the inheritedChannel mechanism to have the rmid
process create the server channel on an ephemeral port and report it
back to the test, i.e. remove the free port pattern.

http://cr.openjdk.java.net/~chegar/8085192_webrev/ 

1) The port number is reported from rmid to the test over stdout.

2) All tests pass except CheckAnnotations.java, which looks for stderr
( see 3 below ). I think the stderr check can be removed, and the
just check stdout.

3)  rmid, when using inheritChannel, redirects stderr to a tmp file, so
 it is not possible to get the processes stderr over the processes
 stderr pipe. I did’t find that this was an issue when testing ( other
 than having to clear out /tmp )

4) This could be expanded to other tests, outside of activation, to
 remove more usages of free port.

This is not yet complete, I just want to share the idea to see if it is a 
runner, or not.

-Chris.



Re: Proposal to introduce method "Instrumentation.getInstance()" to instrument the current VM

2016-09-29 Thread Alan Bateman

On 29/09/2016 18:50, Rafael Winterhalter wrote:


:

Therefore I want to propose adding a static method to the Instrumentation
interface:

interface Instrumentation {
   static Instrumentation getInstance(boolean redefine, boolean retransform,
boolean nativePrefix) {
 // security manager check
 return getInstance0(redefine, retransform, nativePrefix);
   }
}

The original intention, back in JSR 163, was that java.lang.instrument 
be for instrumentation agents, not applications. This is why the API 
deliberately does not include a method to get the Instrumentation object 
along the lines you have propose. Sure, you can leak it from an agent 
started on the command line or attached at runtime but that is not the 
original intention. So I think introducing this method is a very 
significant change that would require a lot of consideration (previous 
requests to provide a way for applications to get the Instrumentation 
object without an agent in the picture were resisted).


Separately, introducing this method creates a complication for runtimes 
that do not have JVM TI support (the JPLIS agent is based on JVM TI). As 
things stand then it's possible to create a runtime that does not have a 
means to start agents from the command-line (the spec allows this) and 
so there is no need to have the implementation support for this API. So 
if a method like this is every introduced then it would either have to 
be optional or it would require changes to the JVM TI spec to make it 
non-optional (or is based on an alternative JPLIS implementation that 
doesn't use JVM TI).


-Alan


Re: Proposal to introduce method "Instrumentation.getInstance()" to instrument the current VM

2016-09-29 Thread Aleksey Shipilev
On 09/29/2016 07:50 PM, Rafael Winterhalter wrote:
> I want to propose adding a method to the instrumentation API to receive an
> instance of the current VM's instrumentation API. Currently, many
> applications "self-attach" to gain such access. Unfortunately, this only
> works on JDK-VMs but I believe that this approach will grow in popularity
> with Java 9 where the Attach API is also part of any regular VM.

> interface Instrumentation {
>   static Instrumentation getInstance(boolean redefine, boolean retransform,
> boolean nativePrefix) {
> // security manager check
> return getInstance0(redefine, retransform, nativePrefix);
>   }
> }

I would like to second this proposal.

Our very own JOL [1] uses self-attach [2] to get Instrumentation
instance for carefully polling Object instance sizes. Self-attach
enables JOL usage as the library dependency without requiring command
line tweaks.

Thanks,
-Aleksey

[1] http://openjdk.java.net/projects/code-tools/jol/
[2]
http://hg.openjdk.java.net/code-tools/jol/file/b5653b56d154/jol-core/src/main/java/org/openjdk/jol/vm/InstrumentationSupport.java



Re: RFR: JEP draft for Linux/s3990x port

2016-09-29 Thread Vladimir Kozlov
You need to wait when Mark (OpenJDK Lead) move it to Candidate (or 
other) state:


http://cr.openjdk.java.net/~mr/jep/jep-2.0-02.html

Vladimir

On 9/29/16 9:55 AM, Volker Simonis wrote:

Hi Vladimir,

thanks a lot for reviewing and endorsing the JEP.

I've linked all the relevant issues to the JEP  (they all have a link
to a webrev) and change the state to "Submitted".

There's just one more small shared change we need for the port for
which we haven't opened a bug now because we are still working on
simplifying it. The current version looks as follows:

http://cr.openjdk.java.net/~simonis/webrevs/2016/s390x/916-constant_table_offset.patch

What are the next steps? Should I add a "jdk9-fc-request" label to t
he JEP and add a corresponding "FC Extension Request" comment to it?
Or will this be done automatically once I move it to "Candidate"?

Is there anything left to do before I can move it to "Candidate" state?

Thanks a lot and best regards,
Volker




On Tue, Sep 27, 2016 at 8:15 PM, Vladimir Kozlov
 wrote:

On 9/27/16 10:49 AM, Volker Simonis wrote:


Hi,

can you please review and endorse the following draft JEP for the
integration of the Linux/s390x port into the jkd9 master repository:

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



Good.
Add links to webrevs in a comment. It will help to get umbrella FC extension
approval.



As detailed in the JEP, the Linux/s390x requires very few shared
changes and we therefore don't foresee any impact on the existing
platforms at all. Following you can find a short description of the
planned changes:

hotspot:
===

Out for review:
8166560: [s390] Basic enablement of s390 port.
http://cr.openjdk.java.net/~goetz/wr16/8166560-basic_s390/hotspot.wr01/

Reviewed:
8166562: C2: Suppress relocations in scratch emit.
http://cr.openjdk.java.net/~goetz/wr16/8166562-scratch_emit/webrev.01/

Will send RFR soon (depends on 8166560):
8166561: [s390] Adaptions needed for s390 port in C1 and C2.
http://cr.openjdk.java.net/~goetz/wr16/8166562-scratch_emit/webrev.01



Wrong link.

Thanks,
Vladimir




We are still investigating the need of these shared changes:

http://cr.openjdk.java.net/~goetz/wr16/s390x_patch_queue/hotspot/911-pass_PC_to_retAddrOffset.patch

http://cr.openjdk.java.net/~goetz/wr16/s390x_patch_queue/hotspot/916-constant_table_offset.patch

And finally the patch with the s390x-only platform files. We are still
editing these to get them into OpenJdk style and shape.
Hotspot passes most jck, jtreg and spec tests with these.

http://cr.openjdk.java.net/~goetz/wr16/s390x_patch_queue/hotspot/9000101-zFiles.patch

top-level repository:
===

The following is just adding some s390x specific compiler flags to
flags.m4
8166800: [s390] Top-level build changes required for Linux/s390x
https://bugs.openjdk.java.net/browse/JDK-8166800

jdk repository:


This one just adds a new jvm.cfg file for s390x
8166801: [s390] Add jvm.cfg file for Linux/s390x
https://bugs.openjdk.java.net/browse/JDK-8166801


And finally we plan to do one more change which fixes the jtreg test
on Linux/s390x. But this is mainly for the correct detection of the
platform and for excluding the tests which are not appropriate for
s390x.

Thank you and best regards,
Volker





Proposal to introduce method "Instrumentation.getInstance()" to instrument the current VM

2016-09-29 Thread Rafael Winterhalter
Hello,

I want to propose adding a method to the instrumentation API to receive an
instance of the current VM's instrumentation API. Currently, many
applications "self-attach" to gain such access. Unfortunately, this only
works on JDK-VMs but I believe that this approach will grow in popularity
with Java 9 where the Attach API is also part of any regular VM.

Currently, such self-attachment is a rather flaky procedure:

1. Locate the "tools.jar" relatively to the Java Home.
2. Create a new URLClassLoader for this jar.
3. Locate the VirtualMachine type.
4. Parse the process Id from the JMX ManagementBean.
5. Load an agent that stores the instrumentation instance in a public field.
6. Locate this type from the class loader and read the field to receive the
instance.

I maintain a library offering an API for such self-attach and we can do
some amazing things with it. For example, the Mockito library (ca. 400k
downloads/month) now allows for mocking of final methods and types by
inlining the mocking logic into a class what avoids class creation to
create mocks by a subclass with virtual method overrides. Or cache
libraries can call the objectSize method to limit memory usage by a given
number in byte.

Due to the need of publicly exposing the instrumentation API in a public
field when using the above approach, this is however also a security risk
and the procedure is also less stable as it should be as it needs I/O. In
Java 9, this improves as there is an API for reading the current process id
and for accessing the classes of tools.jar but the situation is still far
from ideal. Within any library that uses for example EhCache, the instance
can be stolen by any application on the class path by simple (public)
reflection what then allows instrumenting the security manager to gain all
privileges.

Therefore I want to propose adding a static method to the Instrumentation
interface:

interface Instrumentation {
  static Instrumentation getInstance(boolean redefine, boolean retransform,
boolean nativePrefix) {
// security manager check
return getInstance0(redefine, retransform, nativePrefix);
  }
}

This would increase security as the instance is only available aftrer a
check and does not need to be exposed. Also, it would speed up the
attachment process and avoid the I/O involved. Finally, it would make
convenience APIs like the one I implemented unneccessary.

What do you think of this?

Thank you for considering my proposal.
Best regards, Rafael


Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-29 Thread Kumar Srinivasan


+1.

Kumar


On 29/09/2016 16:25, Erik Joelsson wrote:


Volker's comment above was directed at the suggestion of taking the
problematic AIX specific code out of the OpenJDK repositories and create
a separate library with a separate lifecycle somewhere else that OpenJDK
for AIX would then need to depend on. Volker was instead proposing what
you describe.


Ah right, in which case we are in violent agreement ;-)





Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-29 Thread Alan Burlison

On 29/09/2016 16:25, Erik Joelsson wrote:


Volker's comment above was directed at the suggestion of taking the
problematic AIX specific code out of the OpenJDK repositories and create
a separate library with a separate lifecycle somewhere else that OpenJDK
for AIX would then need to depend on. Volker was instead proposing what
you describe.


Ah right, in which case we are in violent agreement ;-)

--
Alan Burlison
--


Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-29 Thread Volker Simonis
On Thu, Sep 29, 2016 at 5:25 PM, Erik Joelsson  wrote:
>
>
> On 2016-09-29 16:54, Alan Burlison wrote:
>>
>> On 29/09/2016 08:03, Volker Simonis wrote:
>>
>>> Sorry, but that doesn't sound like a solution to me at all. I think we
>>> should keep the OpenJDK sources self-contained. I don't want to depend
>>> on yet another non-standard, third party library which doesn't even
>>> exist now.
>>
>>
>> Unless I'm completely misunderstanding, that's not what is being proposed.
>> What is being proposed is refactoring code that's currently duplicated
>> across the JVM & JDK into a common library. Such a library would be a
>> standard Java component, not non-standard and not third-party. I can't see
>> what the problem is, to be honest.
>>
> Volker's comment above was directed at the suggestion of taking the
> problematic AIX specific code out of the OpenJDK repositories and create a
> separate library with a separate lifecycle somewhere else that OpenJDK for
> AIX would then need to depend on. Volker was instead proposing what you
> describe.
>

Thanks Erik, this is exactly what I meant :)

And I think the solution you ssketched in your previous mail is the
right way to address this problem.

Regards,
Volker


> /Erik


Re: RFR: JEP draft for Linux/s3990x port

2016-09-29 Thread Volker Simonis
Hi Vladimir,

thanks a lot for reviewing and endorsing the JEP.

I've linked all the relevant issues to the JEP  (they all have a link
to a webrev) and change the state to "Submitted".

There's just one more small shared change we need for the port for
which we haven't opened a bug now because we are still working on
simplifying it. The current version looks as follows:

http://cr.openjdk.java.net/~simonis/webrevs/2016/s390x/916-constant_table_offset.patch

What are the next steps? Should I add a "jdk9-fc-request" label to t
he JEP and add a corresponding "FC Extension Request" comment to it?
Or will this be done automatically once I move it to "Candidate"?

Is there anything left to do before I can move it to "Candidate" state?

Thanks a lot and best regards,
Volker




On Tue, Sep 27, 2016 at 8:15 PM, Vladimir Kozlov
 wrote:
> On 9/27/16 10:49 AM, Volker Simonis wrote:
>>
>> Hi,
>>
>> can you please review and endorse the following draft JEP for the
>> integration of the Linux/s390x port into the jkd9 master repository:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8166730
>
>
> Good.
> Add links to webrevs in a comment. It will help to get umbrella FC extension
> approval.
>
>>
>> As detailed in the JEP, the Linux/s390x requires very few shared
>> changes and we therefore don't foresee any impact on the existing
>> platforms at all. Following you can find a short description of the
>> planned changes:
>>
>> hotspot:
>> ===
>>
>> Out for review:
>> 8166560: [s390] Basic enablement of s390 port.
>> http://cr.openjdk.java.net/~goetz/wr16/8166560-basic_s390/hotspot.wr01/
>>
>> Reviewed:
>> 8166562: C2: Suppress relocations in scratch emit.
>> http://cr.openjdk.java.net/~goetz/wr16/8166562-scratch_emit/webrev.01/
>>
>> Will send RFR soon (depends on 8166560):
>> 8166561: [s390] Adaptions needed for s390 port in C1 and C2.
>> http://cr.openjdk.java.net/~goetz/wr16/8166562-scratch_emit/webrev.01
>
>
> Wrong link.
>
> Thanks,
> Vladimir
>
>
>>
>> We are still investigating the need of these shared changes:
>>
>> http://cr.openjdk.java.net/~goetz/wr16/s390x_patch_queue/hotspot/911-pass_PC_to_retAddrOffset.patch
>>
>> http://cr.openjdk.java.net/~goetz/wr16/s390x_patch_queue/hotspot/916-constant_table_offset.patch
>>
>> And finally the patch with the s390x-only platform files. We are still
>> editing these to get them into OpenJdk style and shape.
>> Hotspot passes most jck, jtreg and spec tests with these.
>>
>> http://cr.openjdk.java.net/~goetz/wr16/s390x_patch_queue/hotspot/9000101-zFiles.patch
>>
>> top-level repository:
>> ===
>>
>> The following is just adding some s390x specific compiler flags to
>> flags.m4
>> 8166800: [s390] Top-level build changes required for Linux/s390x
>> https://bugs.openjdk.java.net/browse/JDK-8166800
>>
>> jdk repository:
>> 
>>
>> This one just adds a new jvm.cfg file for s390x
>> 8166801: [s390] Add jvm.cfg file for Linux/s390x
>> https://bugs.openjdk.java.net/browse/JDK-8166801
>>
>>
>> And finally we plan to do one more change which fixes the jtreg test
>> on Linux/s390x. But this is mainly for the correct detection of the
>> platform and for excluding the tests which are not appropriate for
>> s390x.
>>
>> Thank you and best regards,
>> Volker
>>
>


Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-29 Thread Chris Bensen
A lot of ideas have been thrown around so solve the problem of duplicated code 
in this situation. And there are other cases that are nearly identical so we 
need a general solution. The fact is this code should be moved to a common 
location. Since OpenJDK depends on Posix, Windows API and a few other non 
standard things that are mostly standard such as dladdr, it has been suggested 
by me and others that a library that doesn’t depend on anything else be created 
containing these methods to backfill the API that OpenJDK depends on if that 
API is not present on that platform. If that is a library outside OpenJDK or 
inside I’m not sure if that matters. Fact is there should be a library that 
contains method such as dladdr in cases such as AIX that do not have that 
method. Personally I’m not sure if the hack implementation of dladdr for AIX 
should be located in OpenJDK.

Chris


> On Sep 29, 2016, at 8:25 AM, Erik Joelsson  wrote:
> 
> 
> 
> On 2016-09-29 16:54, Alan Burlison wrote:
>> On 29/09/2016 08:03, Volker Simonis wrote:
>> 
>>> Sorry, but that doesn't sound like a solution to me at all. I think we
>>> should keep the OpenJDK sources self-contained. I don't want to depend
>>> on yet another non-standard, third party library which doesn't even
>>> exist now.
>> 
>> Unless I'm completely misunderstanding, that's not what is being proposed. 
>> What is being proposed is refactoring code that's currently duplicated 
>> across the JVM & JDK into a common library. Such a library would be a 
>> standard Java component, not non-standard and not third-party. I can't see 
>> what the problem is, to be honest.
>> 
> Volker's comment above was directed at the suggestion of taking the 
> problematic AIX specific code out of the OpenJDK repositories and create a 
> separate library with a separate lifecycle somewhere else that OpenJDK for 
> AIX would then need to depend on. Volker was instead proposing what you 
> describe.
> 
> /Erik



Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-29 Thread Erik Joelsson



On 2016-09-29 16:54, Alan Burlison wrote:

On 29/09/2016 08:03, Volker Simonis wrote:


Sorry, but that doesn't sound like a solution to me at all. I think we
should keep the OpenJDK sources self-contained. I don't want to depend
on yet another non-standard, third party library which doesn't even
exist now.


Unless I'm completely misunderstanding, that's not what is being 
proposed. What is being proposed is refactoring code that's currently 
duplicated across the JVM & JDK into a common library. Such a library 
would be a standard Java component, not non-standard and not 
third-party. I can't see what the problem is, to be honest.


Volker's comment above was directed at the suggestion of taking the 
problematic AIX specific code out of the OpenJDK repositories and create 
a separate library with a separate lifecycle somewhere else that OpenJDK 
for AIX would then need to depend on. Volker was instead proposing what 
you describe.


/Erik


Re: [9] RFR of JDK-8085192: java/rmi/activation/Activatable tests fail intermittently due to "Port already in use"

2016-09-29 Thread Chris Hegarty
I have asked Hamlin to hold off on this for a day or so.  I have an 
alternative proposal that eliminates the free port anti-pattern.

-Chris.

> On 29 Sep 2016, at 14:55, Roger Riggs  wrote:
> 
> Hi Hamlin,
> 
> One more suggested improvement.   Instead of two copy/paste copies of the 
> launch with options code,
> it would cleaner to create a separate RMID.launch(String[] options) method 
> that would be passed the extra arguments.
> Use it in forceLogSnapshot.java and ShutdownGracefully.java.
> 
> The (current) no-arg version could call the version with args with an empty 
> array(or null).
> There would be only 1 copy of the launch algorithm.
> 
> Roger
> 
> 
> 
> On 9/27/2016 11:22 PM, Hamlin Li wrote:
>> Hi Roger,
>> 
>> Thank you for reviewing.
>> Please check the new webrev: 
>> http://cr.openjdk.java.net/~mli/8085192/webrev.01/, and comments inline 
>> below.
>> 
>> On 2016/9/27 23:14, Roger Riggs wrote:
>>> Hi Hamlin,
>>> 
>>> Marking each test that uses RMID.launch with the bugid does not seem to be 
>>> meaningful
>>> since the bug is in the support infrastructure of the test and not specific 
>>> to the test itself.
>>> It would be overkill to try to confirm the bug was fixed by running all 
>>> those tests.
>>> Putting the bugid on 1 of the tests would be sufficient.
>> Remove all bug ids except of the one in CheckImplClassLoader.java.
>>> 
>>> JavaVM.java:
>>> 
>>> - 134-138:  Why define these private methods if they are not going to be 
>>> used in outputContains?
>>> 
>>> - 185:  Its inefficient to convert the byte array to a string for when 
>>> looking for each string.
>>>It would be cleaner for JavaVM to have public outputString and 
>>> errorString methods
>>>and check them separately in RMID.
>>>   (remove the JavaVM.outputContains method which hides which stream the 
>>> string appeared in).
>> Fixed.
>>> (It would be a bigger change to change this to use the test library 
>>> ProcessTools and OutputAnalyzer).
>> Thank you suggestion. Yes, you're right, it will be a little bit complicated 
>> to use ProcessTools and OutputAnalyzer in this situation, need to modify 
>> these classes, because they will block until process terminates. So I prefer 
>> to use current implementation, as it's simple and clear.
>> 
>> Thank you
>> -Hamlin
>>> 
>>> Roger
>>> 
>>> 
>>> On 9/27/2016 5:22 AM, Hamlin Li wrote:
 Please review the fix for JDK-8085192. The fix checks whether it fails to 
 launch rmid due to "Port already in use" error, it will launch rmid again 
 and again(20 times at most) until no such issue.
 
 bug:
https://bugs.openjdk.java.net/browse/JDK-8085192
 webrev:
http://cr.openjdk.java.net/~mli/8085192/webrev.00/
 
 Thank you
 -Hamlin
>>> 
>> 
> 



Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-29 Thread Alan Burlison

On 29/09/2016 08:03, Volker Simonis wrote:


Sorry, but that doesn't sound like a solution to me at all. I think we
should keep the OpenJDK sources self-contained. I don't want to depend
on yet another non-standard, third party library which doesn't even
exist now.


Unless I'm completely misunderstanding, that's not what is being 
proposed. What is being proposed is refactoring code that's currently 
duplicated across the JVM & JDK into a common library. Such a library 
would be a standard Java component, not non-standard and not 
third-party. I can't see what the problem is, to be honest.


--
Alan Burlison
--


Re: [9] RFR of JDK-8085192: java/rmi/activation/Activatable tests fail intermittently due to "Port already in use"

2016-09-29 Thread Roger Riggs

Hi Hamlin,

One more suggested improvement.   Instead of two copy/paste copies of 
the launch with options code,
it would cleaner to create a separate RMID.launch(String[] options) 
method that would be passed the extra arguments.

Use it in forceLogSnapshot.java and ShutdownGracefully.java.

The (current) no-arg version could call the version with args with an 
empty array(or null).

There would be only 1 copy of the launch algorithm.

Roger



On 9/27/2016 11:22 PM, Hamlin Li wrote:

Hi Roger,

Thank you for reviewing.
Please check the new webrev: 
http://cr.openjdk.java.net/~mli/8085192/webrev.01/, and comments 
inline below.


On 2016/9/27 23:14, Roger Riggs wrote:

Hi Hamlin,

Marking each test that uses RMID.launch with the bugid does not seem 
to be meaningful
since the bug is in the support infrastructure of the test and not 
specific to the test itself.
It would be overkill to try to confirm the bug was fixed by running 
all those tests.

Putting the bugid on 1 of the tests would be sufficient.

Remove all bug ids except of the one in CheckImplClassLoader.java.


JavaVM.java:

 - 134-138:  Why define these private methods if they are not going 
to be used in outputContains?


- 185:  Its inefficient to convert the byte array to a string for 
when looking for each string.
It would be cleaner for JavaVM to have public outputString and 
errorString methods

and check them separately in RMID.
   (remove the JavaVM.outputContains method which hides which stream 
the string appeared in).

Fixed.
(It would be a bigger change to change this to use the test library 
ProcessTools and OutputAnalyzer).
Thank you suggestion. Yes, you're right, it will be a little bit 
complicated to use ProcessTools and OutputAnalyzer in this situation, 
need to modify these classes, because they will block until process 
terminates. So I prefer to use current implementation, as it's simple 
and clear.


Thank you
-Hamlin


Roger


On 9/27/2016 5:22 AM, Hamlin Li wrote:
Please review the fix for JDK-8085192. The fix checks whether it 
fails to launch rmid due to "Port already in use" error, it will 
launch rmid again and again(20 times at most) until no such issue.


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

Thank you
-Hamlin








Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-29 Thread Peter Levart



On 09/29/2016 02:58 AM, Vitaly Davidovich wrote:

On Wednesday, September 28, 2016, Carsten Varming 
wrote:


Dear David,

See inline.

On Wed, Sep 28, 2016 at 7:47 PM, David Holmes > wrote:


On 29/09/2016 3:44 AM, Carsten Varming wrote:


Dear Vitaly and David,

Looking at your webrev the original code is:

public int hashCode() {
   if (hash == 0 && value.length > 0) {
 hash = isLatin1() ? StringLatin1.hashCode(value)
   }
   return hash;
}

There is a constructor:

public String(String original) {
   this.value = original.value;
   this.coder = original.coder;
   this.hash = original.hash;
}

that might write zero to the mutable field "hash".

The object created by this constructor might be shared using plain reads
and writes between two threads[1] and the write of 0 in the constructor
might be interleaved with the reads and write in hashCode. Does this
capture the problem?


Because String has final fields there is a freeze action at the end of
construction so that String instances are always safely published even if
not "safely published".


I always thought that the freeze action only freezes final fields. The
hash field in String is not final and example 17.5-1 is applicable as far
as I can see (https://docs.oracle.com/javase/specs/jls/se8/html/jls-
17.html#jls-17.5). Has the memory model changed in JDK9 to invalidate
example 17.5-1 or I am missing something about String.


Right - the spec only talks about final fields being ok in such cases.  In
practice, compilers put a freeze action at the end of the constructor and
non-final fields come along for the ride, AFAIK.

However, I'm not sure this example is all that interesting.  Suppose you
published a String copied from another, but the hash value was reordered
and didn't appear to the reader.  That ought to be fine because hashCode is
supposed to recompute it.  At worst, you perform the same computation
"unnecessarily".

The possible compiler reordering, as discussed in this thread, within
hashCode itself is problematic because it can, theoretically, yield 0
erroneously.


I think Carsten has a point. If we bring this constructor to the picture:

public String(String original) {
this.value = original.value;
this.coder = original.coder;
this.hash = original.hash;
}

and combine it with the hashCode implementation:

public int hashCode() {
  if (hash == 0 && value.length > 0) {
hash = isLatin1() ? ...
  }
  return hash;
}

...then we can get an erroneous result of 0 from hashCode() even without 
compiler (or hardware) reordering reads in hashCode():


Possible execution:

Thread 3:
 hash = isLatin1() ? ; // write non-zero

Thread 1:
if (hash == 0 && ...) // false, skip assignment

Thread 2:
this.hash = original.hash; // zero (hash not computed yet in original)

Thread 1:
return hash; // returns zero


This would of course require the reference to String constructed by 
Thread 2 to be unsafely published to Thread 1 & 3 before this.hash in 
constructor is written, meaning that "freeze" action for final fields 
would have to be performed immediately after the write of last final 
field (coder). And that's another possibility that JMM allows.


I think that this is enough to apply the fix, thanks.

Regards, Peter


Carsten



David


[1]: Meaning the is no happens-before relationship established between

object construction and another thread calling hashCode on the object.

Carsten

On Wed, Sep 28, 2016 at 10:13 AM, Vitaly Davidovich 
> wrote:

 On Wednesday, September 28, 2016, David Holmes
  >>
 wrote:

 > On 28/09/2016 10:44 PM, Peter Levart wrote:
 >
 >> Hi,
 >>
 >> According to discussion here:
 >>
 >> http://cs.oswego.edu/pipermail/concurrency-interest/2016-
 
 >> September/015414.html
 >>
 >>
 >> it seems compact strings introduced (at least theoretical)
non-benign
 >> data race into String.hasCode() method.
 >>
 >> Here is a proposed patch:
 >>
 >> http://cr.openjdk.java.net/~plevart/jdk9-dev/8166842_String
 .

 >> hashCode/webrev.01/
 >>
 >
 > I'm far from convinced that the bug exists - theoretical or
 otherwise -
 > but the "fix" is harmless.
 >
 > When we were working on JSR-133 one of the conceptual models is
 that every
 > write to a variable goes into the set of values that a read may
 potentially
 > return (so no out-of-thin-air for example). happens-before
establishes
 > constraints on which value can legally be returned - the most
 recent. An
 > additional property was that once a value was returned, a later
 read could
 > not return an earlier value - in essence once a read returns a
 given value,
 > all earlier written values are removed from the set of potential
 values
 > that c

Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-29 Thread Erik Joelsson

Hello,

From my point of view, now that I better understand what aix/porting 
actually was/is, I would say go for it. Put something together the way 
you would like it. I doubt there will ever be much code needed in this 
new entity so it can go in the top level repo without problems. There is 
already a common/src you can sort it into. I assume you will want to 
build a static lib that is easily picked up by other libraries and 
provide an include directory. Put a new makefile in /make that 
builds this, create a new top level target in /make/Main.gmk that 
calls it. Build it into /support/native/. 
Since this is only for AIX now, surround it with proper ifeqs. Doesn't 
really seem that complicated to me. :)


Neither hotspot nor JDK is buildable on their own now, so there is no 
problem putting dependencies on either at the top level.


/Erik

On 2016-09-29 09:03, Volker Simonis wrote:

On Wed, Sep 28, 2016 at 8:54 PM, Chris Bensen  wrote:

On Sep 28, 2016, at 11:50 AM, Thomas Stüfe  wrote:



On Wed, Sep 28, 2016 at 7:33 PM, Martin Buchholz 
wrote:

On Wed, Sep 28, 2016 at 9:33 AM, Volker Simonis 
wrote:


I don't think this can be easily done with the current build system.
Remember for example that even such a sensitive part like jni.h is
still duplicated between the hotspot and the jdk repository:

hotspot/src/share/vm/prims/jni.h
jdk/src/java.base/share/native/include/jni.h


It's one of the frustrating aspects of openjdk development that it's hard
to share C level infrastructure among different components.  Components
sometimes grow their own local C infrastructure, but when another
component
has the same problem, one often resorts to copy-paste as the most
expedient
way to get code reuse.  In part, the mercurial repo organization
reinforces
this - there is one top-level repo with fan-out, but there is nothing at
the bottom with fan-in.


At SAP, we often solve this by moving code to the hotspot and exporting it
from there, the hotspot being the central part which (almost) has to be
loaded from the very beginning. I think we did this for dladdr() too.


As I said before, this obviously can't work for libjli which is used
to dynamically load the libjvm.


But that is an ugly hack too, because it bloats the hotspot and forces us to
add -ljvm to a lot of makefiles or to resolve those functions dynamically.

Another solution for us on AIX could be to put this stuff into an own
library and provide it independently from the OpenJDK build system, just
declare it to be a build dependency, similar to Apples JavaRuntimeServices.

As we need dladdr() in both hotspot and JDK, maybe that would be the best
option.


That sounds like a reasonable solution to me.


Sorry, but that doesn't sound like a solution to me at all. I think we
should keep the OpenJDK sources self-contained. I don't want to depend
on yet another non-standard, third party library which doesn't even
exist now.

The solution proposed by Martin could work, but it is not great from a
software engineering perspective either because it doesn't allow
sharing of header files.

What would be actually needed would be a new top-level
repository/directory (call it 'base' or 'porting' or whatsoever) which
should be organized like for example jdk/java.base/src (i.e. have
'share/', 'unix/', 'windows/', etc. subdirectories.

The artifacts from this new directory would be build as the very first
step of the build process (if necessary) and the results, as well as
the source directories themselves would have to be made available to
all the other build steps (in particular to. hotspot and jdk). In fact
I had implemented a similar solution for the jdk repository in order
to stay with a single copy of dladdr on AIX  (i.e.
jdk/src/aix/porting). But this solution had to go away when the
sources were modularized because the new schema doesn't allow sharing
of files between modules anymore :(

However, I'm not sure if we want to start such a project right now?


Chris





One code sharing mechanism that does get used is seen in
ClassLoader::load_zip_library()
where code from the jdk repo is packaged into a shared object and invoked
from hotspot, dynamically.







Re: RFR: 8166842: String.hashCode() has a non-benign data race

2016-09-29 Thread Andrew Haley
On 29/09/16 05:31, David Holmes wrote:
> 
> On 29/09/2016 10:49 AM, Carsten Varming wrote:

>> Because String has final fields there is a freeze action at the end
>> of construction so that String instances are always safely published
>> even if not "safely published".
>>
>>
>> I always thought that the freeze action only freezes final fields. The
>> hash field in String is not final and example 17.5-1 is applicable as
>> far as I can see
>> (https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5). Has
>> the memory model changed in JDK9 to invalidate example 17.5-1 or I am
>> missing something about String.
> 
> Sorry - I was confusing what the spec says versus what the VM actually 
> does - as Vitaly pointed out.

A HotSpot back end could rewrite the constructor so that the last
final field to be written used a store release instruction.  Whether
it should is another matter: I suppose it would be rather risky.

Andrew.




Re: RFR: 8166189: Fix for Bug 8165524 breaks AIX build

2016-09-29 Thread Volker Simonis
On Wed, Sep 28, 2016 at 8:54 PM, Chris Bensen  wrote:
>
> On Sep 28, 2016, at 11:50 AM, Thomas Stüfe  wrote:
>
>
>
> On Wed, Sep 28, 2016 at 7:33 PM, Martin Buchholz 
> wrote:
>>
>> On Wed, Sep 28, 2016 at 9:33 AM, Volker Simonis 
>> wrote:
>>
>> >
>> > I don't think this can be easily done with the current build system.
>> > Remember for example that even such a sensitive part like jni.h is
>> > still duplicated between the hotspot and the jdk repository:
>> >
>> > hotspot/src/share/vm/prims/jni.h
>> > jdk/src/java.base/share/native/include/jni.h
>> >
>>
>> It's one of the frustrating aspects of openjdk development that it's hard
>> to share C level infrastructure among different components.  Components
>> sometimes grow their own local C infrastructure, but when another
>> component
>> has the same problem, one often resorts to copy-paste as the most
>> expedient
>> way to get code reuse.  In part, the mercurial repo organization
>> reinforces
>> this - there is one top-level repo with fan-out, but there is nothing at
>> the bottom with fan-in.
>
>
> At SAP, we often solve this by moving code to the hotspot and exporting it
> from there, the hotspot being the central part which (almost) has to be
> loaded from the very beginning. I think we did this for dladdr() too.
>

As I said before, this obviously can't work for libjli which is used
to dynamically load the libjvm.

> But that is an ugly hack too, because it bloats the hotspot and forces us to
> add -ljvm to a lot of makefiles or to resolve those functions dynamically.
>
> Another solution for us on AIX could be to put this stuff into an own
> library and provide it independently from the OpenJDK build system, just
> declare it to be a build dependency, similar to Apples JavaRuntimeServices.
>
> As we need dladdr() in both hotspot and JDK, maybe that would be the best
> option.
>
>
> That sounds like a reasonable solution to me.
>

Sorry, but that doesn't sound like a solution to me at all. I think we
should keep the OpenJDK sources self-contained. I don't want to depend
on yet another non-standard, third party library which doesn't even
exist now.

The solution proposed by Martin could work, but it is not great from a
software engineering perspective either because it doesn't allow
sharing of header files.

What would be actually needed would be a new top-level
repository/directory (call it 'base' or 'porting' or whatsoever) which
should be organized like for example jdk/java.base/src (i.e. have
'share/', 'unix/', 'windows/', etc. subdirectories.

The artifacts from this new directory would be build as the very first
step of the build process (if necessary) and the results, as well as
the source directories themselves would have to be made available to
all the other build steps (in particular to. hotspot and jdk). In fact
I had implemented a similar solution for the jdk repository in order
to stay with a single copy of dladdr on AIX  (i.e.
jdk/src/aix/porting). But this solution had to go away when the
sources were modularized because the new schema doesn't allow sharing
of files between modules anymore :(

However, I'm not sure if we want to start such a project right now?

> Chris
>
>
>
>
>>
>> One code sharing mechanism that does get used is seen in
>> ClassLoader::load_zip_library()
>> where code from the jdk repo is packaged into a shared object and invoked
>> from hotspot, dynamically.
>
>
>