RFR: 8167002: JAXP schema validator: Use HashSet instead of ArrayList for tracking XML IDs

[Martin Buchholz]

https://bugs.openjdk.java.net/browse/JDK-8167002
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/xml-id-validation-by-hash/

Re: RFR: 8166840: Synthetic bridge constructor in ArrayList$Itr blocks inlining

[Stuart Marks]

On 9/28/16 4:48 AM, Claes Redestad wrote:

as discussed recently on hotspot-compiler-dev[1], having a private class with no
default constructor can lead to C2 failing to inline, due to the synthetic
bridge constructor using a dummy argument of an uninitialized class. This is
really a problem in C2, as well as something which could ultimately be resolved
by nestmates...

However, there is an easy workaround in adding an empty package-private
constructor. In the most recently found case - a microbenchmark stressing
MethodHandles.iteratedLoop - adding this to ArrayList$Itr lead to a 2.5-3x 
speedup.

This is me asking nicely to do a quick-fix for this in java.util.ArrayList$Itr 
now:

Bug: https://bugs.openjdk.java.net/browse/JDK-8166840
Webrev: http://cr.openjdk.java.net/~redestad/8166840/webrev.01/

[1]
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-September/024407.html


Hi, adding the package-access constructor seems like a reasonable thing to do.

However, should there be a comment explaining this? It seems like just having a 
no-op constructor is something that is liable to be cleaned up by a well-meaning 
individual who didn't happen to see this thread. Something like this:


// avoid creation of synthetic class and bridge constructor
Itr() {}

If this really is a problem in C2, should there also be a reference to a Hotspot 
bug that represents this problem?


On the other hand, addition of the constructor does remove a class from the 
runtime image, so maybe it's worth it to keep around even if the Hotspot bug is 
fixed. But having a comment would be good to prevent this code from being 
"cleaned up".


s'marks

Re: Collection Design FAQ

[Stuart Marks]

On 9/22/16 11:13 PM, Kasper Nielsen wrote:

I accidently bumped into an old friend today
https://docs.oracle.com/javase/8/docs/technotes/guides/collections/designfaq.html
There are couple of questions that might need an update after Java 8. Such
as

Why don't you provide an "apply" method in Collection to apply a given
method ("upcall") to all the elements of the Collection?
Why didn't you provide a "Predicate" interface, and related methods (e.g.,
a method to find the first element in the Collection satisfying the
predicate)?


Hi Kasper,

The collection design FAQ is certainly out of date.

Regarding an "apply" method, as Rémi mentioned, there is Iterable.forEach. For 
List, there is also List.replaceAll.


It was certainly possible to have a Predicate interface before Java 8, but the 
only way to use it would have been to use an anonymous inner class (or a 
pre-existing class that implemented it). This would have been pretty cumbersome 
to use. Of course, this is all different now that we have lambdas.


It's pretty straightforward to use streams to find the first matching element of 
a List, but less so to find the index of that element, like List.indexOf or 
List.lastIndexOf. Would it be useful if something like the following were added 
to List?


int findIndexMatching(Predicate)
int findLastIndexMatching(Predicate)

(doesn't have to be these names, of course)

Or is it sufficient to stream over the list indexes and search that way?

IntStream.range(0, list.size())
 .filter(i -> predicate.test(list.get(i)))
 .findFirst();

s'marks

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

[Stuart Marks]

On 9/30/16 12:41 PM, John Rose wrote:

On Sep 29, 2016, at 11:39 PM, Louis Wasserman  wrote:


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.


Stuart Marks and Brian Goetz gave an excellent talk on this at JavaOne last 
week.

You can view it here.  The talk is called "Thinking in Parallel".
https://youtu.be/WSxKI5S8j90?t=8h51m34s 


InfoQ wrote a useful summary of the talk, for those with no time for video:
https://www.infoq.com/news/2016/09/JavaOne-2016-parallel-streams 


But the video is very enjoyable, even if you think you know what they are going 
to say.

— John

P.S.  One influence of this year's presentation is an epic manifesto by Guy 
Steele in 2010.
https://www.infoq.com/presentations/Thinking-Parallel-Programming 




John, thanks for the plug!

If you just want to look at the slides, they're linked from this post on my 
blog:

https://stuartmarks.wordpress.com/2016/09/23/javaone-2016-sessions/

s'marks

RFR: 6378384 (reflect) subclass can’t access superclass’s protected fields and methods by reflection

[Peter Levart]

Hi,

I have a fix for a 10 year old bug (JDK-6378384 
). It was marked as a 
duplicate of a 19 year old bug (JDK-4032740 
) which is marked as a 
duplicate of a 17 year old bug (JDK-4283544 
) which is still open. 
But this bug is not a strict duplicate. This bug only concerns 
reflective access to protected members.


Here's a proposed fix:

http://cr.openjdk.java.net/~plevart/jdk9-dev/6378384_Reflection.ensureAccess/webrev.01/


The bug manifests itself as not being able to access protected static 
methods or fields from a subclass located in a different package. 
Instance protected methods and fields can be accessed, and using an 
undocumented trick, also static methods and fields, but the trick is 
very subtle. The specification for Field.get/set and Method.invoke says, 
respectively:


 * If the underlying field is a static field, the {@code obj} 
argument

 * is ignored; it may be null.

and:

 * If the underlying method is static, then the specified {@code 
obj}

 * argument is ignored. It may be null.

Well, it is not exactly so! The obj argument is used as a 'target' even 
for protected static members and it is ensured that its class is equal 
or a subclass of the class that accesses the member. So if you pass an 
instance of a subclass of the protected method's declaring class into 
the get/set/invoke, you can access the static protected member. If you 
pass 'null', you get IllegalAccessException.


The problem is in the design of 
jdk.internal.reflect.Reflection#ensureMemberAccess method which is used 
to check reflective access. It takes an Object 'target' argument, which 
is supposed to be null when accessing static methods/fields and it is 
null also when accessing constructors. Because of constructors and the 
method's API, it has to be overly restrictive as it must only allow 
calling protected constructors from within the constructor's declaring 
class itself or same package, while protected static methods could be 
called from any subclass.


By redesigning the API of this method, replacing Object 'target' 
parameter with Class 'targetClass' parameter and by passing the 
constructor's declaring class into this method instead of null, 
reflective checks suddenly start to look more like JLS dictates (but 
still a long way from it, unfortunately).


As a bonus, sun.reflect.misc.ReflectUtil#ensureMemberAccess method (used 
from AtomicXXXFieldUpdater classes) does not need the following comment 
any more:


 * Reflection.ensureMemberAccess is overly-restrictive
 * due to a bug. We awkwardly work around it for now.

...as it can now delegate straight to Reflection.ensureMemberAccess 
without invoking it twice with different modified member access 
modifiers and performing part of the check itself.


java.lang.reflect.AccessibleObject#checkAccess delegates to 
Reflection.ensureMemberAccess and caches the result, so it had to be 
modified too.


Constructor now passes it's declaring class to the 'targetClass' 
parameter and Filed/Method obey the spec and REALLY IGNORE 'obj' 
parameter in get/set/invoke on a static member.


All java/lang/reflect and java/util/concurrent/atomic tests pass with 
this patch applied except the following:


java/lang/reflect/AccessibleObject/ModuleSetAccessibleTest.java: Test 
java.lang.reflect.AccessibleObject with modules
java/lang/reflect/Generics/TestBadSignatures.java: Test bad signatures 
get a GenericSignatureFormatError thrown.
java/lang/reflect/Method/invoke/TestPrivateInterfaceMethodReflect.java: 
Reflection support for private methods in interfaces
java/lang/reflect/Module/AddExportsTest.java: Test Module isExported 
methods with exports changed by -AddExportsTest
java/lang/reflect/Proxy/ProxyModuleMapping.java: Basic test of proxy 
module mapping and the access to Proxy class

java/lang/reflect/WeakPairMap/Driver.java: Functional test for WeakPairMap

...which all fail because of:

javac: invalid flag: -XaddExports:java.base/jdk.internal



Regards, Peter

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

[John Rose]

On Sep 29, 2016, at 11:39 PM, Louis Wasserman  wrote:
> 
> 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.

Stuart Marks and Brian Goetz gave an excellent talk on this at JavaOne last 
week.

You can view it here.  The talk is called "Thinking in Parallel".
https://youtu.be/WSxKI5S8j90?t=8h51m34s 


InfoQ wrote a useful summary of the talk, for those with no time for video:
https://www.infoq.com/news/2016/09/JavaOne-2016-parallel-streams 


But the video is very enjoyable, even if you think you know what they are going 
to say.

— John

P.S.  One influence of this year's presentation is an epic manifesto by Guy 
Steele in 2010.
https://www.infoq.com/presentations/Thinking-Parallel-Programming 

RFR 9: 8155760 Implement Serialization Filtering - revised for extensibility

[Roger Riggs]

Converging on a final version.

The ObjectInputFilter.FilterInfo.getClazz method was renamed to 
serialClass to improve readability
and reduce ambiguity with getClass. A few other editorial improvements 
have been made in the docs
of the FilterInfo interface for arrays, depth(), references(), and 
streamBytes() with respect to the initial values.


Any additional review comments?


Complete webrev supporting Serialization Filtering:
http://cr.openjdk.java.net/~rriggs/webrev-serial-filter-jdk9-8155760/

The specdiff of changes supporting evolving the API more easily:
http://cr.openjdk.java.net/~rriggs/filter-diffs-8166739/overview-summary.html

Javadoc (subset)
http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/ObjectInputStream.html 

http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/ObjectInputFilter.html 

http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/SerializablePermission.html 



Thanks, Roger

RE: Proposal for adding O_DIRECT support into JDK 9

[Lu, Yingqi]

Hi All,

Please find the most recent version of the patch available at 
http://cr.openjdk.java.net/~igraves/8164900-2/

In this version, we have following two changes:

1. Move O_DIRECT flag from StandardOpenOption to ExtendedOpenOption
2. Move the checks of O_DIRECT from native code to Java code.

Please let us know your feedback.

Thanks,
Lucy

-Original Message-
From: nio-dev [mailto:nio-dev-boun...@openjdk.java.net] On Behalf Of Lu, Yingqi
Sent: Tuesday, September 27, 2016 9:57 AM
To: Alan Bateman ; core-libs-dev@openjdk.java.net
Cc: nio-...@openjdk.java.net
Subject: RE: Proposal for adding O_DIRECT support into JDK 9

Alan,

Thank you for the explanation, we will modify the code accordingly and send it 
out soon for review.

Thanks,
Lucy

-Original Message-
From: Alan Bateman [mailto:alan.bate...@oracle.com] 
Sent: Tuesday, September 27, 2016 8:45 AM
To: Lu, Yingqi ; core-libs-dev@openjdk.java.net
Cc: nio-...@openjdk.java.net
Subject: Re: Proposal for adding O_DIRECT support into JDK 9

On 26/09/2016 19:50, Lu, Yingqi wrote:

> Alan, you mean readv0/write0 or read0/write0? I just want to make sure 
> :-)
Apologies, I meant each of the native methods where the decision to use direct 
I/O or not would be a lot more maintainable in Java. You'll see that there are 
already code paths for direct vs. heap buffers.


>
> Anyone else has other opinions on where is the best home for O_DIRECT flag? 
> The flags under jdk.unsupported will eventually be removed in the future JDK 
> release?
>
> If we agree ExtendedOpenOpen is the best home for O_DIRECT, we can modify 
> that for sure.
>
I think ExtendedOpenOption is the right place. It's still TDB as to whether to 
put these extensions but that should be transparent to anyone using this when 
on the class path.

-Alan

Re: [9] Review Request: 8143077 Deprecate InputEvent._MASK in favor of InputEvent._DOWN_MASK

[Jonathan Bluett-Duncan]

Hi Sergey,

I'm not a reviewer, but after reading the deprecation messages in Event.java

* @deprecated It is recommended that {@code AWTEvent} class and its
> subclasses
> * be used instead.


I get the impression they would read better without the redundant "class"
in the middle, like so.

* @deprecated It is recommended that {@code AWTEvent} and its subclasses
> * be used instead.


Kind regards,
Jonathan


On 30 September 2016 at 16:45, Sergey Bylokhov 
wrote:

> Hello.
>
> Please review the fix for jdk9.
>
> This is request to deprecate the obsolete flags inside InputEvent. This
> constants were marked as obsolete in jdk1.4 and were replaced by the new
> one. In jdk1.4 the documentation were update with notion that the new
> constants should be used. And this bug is about official deprecation of
> them.
>
> We can replace old constants by the new one in the whole jdk, but I
> updated only the code in InputEvent to make change smaller and safer. So at
> least the new code in jdk and the code in applications will start to use
> the new constants.
>
> The changes in jconsole are necessary to fix deprecation warning.
>
> jprt build passed, no new issues were found by jck/jtreg tests.
>
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8143077
> Webrev can be found at: http://cr.openjdk.java.net/~serb/8143077/webrev.00
>
> --
> Best regards, Sergey.
>

Re: [9] Review Request: 8143077 Deprecate InputEvent._MASK in favor of InputEvent._DOWN_MASK

[Mandy Chung]

The jconsole change looks fine.  I’m including serviceability-dev and bcc 
core-libs-dev as serviceability-dev is the right mailing list for jconsole 
change.

Mandy

> On Sep 30, 2016, at 8:45 AM, Sergey Bylokhov  
> wrote:
> 
> Hello.
> 
> Please review the fix for jdk9.
> 
> This is request to deprecate the obsolete flags inside InputEvent. This 
> constants were marked as obsolete in jdk1.4 and were replaced by the new one. 
> In jdk1.4 the documentation were update with notion that the new constants 
> should be used. And this bug is about official deprecation of them.
> 
> We can replace old constants by the new one in the whole jdk, but I updated 
> only the code in InputEvent to make change smaller and safer. So at least the 
> new code in jdk and the code in applications will start to use the new 
> constants.
> 
> The changes in jconsole are necessary to fix deprecation warning.
> 
> jprt build passed, no new issues were found by jck/jtreg tests.
> 
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8143077
> Webrev can be found at: http://cr.openjdk.java.net/~serb/8143077/webrev.00
> 
> -- 
> Best regards, Sergey.

[9] Review Request: 8143077 Deprecate InputEvent._MASK in favor of InputEvent._DOWN_MASK

[Sergey Bylokhov]

Hello.

Please review the fix for jdk9.

This is request to deprecate the obsolete flags inside InputEvent. This 
constants were marked as obsolete in jdk1.4 and were replaced by the new 
one. In jdk1.4 the documentation were update with notion that the new 
constants should be used. And this bug is about official deprecation of 
them.


We can replace old constants by the new one in the whole jdk, but I 
updated only the code in InputEvent to make change smaller and safer. So 
at least the new code in jdk and the code in applications will start to 
use the new constants.


The changes in jconsole are necessary to fix deprecation warning.

jprt build passed, no new issues were found by jck/jtreg tests.


Bug: https://bugs.openjdk.java.net/browse/JDK-8143077
Webrev can be found at: http://cr.openjdk.java.net/~serb/8143077/webrev.00

--
Best regards, Sergey.

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

[Peter Levart]

Hi,

Thank you all for reviews and comments, especially instructive was Hans 
Boehm's explanation about why JMM must allow reordering of plain reads 
from the same location.


The fix is now pushed.

Regards, Peter

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

[Andrew Dinn]

On 30/09/16 14:55, Rafael Winterhalter wrote:
> That does not work in the general case but only if the initiating class
> is also on the class path. A class can only be identified uniquely as a
> pair of name and class loader.
> 
> I experimented with this and ended up iterating over
> Instrumentation.getAllLoadedClasses which resulted in rather poor
> performance and ambiguity if there exist classes with the same name on
> different class loaders.

Yeah but don't you still have your ByteBuddy API class, the one which
initiates the load in the class path? Your agent can still push the
instance to that class by calling a setter it provides and it can hand
the instance out to classes which are allowed to use it.

So, then you would have to stop any old clients from calling your API to
retrieve an Instrumentation instance, for example by setting up a
security manager. Otherwise it makes no difference whether the
Instrumentation instance is accessible via a public static field or is
handed out on demand by your API. You have set up this circumstance by
providing a library which hands out an Instrumentation instance on demand.

What I don't understand is why you think migrating this into the JDK
changes things. You suggested that this would also need a security
manager to control access. If that's needed to safeguard the API on
Instrumentation then why is i tnot good enough to use the same mechanism
to restrict access to your ByteBuddy API class?

regards,


Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

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

[Carsten Varming]

Dear Peter,

On Thu, Sep 29, 2016 at 5:31 AM, Peter Levart 
wrote:

> I think Carsten has a point.
> I think that this is enough to apply the fix, thanks.
>

You are welcome. I added my thoughts to the jira ticket.

Carsten

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

[Martin Buchholz]

On Thu, Sep 29, 2016 at 2:31 AM, Peter Levart 
wrote:

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

I think general concurent software hygiene considerations are enough to
commit this fix.  Don't re-read fields, especially ones that are involved
in a race!  Finding an actual bug is awesome, but it's not necessary to get
approval, at least from this reviewer!

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

[Jonathan Bluett-Duncan]

Hi Stuart,

Thanks for doing such a thorough review of the parts you've managed to look
at so far.

I see you had a number of questions in your numbered bullet points, so I'll
do my best to answer them below.

1) It actually didn't occur to me that there was a potential TOCTOU problem
in ModuleFinder.compose, despite the fact that I found a potential problem
in FileTreeIterator - it completely missed me, so thank you for finding it!
I'll see if I can put a similar comment there to what I wrote in
FileTreeIterator.

2) I seem to remember doing an at least semi-thorough search of the JDK
codebase, and coming to the conclusion that none of the code I touched was
being serialized by the JDK itself. I think I mentioned this is a previous
email, but I also remember saying that I wasn't sure if I took everything
into account, as I'm not that familiar with serialization. So I decided
just now to do a search on Grepcode for usages of CookieManager.get

and
CookieHandler.get
,
and curiously it looks like they're only used in sun classes in the JDK. So
this change seems safe to me, unless I've missed something?

Regarding code search engines, I know of sourcegraph.com, but I think it
requires an account to use their service, and I don't know which
repositories they index apart from GitHub.

3) In my local copy of jdk9, I've removed the TOCTOU comment in
FileTreeIterator and changed List.of back to Arrays.asList, as your
explanation regarding FileTreeWalker has convinced me that TOCTOU is not a
real problem here, and I don't know if future memory improvements to
List.of(T...) (like returning an ImmutableCollections.List1 when there's
only 1 element) are worth the extra time spent copying into a new list.

4) The 'resolverFields' problem/comment was regarding DateTimeFormatter
(see this part of latest webrev
),
where I realised I couldn't use Set.of instead of
Collections.unmodifiableSet(new HashSet<>(Arrays.asList(*))), because I
noticed that one of the java.time tests was testing whether
DateTimeFormatter.withResolverFields(TemporalField...) could accept null
parameters, which made using Set.of impossible since it's null-hostile (as
in it throws NullPointerException upon being constructed with null
element(s)).

So I never did submit a change with that bit of code replaced with Set.of.
Instead I wrote a comment in the method explaining why one can't use
Set.of. I don't really know if Stephen Colebourne saw that comment, though.

Stephen Colebourne, are you still fine with all the changes I've made to
the java.time classes? http://cr.openjdk.java.net/~reinhapa/reviews/8134373/
webrev.01/

Kind regards,
Jonathan

P.S. I'll wait until everything's reviewed before asking for a new webrev.

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

[Rafael Winterhalter]

That does not work in the general case but only if the initiating class is
also on the class path. A class can only be identified uniquely as a pair
of name and class loader.

I experimented with this and ended up iterating over
Instrumentation.getAllLoadedClasses which resulted in rather poor
performance and ambiguity if there exist classes with the same name on
different class loaders.

Again, I argue that this problem is therefore worth addressing.

Best regards, Rafael

2016-09-30 15:51 GMT+02:00 Andrew Dinn :

> On 30/09/16 14:27, Rafael Winterhalter wrote:
> > A Java agent ends up on the class path. The Byte Buddy agent (or any
> > similar library) basically adds a single class:
> >
> > package net.bytebuddy.agent;
> > public class Installer {
> >   public static volatile Instrumentation instrumentation;
> >   public static void agentMain(String argument, Instrumentation
> > instrumentation) {
> > Agent.instrumentation = instrumentation
> >   }
> > }
> >
> > Since the class is added using self-attachment, the agentmain method is
> > called by the VM and the field value is set. In order to keep the field
> > accessible, it needs to be public such that any class loader can call:
> >
> > Instrumentation instrumentation = (Instrumentation)
> > Class.forName("net.bytebuddy.agent.Installer", false,
> > ClassLoader.getSystemClassLoader()).getDeclaredField("
> instrumentation").get(null);
> >
> > Any library on the class path can now also call the above code without
> > requiring any priviledges as the Instrumentation instance is exposed
> > without constraints. Adding a proper method for reading an instance of
> > Instrumentation would prevent this.
>
> Well, that's easily fixed. Make the agent push the Instrumentation
> instance to the class which loaded the agent.
>
> For example, provide the name of the class and name of a public setter
> method as arguments to agentMain (and, if you want, a shared key to
> validate the set). Then get the agent to locate the class, lookup the
> setter method and hand over the instance.
>
> regards,
>
>
> Andrew Dinn
> ---
>
>

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

[Andrew Dinn]

On 30/09/16 14:27, Rafael Winterhalter wrote:
> A Java agent ends up on the class path. The Byte Buddy agent (or any
> similar library) basically adds a single class:
> 
> package net.bytebuddy.agent;
> public class Installer {
>   public static volatile Instrumentation instrumentation;
>   public static void agentMain(String argument, Instrumentation
> instrumentation) {
> Agent.instrumentation = instrumentation
>   }
> }
> 
> Since the class is added using self-attachment, the agentmain method is
> called by the VM and the field value is set. In order to keep the field
> accessible, it needs to be public such that any class loader can call:
> 
> Instrumentation instrumentation = (Instrumentation)
> Class.forName("net.bytebuddy.agent.Installer", false,
> ClassLoader.getSystemClassLoader()).getDeclaredField("instrumentation").get(null);
> 
> Any library on the class path can now also call the above code without
> requiring any priviledges as the Instrumentation instance is exposed
> without constraints. Adding a proper method for reading an instance of
> Instrumentation would prevent this.

Well, that's easily fixed. Make the agent push the Instrumentation
instance to the class which loaded the agent.

For example, provide the name of the class and name of a public setter
method as arguments to agentMain (and, if you want, a shared key to
validate the set). Then get the agent to locate the class, lookup the
setter method and hand over the instance.

regards,


Andrew Dinn
---

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

[Rafael Winterhalter]

A Java agent ends up on the class path. The Byte Buddy agent (or any
similar library) basically adds a single class:

package net.bytebuddy.agent;
public class Installer {
  public static volatile Instrumentation instrumentation;
  public static void agentMain(String argument, Instrumentation
instrumentation) {
Agent.instrumentation = instrumentation
  }
}

Since the class is added using self-attachment, the agentmain method is
called by the VM and the field value is set. In order to keep the field
accessible, it needs to be public such that any class loader can call:

Instrumentation instrumentation = (Instrumentation)
Class.forName("net.bytebuddy.agent.Installer", false,
ClassLoader.getSystemClassLoader()).getDeclaredField("instrumentation").get(null);

Any library on the class path can now also call the above code without
requiring any priviledges as the Instrumentation instance is exposed
without constraints. Adding a proper method for reading an instance of
Instrumentation would prevent this.

2016-09-30 15:14 GMT+02:00 Andrew Dinn :

> On 30/09/16 13:15, Rafael Winterhalter wrote:
> > Such a library exists already:
> > http://mvnrepository.com/search?q=byte-buddy-agent
>
> Ok, so problem solved :-)
>
> > I do however not share your point of view on this: if there is a valid
> > use case for self-attachment - Aleksey, you and I already named several
> > such use cases - why not add convenience, security and performance to
> > the API? Right now, a library can already self-attach when there is no
> > security manager but will always expose the Instrumentation instance
> > accessibly on the system class loader.
>
> I am not really sure what you mean by "will always expose the
> Instrumentation instance accessibly on the system class loader". Are you
> talking about reflective access from classes loaded by the system class
> loader? or do you mean something else?
>
> regards,
>
>
> Andrew Dinn
> ---
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>

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

[Andrew Dinn]

On 30/09/16 13:15, Rafael Winterhalter wrote:
> Such a library exists already:
> http://mvnrepository.com/search?q=byte-buddy-agent

Ok, so problem solved :-)

> I do however not share your point of view on this: if there is a valid
> use case for self-attachment - Aleksey, you and I already named several
> such use cases - why not add convenience, security and performance to
> the API? Right now, a library can already self-attach when there is no
> security manager but will always expose the Instrumentation instance
> accessibly on the system class loader.

I am not really sure what you mean by "will always expose the
Instrumentation instance accessibly on the system class loader". Are you
talking about reflective access from classes loaded by the system class
loader? or do you mean something else?

regards,


Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

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

[Rafael Winterhalter]

Such a library exists already:
http://mvnrepository.com/search?q=byte-buddy-agent

I do however not share your point of view on this: if there is a valid use
case for self-attachment - Aleksey, you and I already named several such
use cases - why not add convenience, security and performance to the API?
Right now, a library can already self-attach when there is no security
manager but will always expose the Instrumentation instance accessibly on
the system class loader.  It is my opinion that adding such an API would
only improve the situation compared to today. In this light, calling
ByteBuddyAgent.install() is a less secure way of calling the proposed
Instrumentation.getInstance().

I understand that this needs to be assesed thoroughly but I still hope this
proposal is not dismissed prematurely.

Best regards, Rafael

Best regards, Rafael

2016-09-30 13:51 GMT+02:00 Andrew Dinn :

> On 30/09/16 10:37, Alan Bateman wrote:
> > On 30/09/2016 09:39, Rafael Winterhalter wrote:
> >
> >> I agree that this should be considered carefully.
> >>
> >> However, without a security manager, it is already possible to get
> >> such an instance for most environments. And starting with Java 9, this
> >> will extend to non JDK-VMs as the former tools.jar is now a module.
> > Assuming you mean the jdk.attach module then it's JDK-specific. It's not
> > linked into the runtime image that is the JRE for example.
> >
> > In any case, this proposal is a significant concern as it is exposing
> > capabilities in the standard API that were intended for tool agents. The
> > implications are huge.
>
> I agree with all Alan's concerns here.
>
> Also,  although many developers have used the attach API to self-hoist
> an agent into a JVM (Byteman also does this when used with JUnit/TestNG)
> and have, perhaps, re-implemented the same wheel to do so I don't think
> fixing that circumstance really merits a JVM change. A couple of small
> library jars could address this problem, shrink-wrapping the necessary
> functionality into a common API.
>
> The first jar could provide a class with a getInstrumentation() method
> which drives the attach API, loading the 2nd agent jar (which it can
> locate from the classpath).
>
> The agent jar can securely hand over the Instrumentation instance to the
> API class provided in the first jar allowing it to be returned to the
> caller.
>
> If these 2 jars were posted for common use (e.g. to Maven Central) then
> the functionality can be made available simply by adding the required
> jars to the classpath and calling the getInstrumentation() method.
>
> regards,
>
>
> Andrew Dinn
> ---
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>

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

[Andrew Dinn]

On 30/09/16 10:37, Alan Bateman wrote:
> On 30/09/2016 09:39, Rafael Winterhalter wrote:
> 
>> I agree that this should be considered carefully.
>>
>> However, without a security manager, it is already possible to get
>> such an instance for most environments. And starting with Java 9, this
>> will extend to non JDK-VMs as the former tools.jar is now a module.
> Assuming you mean the jdk.attach module then it's JDK-specific. It's not
> linked into the runtime image that is the JRE for example.
> 
> In any case, this proposal is a significant concern as it is exposing
> capabilities in the standard API that were intended for tool agents. The
> implications are huge.

I agree with all Alan's concerns here.

Also,  although many developers have used the attach API to self-hoist
an agent into a JVM (Byteman also does this when used with JUnit/TestNG)
and have, perhaps, re-implemented the same wheel to do so I don't think
fixing that circumstance really merits a JVM change. A couple of small
library jars could address this problem, shrink-wrapping the necessary
functionality into a common API.

The first jar could provide a class with a getInstrumentation() method
which drives the attach API, loading the 2nd agent jar (which it can
locate from the classpath).

The agent jar can securely hand over the Instrumentation instance to the
API class provided in the first jar allowing it to be returned to the
caller.

If these 2 jars were posted for common use (e.g. to Maven Central) then
the functionality can be made available simply by adding the required
jars to the classpath and calling the getInstrumentation() method.

regards,


Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

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

[Alan Bateman]

On 30/09/2016 09:39, Rafael Winterhalter wrote:


I agree that this should be considered carefully.

However, without a security manager, it is already possible to get 
such an instance for most environments. And starting with Java 9, this 
will extend to non JDK-VMs as the former tools.jar is now a module.
Assuming you mean the jdk.attach module then it's JDK-specific. It's not 
linked into the runtime image that is the JRE for example.


In any case, this proposal is a significant concern as it is exposing 
capabilities in the standard API that were intended for tool agents. The 
implications are huge.


-Alan

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

[Rafael Winterhalter]

I agree that this should be considered carefully.

However, without a security manager, it is already possible to get such an
instance for most environments. And starting with Java 9, this will extend
to non JDK-VMs as the former tools.jar is now a module. I think by adding
such a method, security would improve as current solutions often use their
privileged access to read such an instance but often leave this instance
exposed in a public field reachable via the system class loader. I think by
including such a method, one could avoid the spreading of custom libraries
(like mine) that do self-attachment and properly secure the access via a
security manager.

Thank you for considering this!
Best regards, Rafael

2016-09-30 10:31 GMT+02:00 Alan Bateman :

> On 29/09/2016 20:37, Rafael Winterhalter wrote:
>
> 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.
>>
>> Right, but it essentially means that anything that use
> Instrumentation.getInstance(...) can dynamically extend the class path,
> add to the boot class loader search, redefine any class or module, ...
>  Assume the common case where there is no security manager and not using
> JDK-specific APIs. So I think this requires a lot of consideration before
> going any further - the original intention with this API is that it was for
> tool agents and this is why a method like the proposal method has been kept
> out of the API.
>
> -Alan
>

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

[Alan Bateman]

On 29/09/2016 20:37, Rafael Winterhalter wrote:

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.


Right, but it essentially means that anything that use 
Instrumentation.getInstance(...) can dynamically extend the class path, 
add to the boot class loader search, redefine any class or module, ...   
Assume the common case where there is no security manager and not using 
JDK-specific APIs. So I think this requires a lot of consideration 
before going any further - the original intention with this API is that 
it was for tool agents and this is why a method like the proposal method 
has been kept out of the API.


-Alan

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

[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
> >
>