RFR: 8276904: Optional.toString() is unnecessarily expensive

2021-11-10 Thread Eamonn McManus
Use string concatenation instead of `String.format`.

-

Commit messages:
 - 8276904: Optional.toString() is unnecessarily expensive

Changes: https://git.openjdk.java.net/jdk/pull/6337/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6337&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8276904
  Stats: 4 lines in 4 files changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6337.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6337/head:pull/6337

PR: https://git.openjdk.java.net/jdk/pull/6337


Integrated: 8231490: Ugly racy writes to ZipUtils.defaultBuf

2021-11-08 Thread Eamonn McManus
On Wed, 3 Nov 2021 21:46:08 GMT, Eamonn McManus  wrote:

> This change applies the minimal fix suggested in
> https://bugs.openjdk.java.net/browse/JDK-8231490.
> The bug text suggests possibilities for reworking, but notes that
> this change is enough to fix the data race.
> Adding a regression test is probaby not feasible but we do observe
> that Java TSAN no longer reports a race after the change.

This pull request has now been integrated.

Changeset: 905e3e88
Author:Eamonn McManus 
URL:   
https://git.openjdk.java.net/jdk/commit/905e3e88137d46f90de7034e9fc324e97af873a6
Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod

8231490: Ugly racy writes to ZipUtils.defaultBuf

Reviewed-by: lancea

-

PR: https://git.openjdk.java.net/jdk/pull/6242


RFR: 8231490: Ugly racy writes to ZipUtils.defaultBuf

2021-11-03 Thread Eamonn McManus
This change applies the minimal fix suggested in
https://bugs.openjdk.java.net/browse/JDK-8231490.
The bug text suggests possibilities for reworking, but notes that
this change is enough to fix the data race.
Adding a regression test is probaby not feasible but we do observe
that Java TSAN no longer reports a race after the change.

-

Commit messages:
 - 8231490: Fix a data race in java.util.zip.Inflater

Changes: https://git.openjdk.java.net/jdk/pull/6242/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6242&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8231490
  Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6242.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6242/head:pull/6242

PR: https://git.openjdk.java.net/jdk/pull/6242


Re: RFR: 8188044: We need Math.unsignedMultiplyHigh [v2]

2021-07-01 Thread Eamonn McManus
On Thu, 1 Jul 2021 18:08:22 GMT, Brian Burkhalter  wrote:

>> Please consider this proposal to add a method 
>> `unsignedMultiplyHigh(long,long)` to each of `java.lang.Math` and 
>> `java.lang.StrictMath`.
>
> Brian Burkhalter has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8188044: Add @see links between multiplyHigh() and unsignedMultiplyHigh().

Oh right. Then obviously this is the right place for the new method. Sorry for 
the noise.

-

PR: https://git.openjdk.java.net/jdk/pull/4644


Re: RFR: 8188044: We need Math.unsignedMultiplyHigh [v2]

2021-07-01 Thread Eamonn McManus
On Thu, 1 Jul 2021 18:08:22 GMT, Brian Burkhalter  wrote:

>> Please consider this proposal to add a method 
>> `unsignedMultiplyHigh(long,long)` to each of `java.lang.Math` and 
>> `java.lang.StrictMath`.
>
> Brian Burkhalter has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8188044: Add @see links between multiplyHigh() and unsignedMultiplyHigh().

`Long` already has some unsigned arithmetic methods: `divideUnsigned`, 
`compareUnsigned`, `toUnsignedString`. `Math` doesn't. Would it make more sense 
to add the new method to `Long`? That would also avoid the quirk of having 
distinct methods in `Math` and `StringMath` when the numbers in involved are 
integers.

-

PR: https://git.openjdk.java.net/jdk/pull/4644


Re: RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

2015-10-01 Thread Eamonn McManus
I think this is a great idea. In addition to the NetBeans precedent
cited in the Issue, I'll note that Guava has
FinalizableReferenceQueue. Like the NetBeans solution, that has proved
tricky to make collectable in a context where its ClassLoader might
become otherwise unreferenced. Having the facility in the JRE obviates
that problem.

As a minor note, the javadoc says "phantomCleanup registers the
Runnable to be run when object is not phantom reachable", which I
don't think is technically accurate. The java.lang.ref definition of
reachability is that an object becomes (for example) phantom reachable
when it is referenced by a phantom reference but no longer has any
stronger level of reachability. So the text here should say "when the
object becomes phantom reachable". Likewise the per-method text "when
the object is unreachable by a PhantomReference" should say "becomes
phantom reachable". And of course the same, mutatis mutandis, for the
soft and weak equivalents.
Éamonn


2015-10-01 7:12 GMT-07:00 Roger Riggs :
> Please review a proposal for public Cleaner API:
>
> A Cleaner is proposed to provide an easy to use alternative to finalization.
> The service would provide easy registration and cancellation of cleanup
> functions for objects. Applications create a cleanup service for their own
> use and the service terminates when it is no longer in use.
>
> Finalization has a long history of issues both in usage and performance.
> PhantomReferences have been proposed as the alternative GC based mechanism
> for cleaning functions but it has been left as an exercise to the developer
> to construct the necessary mechanisms to handle ReferenceQueues, handle
> threading issues and robust termination.
>
> The Cleaner performs cleaning functions when objects are unreachable as
> found by garbage collection using the existing mechanisms of
> PhantomReference, WeakReference, SoftReferences, and ReferenceQueues. It
> manages a thread that dequeues references to unreachable objects and invokes
> the corresponding cleaning function. Registered cleaning functions can be
> cleared if no longer needed, can be invoked explicitly to perform the
> cleanup immediately, or be invoked when the object is not reachable (as
> detected by garbage collection) and handled by a cleanup thread.
>
> The java.lang.ref package is proposed for the Cleaner because it is
> complementary to the reference classes and reference queues and to make it
> easy to find.
>
> It is not a goal to replace all uses of finalization or sun.misc.Cleaner in
> the JDK.
> Investigation will evaluate if and in what cases the Cleaner can replace
> finalization.
> A subsequent task will examine uses of finalization and propose specific
> changes
> on a case by base basis.
>
> Please review and comment:
>
> Javadoc:
>   http://cr.openjdk.java.net/~rriggs/cleaner-doc/
>
> Webrev:
>http://cr.openjdk.java.net/~rriggs/webrev-cleaner-8138696/
>
> Issue:
>https://bugs.openjdk.java.net/browse/JDK-8138696
>
> Thanks, Roger
>


Re: Enum.valueOf(String)

2013-08-20 Thread Eamonn McManus
It might occur to me to look at valueOf(Class, String) if I was
looking for a method to convert a string to an enum constant, but I
don't think it would occur to me to look there if I was looking for a
method to get all the values of an enum. I'm sure plenty of people end
up using MyEnum.class.getEnumConstants() because that *is* linked to
from the class javadoc. Perhaps a {@link} from the class javadoc to
the valueOf(Class, String) method would be sufficient, like this:

* More information about enums, including descriptions of the
* implicitly declared methods synthesized by the compiler, can be
* found in section 8.9 of
* The Java™ Language Specification. See also
* the description in {@link #valueOf valueOf}.

Additionally, Class.getEnumConstants() could mention the simpler
alternative if the class is known at compile time.

Éamonn


2013/8/20 Jonathan Gibbons :
> Eamonn,
>
> See
> http://docs.oracle.com/javase/7/docs/api/java/lang/Enum.html#valueOf(java.lang.Class,%20java.lang.String)
>
>
>> Note that for a particular enum type T, the implicitly declared public
>> static T valueOf(String) method on that enum may be used instead of this
>> method to map from a name to the corresponding enum constant. All the
>> constants of an enum type can be obtained by calling the implicit public
>> static T[] values() method of that type.
>
>
> -- Jon
>
> On 08/20/2013 11:00 AM, Eamonn McManus wrote:
>>
>> As I mentioned earlier in the thread, it's kind of user-hostile for
>> the Enum javadoc to send the user to the JLS instead of just saying,
>> even briefly, what the methods are. Even more so since it doesn't
>> actually link to the relevant section of the JLS or in fact to the JLS
>> at all. This is not a complaint that the information isn't documented,
>> just that it is too hard to find. Imagine how much work I have to do
>> if I can't remember the name of the method to convert from a string to
>> an enum constant in a new enum I'm writing, and it doesn't occur to me
>> to pick some other random enum and look at its javadoc.
>>
>> Éamonn
>>
>>
>> 2013/8/20 Jonathan Gibbons :
>>>
>>> Paul,
>>>
>>> Enums are well covered in JLS 7, section 8.9. In particular, see 8.9.2,
>>> Enum
>>> Body Declarations, beginning at the line
>>>
>>> "In addition, if E is the name of an enum type, then that type has the
>>> following implicitly declared static methods:"
>>>
>>> -- Jon
>>>
>>>
>>> On 08/20/2013 06:27 AM, Paul Benedict wrote:
>>>>
>>>> Jon, it's not a problem with the method docs, per se. The issue is about
>>>> how the generation isn't documented. My questioning started because I
>>>> was
>>>> using several enums without javadoc available, but I did have the source
>>>> available, and couldn't figure out how the method came to be. Since I've
>>>> asked, everyone knew (but me!) it was a generated method, but I couldn't
>>>> divine that knowledge.
>>>>
>>>> My recommendation is to add an @implNote on Enum.valueOf(Class, String)
>>>> so
>>>> that people know each subclass will get a generated method that behaves
>>>> similarly. What do you think?
>>>>
>>>>
>>>> On Mon, Aug 19, 2013 at 9:23 AM, Paul Benedict >>> <mailto:pbened...@apache.org>> wrote:
>>>>
>>>>  I have been working with classes that don't have javadoc
>>>>  attachments. The problem was I couldn't find the method in the
>>>>  source nor was the method part of the Enum class. So where did it
>>>>  materialize from? Now I know the answer: the compiler generates it.
>>>>
>>>>  I really think this knowledge should be added to the Enum javadoc
>>>>  class. I had to go on quite a goose hunt to find this fact.
>>>>
>>>>  Paul
>>>>
>>>>
>>>>  On Mon, Aug 19, 2013 at 3:32 AM, Alan Bateman
>>>>  mailto:alan.bate...@oracle.com>> wrote:
>>>>
>>>>  On 18/08/2013 05:07, Paul Benedict wrote:
>>>>
>>>>  I think the generated method needs to be listed in the
>>>>  class javadoc at
>>>>  least. I presume it throws an exception too (like the
>>>>  other valueOf) if the
>>>>  String can't be resolved to a constant, but no user is
>>>>  going to discover
>>>>  this fact through the documentation.
>>>>
>>>>  Have you checked the generated avadoc for your enum? The
>>>>  valueOf(String) should be there and specified to throw IAE or
>>>> NPE.
>>>>
>>>>  -Alan
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>  -- Cheers,
>>>>  Paul
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Cheers,
>>>> Paul
>>>
>>>
>


Re: Enum.valueOf(String)

2013-08-20 Thread Eamonn McManus
As I mentioned earlier in the thread, it's kind of user-hostile for
the Enum javadoc to send the user to the JLS instead of just saying,
even briefly, what the methods are. Even more so since it doesn't
actually link to the relevant section of the JLS or in fact to the JLS
at all. This is not a complaint that the information isn't documented,
just that it is too hard to find. Imagine how much work I have to do
if I can't remember the name of the method to convert from a string to
an enum constant in a new enum I'm writing, and it doesn't occur to me
to pick some other random enum and look at its javadoc.

Éamonn


2013/8/20 Jonathan Gibbons :
> Paul,
>
> Enums are well covered in JLS 7, section 8.9. In particular, see 8.9.2, Enum
> Body Declarations, beginning at the line
>
> "In addition, if E is the name of an enum type, then that type has the
> following implicitly declared static methods:"
>
> -- Jon
>
>
> On 08/20/2013 06:27 AM, Paul Benedict wrote:
>>
>> Jon, it's not a problem with the method docs, per se. The issue is about
>> how the generation isn't documented. My questioning started because I was
>> using several enums without javadoc available, but I did have the source
>> available, and couldn't figure out how the method came to be. Since I've
>> asked, everyone knew (but me!) it was a generated method, but I couldn't
>> divine that knowledge.
>>
>> My recommendation is to add an @implNote on Enum.valueOf(Class, String) so
>> that people know each subclass will get a generated method that behaves
>> similarly. What do you think?
>>
>>
>> On Mon, Aug 19, 2013 at 9:23 AM, Paul Benedict > > wrote:
>>
>> I have been working with classes that don't have javadoc
>> attachments. The problem was I couldn't find the method in the
>> source nor was the method part of the Enum class. So where did it
>> materialize from? Now I know the answer: the compiler generates it.
>>
>> I really think this knowledge should be added to the Enum javadoc
>> class. I had to go on quite a goose hunt to find this fact.
>>
>> Paul
>>
>>
>> On Mon, Aug 19, 2013 at 3:32 AM, Alan Bateman
>> mailto:alan.bate...@oracle.com>> wrote:
>>
>> On 18/08/2013 05:07, Paul Benedict wrote:
>>
>> I think the generated method needs to be listed in the
>> class javadoc at
>> least. I presume it throws an exception too (like the
>> other valueOf) if the
>> String can't be resolved to a constant, but no user is
>> going to discover
>> this fact through the documentation.
>>
>> Have you checked the generated avadoc for your enum? The
>> valueOf(String) should be there and specified to throw IAE or NPE.
>>
>> -Alan
>>
>>
>>
>>
>>
>> -- Cheers,
>> Paul
>>
>>
>>
>>
>> --
>> Cheers,
>> Paul
>
>


Re: Enum.valueOf(String)

2013-08-16 Thread Eamonn McManus
The valueOf method in Enum subclasses is static, so Enum itself can't
usefully declare it. But the description of the Enum class could say
in text what the generated methods are, rather than referring the
reader to the JLS without even a link. If you forget what the methods
are you can always look in any enum, such as Thread.State
,
but you shouldn't have to.

Éamonn


2013/8/16 Nick Williams :
> That method doesn't exist in the actual java.lang.Enum base class. It gets 
> generated automatically when the enum is compiled and is part of the compiled 
> enum class, not part of the Enum base class.
>
> With that said, I don't disagree that it could use some documentation. I've 
> often wondered why java.lang.Enum didn't have the following method definition:
>
> public abstract E valueOf(String name);
>
> That is essentially the method that all compiled enums implement, it's just 
> not actually specified as an abstract method on the base class. If it were, 
> then there could be JavaDoc for it.
>
> Nick
>
> On Aug 16, 2013, at 11:30 PM, Paul Benedict wrote:
>
>> I noticed this method is not listed in the Javadocs for 5/6/7/8 but it's
>> part of every enum. Is this an oversight or is there a good reason why it's
>> not documented?
>>
>> --
>> Cheers,
>> Paul
>


Re: JDK 8 code review request for JDK-8020095: Fix doclint warnings in java.util.regex

2013-07-09 Thread Eamonn McManus
While you are at it, ... is a lot simpler than


Éamonn


2013/7/8 Joe Darcy 
>
> PS Updated webrev so that the ... structure don't have an 
> empty "..." portion.
>
> http://cr.openjdk.java.net/~darcy/8020095.1/
>
> -Joe
>
>
> On 07/08/2013 07:10 PM, Joe Darcy wrote:
>>
>> Hello,
>>
>> Please review my changes to resolve (almost all of):
>>
>> JDK-8020095 Fix doclint warnings in java.util.regex
>> http://cr.openjdk.java.net/~darcy/8020095.0/
>>
>> Full text of the patch also included below.
>>
>> For some reason I have not been able to determine, even with the patch, one 
>> error remains:
>>
>>> src/share/classes/java/util/regex/Pattern.java:222: error: text not allowed 
>>> in  element
>>>  * Classes for Unicode 
>>> scripts, blocks, categories and binary properties
>>> ^
>>> 1 error
>>
>>
>> This line has the same structure as other that appear unproblematic. In any 
>> case, since over 200 doclint issues are resolved with the patch, I'd like to 
>> go forward with the patch and have this lone remaining error investigated 
>> later on.
>>
>> Thanks,
>>
>> -Joe
>>
>


Re: CloneNotSupportedException should extends RuntimeException not Exception

2012-10-14 Thread Eamonn McManus
Brian Goetz  writes:
> try { clone() }
> catch (RuntimeException e) { ... }
> catch (CloneNotSupportedException e1) {  }

I guess that means the change is not binary compatible either, since before
the change the second catch block would run but after it the first one
would.

Éamonn


2012/10/14 Brian Goetz 

> I believe this change is not source compatible.  If a user says:
>
> try { clone() }
> catch (RuntimeException e) { ... }
> catch (CloneNotSupportedException e1) {  }
>
> this compiles today but would fail to compile under this change.
>
> On Oct 14, 2012, at 7:06 PM, Mike Duigou wrote:
>
> > Seems reasonable to me at first glance. I am still reviewing this but
> wanted to add two clarify notes:
> >
> > - This change means that CloneNotSupportedException is no longer a
> checked exception. This change is generally harmless.
> >
> > - Cases where CloneNotSupportedException is being caught were probably
> added because it was a checked exception. They can be safely left in.
> >
> > Mike
> >
> >
> > On Oct 14 2012, at 09:19 , Remi Forax wrote:
> >
> >> Hi everybody,
> >> CloneNotSupportedException is thrown when a developer calls
> Object.clone() and forget to mark the current object as Cloneable. Because
> it's clearly a developer error, CloneNotSupportedException should be a
> subtype of RuntimeException and not of Exception.
> >>
> >> I believe this change is backward compatible because RuntimeException
> is a subclass of Exception,
> >> so I propose to first change CloneNotSupportedException to extends
> RuntimeException.
> >>
> >> diff -r ff641c5b329b
> src/share/classes/java/lang/CloneNotSupportedException.java
> >> --- a/src/share/classes/java/lang/CloneNotSupportedException.java Sat
> Oct 13 10:15:57 2012 +0100
> >> +++ b/src/share/classes/java/lang/CloneNotSupportedException.java Sun
> Oct 14 18:16:35 2012 +0200
> >> @@ -42,7 +42,7 @@
> >> */
> >>
> >> public
> >> -class CloneNotSupportedException extends Exception {
> >> +class CloneNotSupportedException extends RuntimeException {
> >>private static final long serialVersionUID = 5195511250079656443L;
> >>
> >>/**
> >>
> >>
> >> And then to clean up the whole JDK (in several patches) to remove all
> the unnecessary
> >> try/catch like the one in by example ArrayList.clone()
> >>
> >>   public Object clone() {
> >>   try {
> >>   ArrayList v = (ArrayList) super.clone();
> >>   v.elementData = Arrays.copyOf(elementData, size);
> >>   v.modCount = 0;
> >>   return v;
> >>   } catch (CloneNotSupportedException e) {
> >>   // this shouldn't happen, since we are Cloneable
> >>   throw new InternalError(e);
> >>   }
> >>   }
> >>
> >> will become
> >>
> >>   public Object clone() {
> >>   ArrayList v = (ArrayList) super.clone();
> >>   v.elementData = Arrays.copyOf(elementData, size);
> >>   v.modCount = 0;
> >>   return v;
> >>   }
> >>
> >>
> >> cheers,
> >> Rémi
> >>
> >
>
>


Re: Reviewer needed: 6282196 There should be Math.mod(number, modulo) methods

2012-10-10 Thread Eamonn McManus
One edge case: the spec for floorDiv implies that
floorDiv(Integer.MIN_VALUE, -1) should be Integer.MAX_VALUE but I
believe the code produces Integer.MIN_VALUE. EIther the spec or the
code should be fixed.

Éamonn


2012/10/10 Roger Riggs :
> A reviewer is needed for:
>
> 6282196 There should be Math.mod(number, modulo) methods
>
> The webrev is: http://cr.openjdk.java.net/~rriggs/6282196.4/
>
> Thanks, Roger


Re: HashMap bug for large sizes

2012-06-01 Thread Eamonn McManus
> But it is not just serializing a HashMap that does not work. HashMap.size()
> and HashMap.clear() isn't working as well.

I don't see what's wrong with HashMap.clear(), but HashMap.size() is
clearly buggy and should be fixed. There's also a performance problem
in that accesses start becoming linear once there are more than 1 <<
30 entries, but fixing that is substantially harder than just fixing
size(), and ConcurrentHashMap already provides a better alternative
for such huge maps.

Éamonn


On 1 June 2012 12:28, Kasper Nielsen  wrote:
> On 01-06-2012 21:12, Eamonn McManus wrote:
>>
>> It seems to me that since the serialization of HashMaps with more than
>> Integer.MAX_VALUE entries produces an output that cannot be
>> deserialized, nobody can be using it, and we are free to change it.
>> For example we could say that if the read size is -1 then the next
>> item in the stream is a long that is the true size, and arrange for
>> that to be true in writeObject when there are more than
>> Integer.MAX_VALUE entries.
>
> Yeah,
> I thought of something along the lines of:
>
> long mapSize;
> s.writeInt(mapSize> Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) mapSize );
> for (int i=0;i
> if (mapSize>=Integer.MAX_VALUE) {
>  s.writeLong(size);//write the real size
>  for (long i=Integer.MAX_VALUE;i }
>
> }
>
>> Whether there really are people who have HashMaps with billions of
>> entries that they want to serialize with Java serialization is another
>> question.
>
>
> But it is not just serializing a HashMap that does not work. HashMap.size()
> and HashMap.clear() isn't working as well.
>
> - Kasper


Re: HashMap bug for large sizes

2012-06-01 Thread Eamonn McManus
It seems to me that since the serialization of HashMaps with more than
Integer.MAX_VALUE entries produces an output that cannot be
deserialized, nobody can be using it, and we are free to change it.
For example we could say that if the read size is -1 then the next
item in the stream is a long that is the true size, and arrange for
that to be true in writeObject when there are more than
Integer.MAX_VALUE entries.

Whether there really are people who have HashMaps with billions of
entries that they want to serialize with Java serialization is another
question.

Éamonn


On 1 June 2012 06:36, Doug Lea  wrote:
>
> On 06/01/12 05:29, Kasper Nielsen wrote:
>>
>> Hi,
>>
>> I don't know if this has been discussed before. But I was looking at
>> the HashMap implementation today and noticed that there are some
>> issues with very large sized hashmaps with more then Integer.MAX_VALUE
>> elements.
>
>
> I think this arose on this list (or possibly some side-exchanges)
> three years ago when discussing possible HashMap improvements. It
> seems impossible to fix this without breaking serialization
> compatibility, so people seemed resigned to let the limitations
> remain until there was some change forcing incompatibility anyway.
> At the least though, an implementation note could be added to
> the javadocs. I think several other java.util classes also have
> this problem, but none of the java.util.concurrent ones do.
> So as a workaround, people can use ConcurrentHashMap even if
> they aren't using it concurrently.
>
> -Doug
>
>
>>
>> 1.
>> The Map contract says that "If the map contains more than
>> Integer.MAX_VALUE elements, returns Integer.MAX_VALUE." The current
>> implementation will just wrap around and return negative
>> numbers when you add elements (size++).
>>
>> 2.
>> If the size of a HashMap has wrapped around and returns negative size
>> you cannot deserialize it. Because of this loop in readObject
>> for (int i=0; i>    K key = (K) s.readObject();
>>    V value = (V) s.readObject();
>>    putForCreate(key, value);
>> }
>>
>> If someone wants to play around with the size limits of HashMap I
>> suggest taking the source code of HashMap and change the type of the
>> size field from an int to a short, in which case you can test this
>> with less the xx GB of heap.
>>
>> There are probably other map implementations in the JDK with the same issues.
>>
>> Cheers
>>   Kasper
>>
>


Re: Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec

2012-03-13 Thread Eamonn McManus
>>> Why don't we have
>>> public  T[] toArray(T[] a) ?
>>> This would prevent from the cast
>>> r[i] = (T)it.next();
>>
>> It's too late to change the method signature now.
>
> Sorry about my english, I meant why don't we have had ...
> In other words, has there been a reason that it was not like that from the
> beginning?

Perhaps because it is not legal Java?

Éamonn


On 13 March 2012 12:16, Ulf Zibis  wrote:
>
> Am 10.03.2012 13:52, schrieb David Holmes:
>>
>> On 10/03/2012 12:02 PM, Ulf Zibis wrote:
>>>
>>> Why don't we have
>>> public  T[] toArray(T[] a) ?
>>> This would prevent from the cast
>>> r[i] = (T)it.next();
>>
>>
>> It's too late to change the method signature now.
>
> Sorry about my english, I meant why don't we have had ...
> In other words, has there been a reason that it was not like that from the
> beginning?
>
>
>> Wouldn't following statement potentially throw a ClassCastException ?
>>>
>>> r[i] = (T)it.next();
>>
>>
>> Apparently not. I passed in a String[] when it should be Object[] and got
>> ArrayStoreException. Checking the bytecode I don't see a checkcast.
>
> Thanks, checking that out.
>
> -Ulf
>


Re: RFR: 6988220: java.lang.ObjectName use of String.intern() causes major performance issues at scale

2012-02-24 Thread Eamonn McManus
Hi Dan,

I got a bounce from serviceability-dev because I wasn't subscribed to
it, but the message went out to core-libs-dev because I was subscribed
to that. That probably explains what you saw.

Regards,
Éamonn


On 24 February 2012 09:33, Daniel D. Daugherty
 wrote:
>
> Just FYI: I haven't seen Éamonn's posting come in. Just replies to
> his posting. This may mean that other comments are stuck in the
> ether somewhere...
>
> I suspect that the OpenJDK list server is again having issues...
>
> Dan
>
>
> On 2/24/12 8:21 AM, Olivier Lagneau wrote:
>
> I think I have not been clear enough here.
>
> I Agree with Eammon's argument, and anyway ok with this change.
>
> Olivier.
>
>
> Olivier Lagneau said  on date 2/24/2012 12:38 PM:
>
> Hi Éamonn,
>
> Eamonn McManus said  on date 2/23/2012 8:44 PM:
>
> I am not sure it is worth the complexity of extra checks. Do you have data 
> showing that ObjectName.equals usually returns false?In a successful HashMap 
> lookup, for example, it will usually return true since the equals method is 
> used to guard against collisions, and collisions are rare by design. 
> Meanwhile, String.equals is intrinsic in HotSpot so we may assume that it is 
> highly optimized, and you are giving up that optimization if you use other 
> comparisons.
>
> Don't have this kind of data indeed. I don't know of any benchmark/data about 
> usage of ObjectName.equals()
> in most applications. That would be needed to evaluate the exact impact of 
> the change.
> And I agree with the argument that usual semantics of an equals call is to 
> check for equality,
> not the difference.
>
> My argument is mainly that we are moving from comparing identity to equality.
> Thus there will be an impact on the throughput of equals, possibly impacting
> some applications.
>
> Olivier.
>
> Éamonn
>
>
> On 23 February 2012 10:52, Olivier Lagneau  <mailto:olivier.lagn...@oracle.com>> wrote:
>
>     Hi Frederic,
>
>     Performance and typo comments.
>
>     Regarding performance of ObjectName.equals method, which is certainely
>     a frequent call on ObjectNames, I think that using inner fields
>     (Property array for canonical name and domain length) would be
>     more efficient
>     than using String.equals() on these potentially very long strings.
>
>     Two differents objectNames may often have the same length with
>     different key/properties values, and may often be different only
>     on the last property of the canonical name.
>
>     The Property array field ca_array (comparing length and property
>     contents)
>     and domain length are good candidates to filter out more efficiently
>     different objectNames, knowing that String.equals will compare every
>     single char of the two char arrays.
>
>     So for performance purpose, I suggest to filter out different
>     objectNames
>     by doing inner comparisons in the following order : length of the two
>     canonical names, then domain_length, then ca_array size, then its
>     content,
>     and lastly if all of this fails to filter out, then use String.equals.
>
>  _canonicalName = (new String(canonical_chars, 0, prop_index));
>
>     Typo : useless parentheses.
>
>     Thanks,
>     Olivier.
>
>     -- Olivier Lagneau, olivier.lagn...@oracle.com
>     <mailto:olivier.lagn...@oracle.com>
>     Oracle, Grenoble Engineering Center - France
>     Phone : +33 4 76 18 80 09  Fax
>     : +33 4 76 18 80 23 
>
>
>
>
>     Frederic Parain said  on date 2/23/2012 6:01 PM:
>
>     No particular reason. But after thinking more about it,
>     equals() should be a better choice, clearer code, and
>     the length check in equals() implementation is likely
>     to help performance of ObjectName's comparisons as
>     ObjectNames are often long with a common section at the
>     beginning.
>
>     I've updated the webrev:
>     http://cr.openjdk.java.net/~fparain/6988220/webrev.01/
>     <http://cr.openjdk.java.net/%7Efparain/6988220/webrev.01/>
>
>     Thanks,
>
>     Fred
>
>     On 2/23/12 4:58 PM, Vitaly Davidovich wrote:
>
>     Hi Frederic,
>
>     Just curious - why are you checking string equality via
>     compareTo()
>     instead of equals()?
>
>     Thanks
>
>     Sent from my phone
>
>     On Feb 23, 2012 10:37 AM, "Frederic Parain"
>          <mailto:frederic.par...@oracle.com>
>     <mailto:frederic.par...@oracle.com
>     <ma

Re: RFR: 6988220: java.lang.ObjectName use of String.intern() causes major performance issues at scale

2012-02-23 Thread Eamonn McManus
I am not sure it is worth the complexity of extra checks. Do you have data
showing that ObjectName.equals usually returns false? In a successful
HashMap lookup, for example, it will usually return true since the equals
method is used to guard against collisions, and collisions are rare by
design. Meanwhile, String.equals is intrinsic in HotSpot so we may assume
that it is highly optimized, and you are giving up that optimization if you
use other comparisons.

Éamonn


On 23 February 2012 10:52, Olivier Lagneau wrote:

> Hi Frederic,
>
> Performance and typo comments.
>
> Regarding performance of ObjectName.equals method, which is certainely
> a frequent call on ObjectNames, I think that using inner fields
> (Property array for canonical name and domain length) would be more
> efficient
> than using String.equals() on these potentially very long strings.
>
> Two differents objectNames may often have the same length with
> different key/properties values, and may often be different only
> on the last property of the canonical name.
>
> The Property array field ca_array (comparing length and property contents)
> and domain length are good candidates to filter out more efficiently
> different objectNames, knowing that String.equals will compare every
> single char of the two char arrays.
>
> So for performance purpose, I suggest to filter out different objectNames
> by doing inner comparisons in the following order : length of the two
> canonical names, then domain_length, then ca_array size, then its content,
> and lastly if all of this fails to filter out, then use String.equals.
>
>   _canonicalName = (new String(canonical_chars, 0, prop_index));
>>
> Typo : useless parentheses.
>
> Thanks,
> Olivier.
>
> -- Olivier Lagneau, olivier.lagn...@oracle.com
> Oracle, Grenoble Engineering Center - France
> Phone : +33 4 76 18 80 09 Fax : +33 4 76 18 80 23
>
>
>
>
> Frederic Parain said  on date 2/23/2012 6:01 PM:
>
>  No particular reason. But after thinking more about it,
>> equals() should be a better choice, clearer code, and
>> the length check in equals() implementation is likely
>> to help performance of ObjectName's comparisons as
>> ObjectNames are often long with a common section at the
>> beginning.
>>
>> I've updated the webrev:
>> http://cr.openjdk.java.net/~**fparain/6988220/webrev.01/
>>
>> Thanks,
>>
>> Fred
>>
>> On 2/23/12 4:58 PM, Vitaly Davidovich wrote:
>>
>>> Hi Frederic,
>>>
>>> Just curious - why are you checking string equality via compareTo()
>>> instead of equals()?
>>>
>>> Thanks
>>>
>>> Sent from my phone
>>>
>>> On Feb 23, 2012 10:37 AM, "Frederic Parain" >> >
>>> wrote:
>>>
>>>This a simple fix to solve CR 6988220:
>>>
>>> http://bugs.sun.com/__**bugdatabase/view_bug.do?bug___**id=6988220
>>> 
>>> >
>>>
>>>The use of String.intern() in the ObjectName class prevents
>>>the class the scale well when more than 20K ObjectNames are
>>>managed. The fix simply removes the use of String.intern(),
>>>and uses regular String instead. The Object.equals() method
>>>is modified too to make a regular String comparison. The
>>>complexity of this method now depends on the length of
>>>the ObjectName's canonical name, and is not impacted any
>>>more by the number of ObjectName instances being handled.
>>>
>>>The Webrev:
>>>
>>> http://cr.openjdk.java.net/~__**fparain/6988220/webrev.00/
>>> 
>>> >
>>>
>>>I've tested this fix with the jdk_lang and jdk_management
>>>test suites.
>>>
>>>Thanks,
>>>
>>>Fred
>>>
>>>--
>>>Frederic Parain - Oracle
>>>Grenoble Engineering Center - France
>>>Phone: +33 4 76 18 81 17 
>>>Email: frederic.par...@oracle.com >> oracle.com >
>>>
>>>
>>
>


Re: Review: JDK 8 CR for Support Integer overflow updated

2012-02-16 Thread Eamonn McManus
Reviewed-by: emcmanus

Éamonn



On 16 February 2012 08:23, Xueming Shen  wrote:
> I can do the commit.
>
>
> On 2/16/2012 8:09 AM, Roger Riggs wrote:
>>
>> I don't anticipate making any more changes though a few of the
>> comments deserve followup as a separate CR.
>>
>> Is there an OpenJDK committer who would commit?
>>
>> Thanks, Roger
>>
>>> Updated the webrev for CR6708398:
>>>         http://cr.openjdk.java.net/~rriggs/6708398.2
>>>
>>
>


Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-13 Thread Eamonn McManus
Hacker's Delight gives a formula equivalent to this one on page 27:

((x ^ r) & (y ^ r)) < 0

Having said that, I think Florian's assertion that such a rewrite will be
faster needs proof. In the original form...

(x ^ r) < 0 && (x ^ y) >= 0

...the first condition will be false half the time for random inputs and
(hand-waving) probably a lot more than half the time for typical inputs. To
the extent that the difference is measurable at all, I would not like to
have to bet on which alternative is faster, and the original form is
definitely easier to read as "both arguments have the same sign and the
result has the opposite sign."

Éamonn


On 3 February 2012 09:07, Joe Darcy  wrote:

> On 02/03/2012 06:44 AM, Florian Weimer wrote:
>
>> * Roger Riggs:
>>
>>  The boolean expression can be refactored. (Only bit-31 is significant).
>>> But it becomes pretty inscrutable.
>>>
>>> (x ^ r)<   0&&   (x ^ y)>= 0  becomes
>>>
>>> (x ^ r)<   0&&   (~(x ^ y))<   0  becomes
>>>
>>> ((x ^ r)&   ~(x ^ y))<   0
>>>
>> That's what I got in my second attempt, too. It doesn't result in a
>> longer dependency chain because the left side of the&  is two operations
>>
>> deep, too.  Therefore, this version should be faster (at least after
>> compilation), independent of the input arguments.
>>
>>
> These sorts of concerns and lovingly (and laboriously!) detailed to Hank
> Warren's bit-twiddling tome "Hacker's Delight" (http://www.hackersdelight.
> **org/ ).
>
> Roger's initial code used one of the recommend idioms from that source.
>  We should stick with those idioms unless there is a good reason not to.
>
> -Joe
>


Re: Review: JDK 8 CR for Support Integer overflow

2012-02-06 Thread Eamonn McManus
I'm with Roger on this.

> Sorry, but I can't agree with this. Developers get negation of numbers
> wrong all the time by ignoring the special case. Its a big source of
> hidden bugs. Increment/decrement are replaceable by add/subtract (with
> less readability), but negate is not.

First of all, negate(x) certainly is replaceable by subtractExact(0,
x). Secondly, the problem is more that people don't realize that -x
might not be exact. If you know there's a problem then it isn't any
easier for you to know that a solution is to be found in Math than to
know that the problem is with MIN_VALUE.

Éamonn



On 6 February 2012 14:35, Stephen Colebourne  wrote:
> On 6 February 2012 21:16, Roger Riggs  wrote:
>>  * Removed the negateExact methods since they don't pull their weight
>>   in the API,
>>   simple tests for MIN_VALUE and MAX_VALUE can be done by the
>>   developer more efficiently.
>
> Sorry, but I can't agree with this. Developers get negation of numbers
> wrong all the time by ignoring the special case. Its a big source of
> hidden bugs. Increment/decrement are replaceable by add/subtract (with
> less readability), but negate is not.
>
> The whole purpose of methods like this is not to say "oh they're easy
> for the caller to write", but to write it once accurately,
> well-tested, and with clear intent for code readers. Look at the
> methods on Objects to see that complexity of the implementation is not
> the deciding factor.
>
> Many, many developers will thank you for having negate, increment and 
> decrement.
>
> Stephen


Re: Review: JDK 8 CR for Support Integer overflow

2012-02-06 Thread Eamonn McManus
Looks good to me (emcmanus). One really trivial thing is that
Math.java:830 is indented one space too far. I thought that the common
(int)value subexpression could be put in a variable in toIntExact but
it turns out that javac generates more code in that case.

Éamonn



On 6 February 2012 13:16, Roger Riggs  wrote:
> Thanks for the review and comments:
>
> The comments and suggestions are included in the updated webrev:
>   http://cr.openjdk.java.net/~rriggs/6708398.1
>
>  * Corrected error in multipleExact(long,long) with the special case
>   Long.MIN_VALUE * -1.
>  * Verified that retaining the optimization for small (2^31) arguments
>   is worthwhile,
>   not doing the divide saves about 1/2 on the execution time.
>  * Removed the negateExact methods since they don't pull their weight
>   in the API,
>   simple tests for MIN_VALUE and MAX_VALUE can be done by the
>   developer more efficiently.
>  * Simplified the arguments to the ArithmeticExceptions to be simple
>   strings since
>   debugging this kind of exception requires the source code.
>  * Expanded the comments in the implementation include descriptions and
>   references to the Hackers Delight where they are used.
>  * Updated the tests to include missing test cases
>
> More comments, please
>
> Thanks, Roger
>


Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-04 Thread Eamonn McManus
Concerning the long multiplyExactly, I have a number of comments.

public static long multiplyExact(long x, long y) {
long r = x * y;
long ax = Math.abs(x);
long ay = Math.abs(y);
if (((ax | ay) >>> 31 == 0) || (x == 1) || (y == 1)) {
return r;
}
if (((y != 0) && r / y != x) || (r == Long.MIN_VALUE )) {
throw new ArithmeticException("Multiplication overflows a
long: " + x + " * " + y);
}
return r;
}

I believe that the (ax | ay) condition is an optimization, but I
wonder if it is worthwhile. Presumably the intent is to avoid the
division if possible, but is division really more expensive than all
these extra operations (abs, abs, or, shift, compare) that we are
doing?

In addition to returning when x == 1 or y == 1, we could return when y
== 0, since we're going to be making that check anyway to avoid
divide-by-zero.

The final check (r == Long.MIN_VALUE) is incorrect. I presume the
intent is to detect overflow when we multiply Long.MIN_VALUE by -1
(and obtain Long.MIN_VALUE), but Long.MIN_VALUE can be the result of a
non-overflowing multiplication, for example ((Long.MIN_VALUE / 2) *
2). The division check will catch the case of x=-1,y=Long.MIN_VALUE,
so we could just check for the other case x=Long.MIN_VALUE,y=-1
explicitly.

Éamonn



On 4 February 2012 10:51, Roger Riggs  wrote:
> The methods for increment, decrement, and also negation test for
> exactly one value at the extremities of the value range.
>
> The verbosity of method calls in the source code and the
> overhead in the execution make these a poor choice for developers.
> If the code must really operate correctly at the extremities then the
> developer needs to carefully implement and check where appropriate.
>
> The checking for overflow in addition, subtraction, and multiplication
> are subtle or complex enough to warrant support in the runtime.
>
> Roger
>
>
>
>
> On 02/03/2012 01:00 PM, Stephen Colebourne wrote:
>>
>> On 3 February 2012 17:52, Eamonn McManus  wrote:
>>>
>>> I agree with Stephen Colebourne that brief implementation comments would
>>> be
>>> useful. But I disagree with his proposed further methods in Math
>>> (increment, decrement, int+long variants), which I don't think would pull
>>> their weight.
>>
>> FWIW, JSR-310 currently has 18 non-test usages of increment/decrement,
>> vs 42 uses of add. Less, but certainly used.
>>
>> The real value in increment/decrement is that it becomes a simple
>> mapping from operator to method
>> a++ = increment(a)
>> a-- = decrement(a)
>> a + b = add(a, b)
>> This makes it easier to see what the intent would have been were the
>> real operators safe.
>>
>> Stephen
>
>


Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Eamonn McManus
On 3 February 2012 10:22, Vitaly Davidovich  wrote:
> x == Integer.MIN_VALUE should be faster than x == -x as it's a cmp against a 
>constant whereas the latter requires negating x (that's a dependency too), 
>tying up a register to store the negation, and then doing the cmp.

The value -x is needed anyway since it's the return value. But a more
important reason why my idea is a bad one is that the condition is
true for x == 0! Writing x == -x && x != 0 is definitely not an
improvement.

Éamonn


On 3 February 2012 10:22, Vitaly Davidovich  wrote:
>
> x == Integer.MIN_VALUE should be faster than x == -x as it's a cmp against a 
> constant whereas the latter requires negating x (that's a dependency too), 
> tying up a register to store the negation, and then doing the cmp.
>
> Sent from my phone
>
> On Feb 3, 2012 12:53 PM, "Eamonn McManus"  wrote:
>>
>> My initial remarks:
>>
>> In negateExact, the condition x == -x should be faster to evaluate than x
>> == Integer.MIN_VALUE and reflects the intent just as well.
>>
>> In addExact and subtractExact, I would be inclined to implement the int
>> versions using long arithmetic, like this:
>>
>> long lr = x + y;
>> int r = (int) lr;
>> if (r == lr) {
>>  return r;
>> } else {
>>  throw...
>> }
>>
>> I would use this technique of cast-and-compare in the int multiplyExact
>> instead of comparing against MIN_VALUE and MAX_VALUE, and especially in
>> toIntExact(long).
>>
>> I agree with Stephen Colebourne that brief implementation comments would be
>> useful. But I disagree with his proposed further methods in Math
>> (increment, decrement, int+long variants), which I don't think would pull
>> their weight.
>>
>> Éamonn
>>
>>
>> On 2 February 2012 12:15, Roger Riggs  wrote:
>>
>> > There is a need for arithmetic operations that throw exceptions
>> > when the results overflow the representation of int or long.
>> >
>> > The CR is 6708398: Support integer overflow <http://bugs.sun.com/**
>> > bugdatabase/view_bug.do?bug_**id=6708398<http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6708398>
>> > >
>> >
>> > Please review this webrev <http://cr.openjdk.java.net/%**
>> > 7Erriggs/CR6708398/webrev/<http://cr.openjdk.java.net/%7Erriggs/CR6708398/webrev/>>
>>
>> > to add static methods in java.lang.Math
>> > to support addExact(), subtractExact(), negateExact(), multiplyExact(),
>> > and toIntExact() for int and long primitive types.
>> >
>> > Thanks, Roger Riggs
>> >


Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Eamonn McManus
My initial remarks:

In negateExact, the condition x == -x should be faster to evaluate than x
== Integer.MIN_VALUE and reflects the intent just as well.

In addExact and subtractExact, I would be inclined to implement the int
versions using long arithmetic, like this:

long lr = x + y;
int r = (int) lr;
if (r == lr) {
  return r;
} else {
  throw...
}

I would use this technique of cast-and-compare in the int multiplyExact
instead of comparing against MIN_VALUE and MAX_VALUE, and especially in
toIntExact(long).

I agree with Stephen Colebourne that brief implementation comments would be
useful. But I disagree with his proposed further methods in Math
(increment, decrement, int+long variants), which I don't think would pull
their weight.

Éamonn


On 2 February 2012 12:15, Roger Riggs  wrote:

> There is a need for arithmetic operations that throw exceptions
> when the results overflow the representation of int or long.
>
> The CR is 6708398: Support integer overflow  bugdatabase/view_bug.do?bug_**id=6708398
> >
>
> Please review this webrev  7Erriggs/CR6708398/webrev/>
> to add static methods in java.lang.Math
> to support addExact(), subtractExact(), negateExact(), multiplyExact(),
> and toIntExact() for int and long primitive types.
>
> Thanks, Roger Riggs
>


Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Eamonn McManus
On 3 February 2012 00:11, Florian Weimer  wrote:
> Would it make sense to replace (x ^ r) < 0 && (x ^ y) >= 0
> with (x ^ y ^ r) < 0?

That would not be correct. For example, it would signal overflow for -1 + 1.

Éamonn


On 3 February 2012 00:11, Florian Weimer  wrote:

> * Roger Riggs:
>
> > to support addExact(), subtractExact(), negateExact(), multiplyExact(),
> > and toIntExact() for int and long primitive types.
>
> Would it make sense to replace (x ^ r) < 0 && (x ^ y) >= 0
> in
>
> +public static int addExact(int x, int y) {
> +int r = x + y;
> +if ((x ^ r) < 0 && (x ^ y) >= 0) {
> +throw new ArithmeticException("Addition overflows an int");
> +}
> +return r;
> +}
>
> with (x ^ y ^ r) < 0?
>
> For substraction, you could use ((x ^ r) & (x ^ y)) < 0.
>
> Will Hotspot be able to optimize away the string construction on the
> exception path in multiplyExact() if the exception is caught locally?
>
> --
> Florian Weimer
> BFK edv-consulting GmbH   http://www.bfk.de/
> Kriegsstraße 100  tel: +49-721-96201-1
> D-76133 Karlsruhe fax: +49-721-96201-99
>


Re: JDK 8 code review request for initial unsigned integer arithmetic library support

2012-01-20 Thread Eamonn McManus
On 20 January 2012 17:53, Ulf Zibis  wrote:

> Am 21.01.2012 01:35, schrieb Joseph Darcy:
>
>> On 1/19/2012 8:05 AM, Ulf Zibis wrote:
>>
>>> But again, moving the entire method to BigInteger would additionally
>>> avoid to clown around with the available BigInteger's public APIs. Having
>>> the method at BigInteger would allow elegant direct access to the private
>>> value fields.
>>>
>>
>> If the operation in question starts becoming a bottleneck, these
>> alternate implementations can be explored.
>>
> But the alternatives for potentially faster algorithms would be limited if
> you stick BigInteger toUnsignedBigInteger(long i) to class Long.


There's no reason Long and BigInteger can't conspire to achieve this
without changing their APIs, if it proves interesting. It's not completely
straightforward since they are in different packages, but perfectly
possible using a sun.* intermediary.

Éamonn


Re: JDK 8 code review request for initial unsigned integer arithmetic library support

2012-01-18 Thread Eamonn McManus
Ulf Zibis writes:
> What about:
>  private static final BigInteger BEYOND_UNSIGNED_LONG =
BigInteger.valueOf(1).**shiftLeft(64);
>  private static BigInteger toUnsignedBigInteger(long i) {
>  BigInteger result = BigInteger.valueOf(i);
>  if (i < 0L)
>  result = result.add(BEYOND_UNSIGNED_**LONG);
>  return result;
>  }

That's a nice idea! But the problem is that it would mean that
BigInteger.class would be loaded as soon as Long.class is, which I think is
undesirable. However it does make me think that we could change this:

if (i >= 0L)
return BigInteger.valueOf(i);
else {
int upper = (int) (i >>> 32);
int lower = (int) i;

// return (upper << 32) + lower
return
(BigInteger.valueOf(Integer.toUnsignedLong(upper))).shiftLeft(32).
add(BigInteger.valueOf(Integer.toUnsignedLong(lower)));
}

to this:

if (i >= 0L) {
return BigInteger.valueOf(i);
} else {
return BigInteger.valueOf(i & Long.MAX_VALUE).setBit(63);
}

Éamonn


On 18 January 2012 19:52, Ulf Zibis  wrote:

> Am 18.01.2012 03:54, schrieb Joe Darcy:
>
>  I've posted a revised webrev at
>>
>>
>> http://cr.openjdk.java.net/~**darcy/4504839.2
>>
>
> Instead
> '\u0030'
> you can use
>{@code '\u005Cu0030'}
>
> Byte:
> =
>  459 public static int toUnsignedInt(byte x) {
>  460 return ((int) x) & 0xff;
>  461 }
> This should be good enough (similar at Short, Integer):
>  459 public static int toUnsignedInt(byte x) {
>  460 return x & 0xff;
>  461 }
> (This notation if regularly used in sun.nio.cs coders.)
>
> missing:
>public static short toUnsignedShort(byte x)
>
> superfluous:
>public static long toUnsignedInt(byte x)
>public static long toUnsignedLong(byte x) (similar at Short)
> one can use:
>int i = toUnsignedShort(x)
>long l = toUnsignedShort(x) (similar at Short)
>
> Integer:
> 
>  623  * The value represented by the string is larger than the
>  624  * largest unsigned {@code int}, 232-1.
> If you format {@code int}, then you speak about the java type int, which
> is always signed, never unsigned.
> IMO you should better write 'unsigned 32-bit integer".
> (similar at Long)
>
>  598  * Parses the string argument as an unsigned integer in the radix
>  599  * specified by the second argument.
> IMHO, there should be a note about what happens on values above 2^31 - 1.
>
>  672  * Parses the string argument as an unsigned decimal integer. The
>  673  * characters in the string must all be decimal digits, except
> Better, like lines 598ff, or contrariwise (similar at Long):
>  672  * Parses the string argument as an unsigned decimal integer.
>  673  *
>  674  * The characters in the string must all be decimal digits, except
>
> Long:
> =
> What about:
>private static final BigInteger BEYOND_UNSIGNED_LONG =
> BigInteger.valueOf(1).**shiftLeft(64);
>private static BigInteger toUnsignedBigInteger(long i) {
>BigInteger result = BigInteger.valueOf(i);
>if (i < 0L)
>result = result.add(BEYOND_UNSIGNED_**LONG);
>return result;
>}
>
> Instead
>private static BigInteger toUnsignedBigInteger(long i)
> at class BigInteger we more generally could have:
>public static BigInteger unsignedValueOf(long i)
>
>  610  * Parses the string argument as an unsigned {@code long} in the
>  611  * radix specified by the second argument.
> IMHO, there should be a note about what happens on values above 2^63 - 1.
>
> -Ulf
>
>


Re: JDK 8 code review request for initial unsigned integer arithmetic library support

2012-01-17 Thread Eamonn McManus
Hi Joe,

That looks great to me (emcmanus). One thing I noticed is that the
behaviour is not explicitly specified when parseUnsignedLong is given a
null String reference. But I see that is also true of the existing
parseLong and valueOf(String) and decode(String), so perhaps there should
be a separate bug to update the spec there. The phrase "If the string
cannot be parsed as a long" does not cover this case as obviously as it
might.

Cheers,
Éamonn


On 17 January 2012 18:54, Joe Darcy  wrote:

>  Hi Eamonn,
>
>
> On 01/15/2012 09:53 AM, Eamonn McManus wrote:
>
> It's great to see this!
>
>
> I agree :-)
>
> I've posted a revised webrev at
>
> http://cr.openjdk.java.net/~darcy/4504839.2
>
> More detailed responses inline.
>
>
> The API looks reasonable to me.
>
>  > For the first cut, I've favored keeping the code straightforward over
> trickier but potentially faster algorithms.
>
>  The code looks clean and correct to me. But I think we could afford one
> or two cheap improvements to Long without diving into the full-blown
> Hacker's Delight algorithms:
>
>  In toUnsignedBigInteger(i) we could check whether i is nonnegative and
> use plain BigInteger.valueOf(i) in that case. Also, although the difference
> is sure to be unmeasurable, I think (int) (i >>> 32) would be better
> than (int) ((i >> 32) & 0x).
>
>
> Good points; changed.
>
>
>
>  In parseUnsignedLong, we can avoid using BigInteger by parsing all but
> the last digit as a positive number and then adding in that digit:
>  long first = parseLong(s.substring(0, len - 1), radix);
> int second = Character.digit(s.charAt(len - 1), radix);
> if (second < 0) {
> throw new NumberFormatException("Bad digit at end of " + s);
> }
> long result = first * radix + second;
> if (compareUnsigned(result, first) < 0) {
> throw new NumberFormatException(String.format("String value %s
> exceeds " +
> "range of unsigned
> long.", s));
> }
>  By my measurements this speeds up the parsing of random decimal unsigned
> longs by about 2.5 times. Changing the existing code to move the limit
> constant to a field or to test for overflow using bi.bitLength() instead
> still leaves it about twice as slow.
>
>
> Changed.
>
> Also from some off-list comments from Mike, I've modified the first
> sentence of the parseUnsignedLong methods to explicitly mention the "long"
> type; this is consistent with the phrasing of the signed parseLong methods
> in java.lang.Long.
>
>
>
>  In divideUnsigned, after eliminating negative divisors we could check
> whether the dividend is also nonnegative and use plain division in that
> case.
>
>
> Changed.
>
>
>
>  In remainderUnsigned, we could check whether both arguments are
> nonnegative and use plain % in that case, and we could also check whether
> the divisor is unsigned-less than the dividend, and return it directly in
> that case.
>
>
> Changed.
>
> I've also added test cases for the unsigned divide and remainder methods.
>
> Thanks again,
>
> -Joe
>
>
>
> Éamonn
>
>
> On 13 January 2012 21:26, Joe Darcy  wrote:
>
>> Hello,
>>
>> Polishing up some work I've had *almost* done for a long time, please
>> review an initial take on providing library support for unsigned integer
>> arithmetic:
>>
>>4504839 Java libraries should provide support for unsigned integer
>> arithmetic
>>4215269 Some Integer.toHexString(int) results cannot be decoded back
>> to an int
>>6322074 Converting integers to string as if unsigned
>>
>>http://cr.openjdk.java.net/~darcy/4504839.1/
>>
>> For the first cut, I've favored keeping the code straightforward over
>> trickier but potentially faster algorithms.  Tests need to be written for
>> the unsigned divide and remainder methods, but otherwise the regression
>> tests are fairly extensive.
>>
>> To avoid the overhead of having to deal with boxed objects, the unsigned
>> functionality is implemented as static methods on Integer and Long, etc. as
>> opposed to introducing new types like UnsignedInteger and UnsignedLong.
>>
>> (This work is not meant to preclude other integer arithmetic enhancements
>> from going into JDK 8, such as add/subtract/multiply/divide methods that
>> throw exceptions on overflow.)
>>
>> Thanks,
>>
>> -Joe
>>
>>
>>
>
>


Re: JDK 8 code review request for initial unsigned integer arithmetic library support

2012-01-15 Thread Eamonn McManus
It's great to see this! The API looks reasonable to me.

> For the first cut, I've favored keeping the code straightforward over
trickier but potentially faster algorithms.

The code looks clean and correct to me. But I think we could afford one or
two cheap improvements to Long without diving into the full-blown Hacker's
Delight algorithms:

In toUnsignedBigInteger(i) we could check whether i is nonnegative and use
plain BigInteger.valueOf(i) in that case. Also, although the difference is
sure to be unmeasurable, I think (int) (i >>> 32) would be better
than (int) ((i >> 32) & 0x).

In parseUnsignedLong, we can avoid using BigInteger by parsing all but the
last digit as a positive number and then adding in that digit:
long first = parseLong(s.substring(0, len - 1), radix);
int second = Character.digit(s.charAt(len - 1), radix);
if (second < 0) {
throw new NumberFormatException("Bad digit at end of " + s);
}
long result = first * radix + second;
if (compareUnsigned(result, first) < 0) {
throw new NumberFormatException(String.format("String value %s
exceeds " +
"range of unsigned
long.", s));
}
By my measurements this speeds up the parsing of random decimal unsigned
longs by about 2.5 times. Changing the existing code to move the limit
constant to a field or to test for overflow using bi.bitLength() instead
still leaves it about twice as slow.

In divideUnsigned, after eliminating negative divisors we could check
whether the dividend is also nonnegative and use plain division in that
case.

In remainderUnsigned, we could check whether both arguments are nonnegative
and use plain % in that case, and we could also check whether the divisor
is unsigned-less than the dividend, and return it directly in that case.

Éamonn


On 13 January 2012 21:26, Joe Darcy  wrote:

> Hello,
>
> Polishing up some work I've had *almost* done for a long time, please
> review an initial take on providing library support for unsigned integer
> arithmetic:
>
>4504839 Java libraries should provide support for unsigned integer
> arithmetic
>4215269 Some Integer.toHexString(int) results cannot be decoded back to
> an int
>6322074 Converting integers to string as if unsigned
>
>
> http://cr.openjdk.java.net/~**darcy/4504839.1/
>
> For the first cut, I've favored keeping the code straightforward over
> trickier but potentially faster algorithms.  Tests need to be written for
> the unsigned divide and remainder methods, but otherwise the regression
> tests are fairly extensive.
>
> To avoid the overhead of having to deal with boxed objects, the unsigned
> functionality is implemented as static methods on Integer and Long, etc. as
> opposed to introducing new types like UnsignedInteger and UnsignedLong.
>
> (This work is not meant to preclude other integer arithmetic enhancements
> from going into JDK 8, such as add/subtract/multiply/divide methods that
> throw exceptions on overflow.)
>
> Thanks,
>
> -Joe
>
>
>


Re: Cleanup fallthrough in FilePermission and PropertyPermission was: Request for Review of 7116890 (Warning Cleanup java.io)

2011-12-05 Thread Eamonn McManus
To my mind, introducing a new shared parent means defining a new API
that can be used by third-party subclasses. The point is that these
permissions have an action list like "read,write,delete", with parsing
code that converts this into a bitmask and toString() code that
converts it back. I think that code is pretty hairy as it stands, all
the more so because it is duplicated across the existing subclasses.
For MBeanPermission, where there are 17 possible actions, the parsing
method is 370 lines long! So refactoring is definitely desirable.

An abstract PermissionWithActions class could use enums to good
effect, something like this:

public abstract class PermissionWithActions> {
public abstract Class actionClass();
private final EnumSet actions;
protected PermissionWithAction(String target, String actions) {
...use reflection to get E[] values() and parse the actions...
}
public String getActions() {
...use toString().toLowerCase() on the actions set...
}
public final Set getActionSet() {
return Collections.unmodifiableSet(actions);
}
public boolean implies(Permission p) {
if (p.getClass() != this.getClass()) {
return false;
}
@SuppressWarnings("unchecked")
PermissionWIthActions pwa = (PermissionWithActions) p;
return this.actions.containsAll(pwa.actions);
}
}
public final class FilePermission extends
PermissionWithActions {
@Override public Class actionClass() {
return FliePermissionActions.class;
}
}
public enum FilePermissionActions {READ, WRITE, EXECUTE, DELETE}

However I'm not sure we actually want to make such an API change now.
A safer path would be to refactor the parsing and getActions() code
into a sun.* class which is not the parent class of anything, using
delegation rather than inheritance. Safer both because it doesn't
involve figuring out the impact on various JSRs, and because there is
some danger that subclassing might introduce unforeseen attack
vectors. FilePermission etc could each have one instance of this class
that they use for parsing and getActions(), and that instance could
construct e.g. a trie for parsing that is essentially as fast as the
unmaintainable hand-coded parsers that each of the affected Permission
subclasses has today. Parsing does have to be fast because when there
is a security manager every checked operation will construct a
Permission to check against.

Éamonn


On 5 December 2011 07:45, Sebastian Sickelmann
 wrote:
>
> Am 05.12.2011 12:53, schrieb Weijun Wang:
>
>>
>>
>> On 12/03/2011 06:12 AM, Stuart Marks wrote:
>>>
>>> I'm adding Weijun (Max) Wang to this thread.
>>>
>>> The same "ackbarfaccept" code had come up a third time when I was
>>> reviewing some of Max's changes. The code in question all has to do with
>>> permissions, and Max is in the security group, so he might have a better
>>> insight whether doing a refactoring is appropriate and how to approach
>>> doing it.
>>
>>
>> Maybe we can group all permissions with a target and a predefined set of 
>> actions under a new abstract class.
>
>
> This sounds like the best idea from the implementors point of view. I would 
> prepare a patch as suggestion for this.
> Regarding to my own question i would say that there isn't any additional 
> jigsaw related problem here by
> introducing a common abstract class. They all rely on 
> java.security.Permission, so java.security seems the correct
> package for the new abstract class.
>
> Any suggestions for the name? PermissionWithActions AbstractPermission??
>
> If i see it right there is the following class hierarchy:
>
> j.s.Permission implements Serializable
> +j.i.FilePermission
> +j.n.SocketPermission
> +jx.m.MBeanPermission
> +jx.s.a.k.ServicePermission
> +j.s.BasicPermission
>  +j.u.PropertyPermission
>
> What happens to serialization when the new hierarchy would look like this:
>
> j.s.Permission implements Serializable
> +j.s.AbstractPermission
> +j.i.FilePermission
> +j.n.SocketPermission
>  +jx.m.MBeanPermission
> +jx.s.a.k.ServicePermission
>  +j.s.BasicPermission
>  +j.u.PropertyPermission
>
> All the classes have a serialVerionUID so i think serialisation is no 
> problem, or is there
> a problem i i don't see here?
>
> -- Sebastian
>
>> -Max
>>
>>>
>>> Some searching around reveals that "ackbarfaccept" appears in the
>>> PlayerPermission class of the Java ME Mobile Media API (not in any of
>>> the OpenJDK repositories) so it looks like the code has been copied even
>>> more times than are visible here. Perhaps a public permissions parsing
>>> utility method is called for.
>>>
>>> s'marks
>>>
>>> On 12/2/11 9:02 AM, Sebastian Sickelmann wrote:

 Am 02.12.2011 16:27, schrieb Brandon Passanisi:
>
> Hi Sebastian. I'm not sure if you had seen the e-mail from Stuart Marks
> regarding this, but Stuart was able to find more instances of the
> similar
> block of "fallthrough" code. I can volunte

Re: Expected behavior with -esa

2011-11-23 Thread Eamonn McManus
Isn't an AssertionError *always* a bug?

Éamonn


On 23 November 2011 12:04, Brandon Passanisi
 wrote:
>
> I'm currently looking at a bug report: 
> http://monaco.sfbay.sun.com/detail.jsf?cr=5066854 which describes an issue 
> where a simple program was run with the option -esa to enable system 
> assertions.  The bug author describes that, in the provided sample code, an 
> AssertionError occurs when the spec says a MissingFormatArgumentException 
> should be thrown.  When running the sample program without the -esa option, a 
> MissingFormatArgumentException is thrown.  It seems to me that it shouldn't 
> be unexpected to see an AssertionError when -esa is used, instead of the 
> MissingFormatArgumentException.  In fact, this behavior appears to not be 
> limited to this particular test case, and can possibly occur with other APIs 
> if an AssertionError is seen before an expected exception.  I was hoping to 
> see some sort of explanation in the evaluation or comments section of the bug 
> report, but the bug report was only updated with references about how the bug 
> was filed too late for a release.  Can someone clarify if this behavior with 
> -esa is actually a bug?
>
> Thanks.


Re: hg: jdk8/tl/jdk: 7080020: Add conventional constructors to InternalError and VirtualMachineError

2011-08-25 Thread Eamonn McManus
Yeah, I guess so. We'd have to add @SuppressWarnings("serial") to some 
non-public classes but so what.


Éamonn


On 25/8/11 6:01 PM, Joe Darcy wrote:

Eamonn McManus wrote:
Could we perhaps have the JDK build use an annotation processor that 
requires every public class in java.* javax.* etc that is 
serializable to either declare a serialVersionUID or have 
@SuppressWarnings("serial")? The amount of grief we have had over the 
years with this would easily justify the effort.


We could achieve that effect using the "-Xlint:serial -Werror" option 
to javac during the JDK build.


-Joe



Éamonn


On 25/8/11 9:00 AM, Joe Darcy wrote:

Alan Bateman wrote:

Joe Darcy wrote:

Hi Alan.

I did check for that point in my review :-)

The VirtualMachineError class is abstract so as long as all its 
subclasses declare a serialVersionUID, like InternalError does, I 
think we're fine.
We need to check the serialization protocol but I'm pretty the 
object stream class of the supertype goes into the stream too. Also 
I quickly checked an an up-to-date build of jdk8/tl and its 
currently unable to deserialize streams contain any of the virtual 
machine errors like OutOfMemoryError, StackOverflowError, etc.


*sigh*

Thanks for checking on this Alan; I'll prepare a changeset with the 
explicit serialVersionUID on InternalError.


-Joe





Re: hg: jdk8/tl/jdk: 7080020: Add conventional constructors to InternalError and VirtualMachineError

2011-08-25 Thread Eamonn McManus
Could we perhaps have the JDK build use an annotation processor that 
requires every public class in java.* javax.* etc that is serializable 
to either declare a serialVersionUID or have 
@SuppressWarnings("serial")? The amount of grief we have had over the 
years with this would easily justify the effort.


Éamonn


On 25/8/11 9:00 AM, Joe Darcy wrote:

Alan Bateman wrote:

Joe Darcy wrote:

Hi Alan.

I did check for that point in my review :-)

The VirtualMachineError class is abstract so as long as all its 
subclasses declare a serialVersionUID, like InternalError does, I 
think we're fine.
We need to check the serialization protocol but I'm pretty the object 
stream class of the supertype goes into the stream too. Also I 
quickly checked an an up-to-date build of jdk8/tl and its currently 
unable to deserialize streams contain any of the virtual machine 
errors like OutOfMemoryError, StackOverflowError, etc.


*sigh*

Thanks for checking on this Alan; I'll prepare a changeset with the 
explicit serialVersionUID on InternalError.


-Joe



Re: JDK 8 code review request for 7082231 "Put a @since 1.7 on System.lineSeparator"

2011-08-24 Thread Eamonn McManus
Looks good. But I am thinking that this shows we need a regression test 
that ensures that there are no missing @since tags, given that JDK 7 
went out without this one. I am imagining two doclets, one of which 
creates a file with a list of the signatures of every public element 
(class, method, constructor, or field), and the other of which checks 
that every public element is either listed in the file or has a @since 
tag. So we would have run the first doclet on JDK 6 and then used its 
output with the second doclet to check JDK 7. I'm probably stating the 
obvious.


Éamonn


On 24/8/11 12:43 AM, Joe Darcy wrote:

Hello.

Please review this simple patch to fix 7082231 "Put a @since 1.7 on 
System.lineSeparator", which will correct an @since tag omitted by the 
JDK 7 fix for 6900043 "Add method to return line.separator property."


diff --git a/src/share/classes/java/lang/System.java 
b/src/share/classes/java/lang/System.java

--- a/src/share/classes/java/lang/System.java
+++ b/src/share/classes/java/lang/System.java
@@ -632,6 +632,7 @@
 *
 * On UNIX systems, it returns {@code "\n"}; on Microsoft
 * Windows systems it returns {@code "\r\n"}.
+ * @since 1.7
 */
public static String lineSeparator() {
return lineSeparator;


Thanks,

-Joe


Re: Review for CR 6728865 : Improved heuristics for Collections.disjoint() [updated]

2010-12-21 Thread Eamonn McManus
I think that is still not quite right. The spec for Collection says that 
a Collection that does not support adding null values may or may not 
support looking them up. So in both places where you say "does not 
permit null values" I think you should probably say "does not permit 
looking up null values".


Éamonn

On 21/12/2010 20:35, Mike Duigou wrote:

On Dec 21 2010, at 02:43 , Chris Hegarty wrote:


On 12/21/10 02:24 AM, David Holmes wrote:

Functionality looks okay to me.

I think those spec/doc clarifications may need to go through CCC though.

I agree with David, a CCC request should be filed for the spec changes. We 
should agree the spec changes on this mailing list before proposing them.

I understand where you're coming from with this spec change, but I think the 
additional text may be too restricting.

  "@throws NullPointerException . or if one collection
   contains {...@code null} and the other collection does not permit
   {...@code null} values."

For example, the following would be required to throw NPE ( but I don't believe 
your impl does):

  Set set = new HashSet();
  set.add(null);
  PriorityQueue pq = new PriorityQueue();
  Collections.disjoint(set, pq);

I think we may have to be a little more relaxed here, maybe just a cautionary note, 
"it may happen"???

You are correct that it's not guaranteed that NPE will be thrown. Here's the 
amended text for the main javadoc:

  *Care must also be exercised when using a mix of collections that
  * permit {...@code null} values and those that do not. If either
  * collection does not permit {...@code null} values then {...@code null} 
must
  * not be a value in either collection.
  *

and this is the revised @throw NullPointerException:

  * @throws NullPointerException if either collection is {...@code null}. 
May
  * also be thrown if one collection contains a {...@code null} value and 
the
  * other collection does not permit {...@code null} values.


Note that the descriptive paragraph says "must not" because we don't commit to which 
collection is used for contains() and the @throw says "may" because, per your example, if 
the collection not permitting null is used for iteration then NPE will not be thrown.

Mike


-Chris.


David

Mike Duigou said the following on 12/21/10 11:48:

I've updated the webrev with Ulf's feedback.
http://cr.openjdk.java.net/~mduigou/6728865.2/webrev/

The old heuristics:

1. If c1 is a Set then iterate over c2.

2. Iterate over the smaller Collection.


I believe that the || in the original should have been a&&

I've rearranged it as branches in my revision.


The new heuristics:

1. If c1 is a Set then iterate over c2.

2. If c2 is a Set then iterate over c1.

3. If either collection is empty then result is always true.

4. Iterate over the smaller Collection.

Mike



On Dec 19 2010, at 16:42 , David Holmes wrote:


Hi Mike,

Mike Duigou said the following on 12/20/10 10:29:

I have updated the webrev for CR 6728865 with Rémi's feedback. The
new webrev is at:
http://cr.openjdk.java.net/~mduigou/6728865.1/webrev/
The size() comparisons are now done only when both c1 and c2 are not
sets and I have removed the isEmpty() micro-optimization.

So to summarise this change:

1. The original code checked for c1 being a set and not c2, but not
vice-versa - this fixes that

2. This code adds an optimization when they are both not sets but at
least one is empty

Did I miss anything?

Seems functionally sound to me, but I can't attest to any performance
benefits.

Cheers,
David Holmes




Re: Review request for 6977034 Thread.getState() very slow

2010-12-06 Thread Eamonn McManus


  
  
Reviewed OK!
The constant JVMTI_THREAD_STATE_WAITING
is not used but that's my only minor niggle.
Éamonn [emcmanus]

On 06/12/2010 20:26, Mandy Chung wrote:

  
  Remi, Eamonn, Brian, David, Doug,
  
  Thanks for the feedback.
  
  On 12/04/10 04:22, Eamonn McManus wrote:
  


Hi Mandy,
This test:

if ((threadStatus & JVMTI_THREAD_STATE_RUNNABLE) == 1) {

is always false, since JVMTI_THREAD_STATE_RUNNABLE is 4.
  (NetBeans 7.0 helpfully flags this; I'm not sure if earlier
  versions do.)

  
  
  Good catch.   This explains why the speed up for RUNNABLE was not
  as high in the microbenchmark measurement.  Correcting it shows
  that Thread.getState() gets 3.5X speed up on a thread in RUNNABLE
  state.
  
  
 
But, once corrected, I think you could use this idea further
  to write a much simpler and faster method, on these lines:
public static Thread.State toThreadState(int threadStatus) {
if ((threadStatus & JVMTI_THREAD_STATE_RUNNABLE) != 0) {
return RUNNABLE;
} else if ((threadStatus & JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER) != 0) {
return BLOCKED;
} else if ((threadStatus & JVMTI_THREAD_STATE_WAITING_WITH_TIMEOUT) != 0) {
return TIMED_WAITING;
} else if ((threadStatus & JVMTI_THREAD_STATE_WAITING_INDEFINITELY) != 0) {
return WAITING;
} else if ((threadStatus & JVMTI_THREAD_STATE_TERMINATED) != 0) {
return TERMINATED;
} else {
return NEW;
}
}

  
  
  I forgot to mention in the email that I implemented this simpler
  approach to compare with the table lookup approach.   There were
  no significant difference.  I now rerun with the corrected fix
  (checking != 0 rather than == 1) and the table lookup approach is
  about 2-6% faster than the sequence of tests approach.
  
  I am also for the simpler approach but I post the table lookup
  approach as a proposed fix to get any opinion on the performance
  aspect with that approach.
  
  Given that the Fork-Join framework doesn't depend on it, I will go
  for a simpler approach (sequence of tests) and further tune its
  performance when there is a use case requiring a perf improvement.
  
  New webrev:
     http://cr.openjdk.java.net/~mchung/6977034/webrev.01/
  
  Can you review this version?
  
  Thanks
  Mandy
  
  
You could tweak the order of the tests based on what might be
  the relative frequency of the different states but it probably
  isn't worth it.

Regards,
Éamonn

On 3/12/10 11:52 PM, Mandy Chung wrote:

  
  
  Fix for 6977034: Thread.getState() very slow
  
  Webrev at:
     http://cr.openjdk.java.net/~mchung/6977034/webrev.00/
  
  This is an improvement to map a Thread's threadStatus field to
  Thread.State.  The VM updates the Thread.threadStatus field
  directly at state transition with the value as defined in JVM
  TI [1].  The java.lang.Thread.getState() implementation can
  directly access the threadStatus value and do a direct lookup
  from an array of Thread.State.  The threadStatus value is a
  bit vector and we would have to create an array of a minimum
  of 1061 (0x425) elements to do direct mapping.   I took the
  approach to use the first highest order bit set to 1 in the
  masked threadStatus value as the index to the Thread.State
  element and only caches 32 elements (could be fewer).  I wrote
  a micro-benchmark measuring the Thread.getState of a thread in
  different state that shows 1.7X to 6X speedup (see below). 
  There is possibly some issue with my micro-benchmark that I
  didn't observe the 14X speed up as Doug did in his
  experiment.  However, I'd like to get this reviewed and pushed
  to the repository so that anyone can do more experiment on the
  performance measurement.
  
  Thanks
  Mandy
  P.S. The discussion on this thread can be found at [2] [3].
  
  [1] http://download.java.net/jdk7/docs/platform/jvmti/jvmti.html#GetThreadState
  [2] http://mail.openjdk.java.net/pipermail/core-libs-dev/2010-July/004567.html
  [3] http://mail.openjdk.java.net/pipermail/core-libs-dev/2010-August/004721.html
  
  
  	JDK 7 b120 (in ms)	With fix (in ms)	Speed up
main		46465	22772			2.04
NEW		50676		29921			1.69
RUNNABLE	42202		14690			2.87
BLOCKED		72773		12

Re: Review request for 6977034 Thread.getState() very slow

2010-12-05 Thread Eamonn McManus


  
  
Yeah, it was a bit blithe of me to write that the sequence of
  tests was faster. In the table-lookup version, if you get rid of
  the initial test for RUNNABLE, and if you use
  Integer.numberOfLeadingZeros, and if the JIT compiler intrinsifies
  that to a native processor instruction, and if the lookup table is
  in L1 cache, then the table-lookup version will run in constant
  time and be better than the worst case of the sequence-of-tests
  version, and probably better than the average case too. But, as
  you say, that last if (the cache hit) will usually not be
  true, and in that case I would not be surprised if the sequence of
  tests were faster even in its worst case.

Anyway the sequence-of-tests version is unquestionably simpler,
  and I would venture that the best solution is probably to go with
  that, plus a new method in the API that explicitly tests whether a
  thread is runnable. That's trivial to implement now that Mandy has
  pulled the knowledge of state bits into the Java code rather than
  being hidden in the bowels of the VM; and its implementation will
  be faster than (Thread.getState() == RUNNABLE) regardless of the
  implementation of the latter.


Éamonn

On 5/12/10 8:27 AM, Brian Goetz wrote:
As
  Eamonn writes it, it will never cache miss but may frequently
  branch mispredict (possibly multiple times).  If you do a shift +
  mask + index into a small table, it will cache miss most the time
  but never branch mispredict. (In a real program it will cache miss
  frequently since thread state calls are infrequent and the lookup
  table will fall out of cache; in a microbenchmark it will almost
  never cache miss as the lookup table will be hot.)
  
  
  On 12/4/2010 7:22 AM, Eamonn McManus wrote:
  
  Hi Mandy,


This test:


 if ((threadStatus&  JVMTI_THREAD_STATE_RUNNABLE) ==
1) {


is always false, since JVMTI_THREAD_STATE_RUNNABLE is 4.
(NetBeans 7.0

helpfully flags this; I'm not sure if earlier versions do.)


But, once corrected, I think you could use this idea further to
write a much

simpler and faster method, on these lines:


 public static Thread.State toThreadState(int threadStatus)
{

 if ((threadStatus&  JVMTI_THREAD_STATE_RUNNABLE)*!=
0*) {

 return RUNNABLE;

 } else if ((threadStatus& 
JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER) != 0) {

 return BLOCKED;

 } else if ((threadStatus& 
JVMTI_THREAD_STATE_WAITING_WITH_TIMEOUT) != 0) {

 return TIMED_WAITING;

 } else if ((threadStatus& 
JVMTI_THREAD_STATE_WAITING_INDEFINITELY) != 0) {

 return WAITING;

 } else if ((threadStatus& 
JVMTI_THREAD_STATE_TERMINATED) != 0) {

 return TERMINATED;

 } else {

 return NEW;

 }

 }


You could tweak the order of the tests based on what might be
the relative

frequency of the different states but it probably isn't worth
it.


Regards,


Éamonn



On 3/12/10 11:52 PM, Mandy Chung wrote:

Fix for 6977034: Thread.getState() very
  slow
  
  
  Webrev at:
  
  http://cr.openjdk.java.net/~mchung/6977034/webrev.00/
  
  
  This is an improvement to map a Thread's threadStatus field to
  Thread.State.
  
  The VM updates the Thread.threadStatus field directly at state
  transition
  
  with the value as defined in JVM TI [1]. The
  java.lang.Thread.getState()
  
  implementation can directly access the threadStatus value and
  do a direct
  
  lookup from an array of Thread.State. The threadStatus value
  is a bit vector
  
  and we would have to create an array of a minimum of 1061
  (0x425) elements
  
  to do direct mapping. I took the approach to use the first
  highest order bit
  
  set to 1 in the masked threadStatus value as the index to the
  Thread.State
  
  element and only caches 32 elements (could be fewer). I wrote
  a
  
  micro-benchmark 

Re: Review request for 6977034 Thread.getState() very slow

2010-12-04 Thread Eamonn McManus


  
  
Hi Mandy,
This test:

if ((threadStatus & JVMTI_THREAD_STATE_RUNNABLE) == 1) {

is always false, since JVMTI_THREAD_STATE_RUNNABLE is 4.
  (NetBeans 7.0 helpfully flags this; I'm not sure if earlier
  versions do.)

But, once corrected, I think you could use this idea further to
  write a much simpler and faster method, on these lines:
public static Thread.State toThreadState(int threadStatus) {
if ((threadStatus & JVMTI_THREAD_STATE_RUNNABLE) != 0) {
return RUNNABLE;
} else if ((threadStatus & JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER) != 0) {
return BLOCKED;
} else if ((threadStatus & JVMTI_THREAD_STATE_WAITING_WITH_TIMEOUT) != 0) {
return TIMED_WAITING;
} else if ((threadStatus & JVMTI_THREAD_STATE_WAITING_INDEFINITELY) != 0) {
return WAITING;
} else if ((threadStatus & JVMTI_THREAD_STATE_TERMINATED) != 0) {
return TERMINATED;
} else {
return NEW;
}
}

You could tweak the order of the tests based on what might be the
  relative frequency of the different states but it probably isn't
  worth it.

Regards,
Éamonn

On 3/12/10 11:52 PM, Mandy Chung wrote:

  
  
  Fix for 6977034: Thread.getState() very slow
  
  Webrev at:
     http://cr.openjdk.java.net/~mchung/6977034/webrev.00/
  
  This is an improvement to map a Thread's threadStatus field to
  Thread.State.  The VM updates the Thread.threadStatus field
  directly at state transition with the value as defined in JVM TI
  [1].  The java.lang.Thread.getState() implementation can directly
  access the threadStatus value and do a direct lookup from an array
  of Thread.State.  The threadStatus value is a bit vector and we
  would have to create an array of a minimum of 1061 (0x425)
  elements to do direct mapping.   I took the approach to use the
  first highest order bit set to 1 in the masked threadStatus value
  as the index to the Thread.State element and only caches 32
  elements (could be fewer).  I wrote a micro-benchmark measuring
  the Thread.getState of a thread in different state that shows 1.7X
  to 6X speedup (see below).  There is possibly some issue with my
  micro-benchmark that I didn't observe the 14X speed up as Doug did
  in his experiment.  However, I'd like to get this reviewed and
  pushed to the repository so that anyone can do more experiment on
  the performance measurement.
  
  Thanks
  Mandy
  P.S. The discussion on this thread can be found at [2] [3].
  
  [1]
  http://download.java.net/jdk7/docs/platform/jvmti/jvmti.html#GetThreadState
  [2]
  http://mail.openjdk.java.net/pipermail/core-libs-dev/2010-July/004567.html
  [3]
  http://mail.openjdk.java.net/pipermail/core-libs-dev/2010-August/004721.html
  
  
  	JDK 7 b120 (in ms)	With fix (in ms)	Speed up
main		46465	22772			2.04
NEW		50676		29921			1.69
RUNNABLE	42202		14690			2.87
BLOCKED		72773		12296			5.92
WAITING		48811		13041			3.74
TIMED_WAITING	45737		12849			3.56
TERMINATED	40314		16376			2.46

  

  



hg: jdk7/tl/jdk: 6984037: jmx/management rebranding vendor changes needed

2010-10-29 Thread eamonn . mcmanus
Changeset: 7fee717f4707
Author:emcmanus
Date:  2010-10-29 12:35 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/7fee717f4707

6984037: jmx/management rebranding vendor changes needed
Reviewed-by: ohair

! make/netbeans/jmx/build.properties
! src/share/classes/com/sun/jmx/defaults/ServiceName.java
! src/share/classes/com/sun/jmx/snmp/ServiceName.java
! src/share/classes/com/sun/management/package.html
! src/share/classes/javax/management/ObjectName.java
! src/share/classes/javax/management/build.xml
! src/share/classes/javax/management/modelmbean/ModelMBeanNotificationInfo.java



Re: Removing a layer of synchronization on Math.random

2010-06-05 Thread Eamonn McManus
It seems to me that if two threads call this Math.random() at the same 
time then two instances of Random() can be constructed. That contradicts 
the specification of the method, and is theoretically observable because 
the two values from Math.random() will typically not be a pair of values 
that could have been returned from consecutive calls to nextDouble() on 
a single Random instance. Granted, the chances of this actually 
mattering are infinitesimal, but I think the performance gain of 
avoiding a synchronized method only on the very first call to 
Math.random() is infinitesimal too. So I'd be inclined to leave well 
enough alone here.

Eamonn

Martin Buchholz wrote:

Here's an optimization for Math.random() that appears to be safe,
despite the double-check access:

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/Math.random/

David, if you agree, could you file a bug?

Thanks,

Martin
  


hg: jdk7/tl/jdk: 6851617: Remove JSR 255 (JMX API 2.0) from JDK 7

2009-10-21 Thread eamonn . mcmanus
Changeset: f23a3ae59169
Author:emcmanus
Date:  2009-10-21 17:33 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/f23a3ae59169

6851617: Remove JSR 255 (JMX API 2.0) from JDK 7
Summary: See 
http://weblogs.java.net/blog/2009/06/16/jsr-255-jmx-api-20-postponed
Reviewed-by: dfuchs

! make/docs/CORE_PKGS.gmk
! src/share/classes/com/sun/jmx/defaults/JmxProperties.java
! src/share/classes/com/sun/jmx/defaults/ServiceName.java
- src/share/classes/com/sun/jmx/event/DaemonThreadFactory.java
- src/share/classes/com/sun/jmx/event/EventBuffer.java
- src/share/classes/com/sun/jmx/event/EventClientFactory.java
- src/share/classes/com/sun/jmx/event/EventConnection.java
- src/share/classes/com/sun/jmx/event/EventParams.java
- src/share/classes/com/sun/jmx/event/LeaseManager.java
- src/share/classes/com/sun/jmx/event/LeaseRenewer.java
- src/share/classes/com/sun/jmx/event/ReceiverBuffer.java
- src/share/classes/com/sun/jmx/event/RepeatedSingletonJob.java
! src/share/classes/com/sun/jmx/interceptor/DefaultMBeanServerInterceptor.java
- src/share/classes/com/sun/jmx/interceptor/DispatchInterceptor.java
- src/share/classes/com/sun/jmx/interceptor/DomainDispatchInterceptor.java
- src/share/classes/com/sun/jmx/interceptor/MBeanServerInterceptorSupport.java
- src/share/classes/com/sun/jmx/interceptor/NamespaceDispatchInterceptor.java
- src/share/classes/com/sun/jmx/interceptor/SingleMBeanForwarder.java
! src/share/classes/com/sun/jmx/mbeanserver/ConvertingMethod.java
! src/share/classes/com/sun/jmx/mbeanserver/DefaultMXBeanMappingFactory.java
! src/share/classes/com/sun/jmx/mbeanserver/DynamicMBean2.java
! src/share/classes/com/sun/jmx/mbeanserver/Introspector.java
! src/share/classes/com/sun/jmx/mbeanserver/JmxMBeanServer.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanAnalyzer.java
- src/share/classes/com/sun/jmx/mbeanserver/MBeanInjector.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanInstantiator.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanSupport.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanIntrospector.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanLookup.java
+ src/share/classes/com/sun/jmx/mbeanserver/MXBeanMapping.java
+ src/share/classes/com/sun/jmx/mbeanserver/MXBeanMappingFactory.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanProxy.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanSupport.java
- src/share/classes/com/sun/jmx/mbeanserver/NotificationMBeanSupport.java
- src/share/classes/com/sun/jmx/mbeanserver/NotifySupport.java
! src/share/classes/com/sun/jmx/mbeanserver/PerInterface.java
- src/share/classes/com/sun/jmx/mbeanserver/PerThreadGroupPool.java
! src/share/classes/com/sun/jmx/mbeanserver/Repository.java
! src/share/classes/com/sun/jmx/mbeanserver/StandardMBeanIntrospector.java
! src/share/classes/com/sun/jmx/mbeanserver/StandardMBeanSupport.java
! src/share/classes/com/sun/jmx/mbeanserver/Util.java
- src/share/classes/com/sun/jmx/namespace/DomainInterceptor.java
- src/share/classes/com/sun/jmx/namespace/HandlerInterceptor.java
- src/share/classes/com/sun/jmx/namespace/NamespaceInterceptor.java
- src/share/classes/com/sun/jmx/namespace/ObjectNameRouter.java
- src/share/classes/com/sun/jmx/namespace/RoutingConnectionProxy.java
- src/share/classes/com/sun/jmx/namespace/RoutingMBeanServerConnection.java
- src/share/classes/com/sun/jmx/namespace/RoutingProxy.java
- src/share/classes/com/sun/jmx/namespace/RoutingServerProxy.java
- src/share/classes/com/sun/jmx/namespace/package.html
- src/share/classes/com/sun/jmx/namespace/serial/DefaultRewritingProcessor.java
- src/share/classes/com/sun/jmx/namespace/serial/IdentityProcessor.java
- src/share/classes/com/sun/jmx/namespace/serial/JMXNamespaceContext.java
- src/share/classes/com/sun/jmx/namespace/serial/RewritingProcessor.java
- src/share/classes/com/sun/jmx/namespace/serial/RoutingOnlyProcessor.java
- src/share/classes/com/sun/jmx/namespace/serial/SerialRewritingProcessor.java
- src/share/classes/com/sun/jmx/namespace/serial/package.html
! src/share/classes/com/sun/jmx/remote/internal/ServerNotifForwarder.java
! src/share/classes/com/sun/jmx/remote/util/EnvHelp.java
- src/share/classes/com/sun/jmx/remote/util/EventClientConnection.java
! src/share/classes/java/lang/management/PlatformComponent.java
! src/share/classes/java/util/logging/Logging.java
! src/share/classes/javax/management/AndQueryExp.java
! src/share/classes/javax/management/AttributeList.java
! src/share/classes/javax/management/AttributeValueExp.java
! src/share/classes/javax/management/BetweenQueryExp.java
! src/share/classes/javax/management/BinaryOpValueExp.java
! src/share/classes/javax/management/BinaryRelQueryExp.java
- src/share/classes/javax/management/ClientContext.java
- src/share/classes/javax/management/Description.java
! src/share/classes/javax/management/Descriptor.java
- src/share/classes/javax/management/DescriptorFields.java
! src/share/classes/

Re: j.u.Objects follow-up: deepEquals(Object, Object)?

2009-10-09 Thread Eamonn McManus

> Don't you know about assertArrayEquals() in JUnit ? It's in version 4.5.

I can use that if I know the values are arrays.  The sort of case I'm thinking
of is a data-driven test where I have a big array with a lot of inputs and
the corresponding expected outputs, some but not all of which might be arrays,
including primitive arrays.  For example, the inputs are expressions in a script
language and the outputs are what those expressions should evaluate to.  The 
test
iterates over the array and does assertDeepEquals for each input/output pair.

But it is true that I could use the same new Object[] {x} trick there, in
other words define assertDeepEquals in terms of assertArrayEquals,
to get better failure messages.  The main advantages I see of
deepEquals(Object, Object) are that it avoids allocating a throw-away array
(which is not a concern in unit tests), and more importantly that it is
easier to find than the new Object[] {x} trick.

Éamonn McManus · JMX Spec Lead · http://weblogs.java.net/blog/emcmanus


Ulf Zibis wrote:

Am 09.10.2009 11:30, Eamonn McManus schrieb:

Joseph D. Darcy wrote:
> What are scenarios where this method would be used?

I use a similar method fairly often in unit tests.  JUnit's 
assertEquals doesn't
do the right thing if its arguments happen to be arrays, so I use the 
following

simple if inefficient implementation:


Don't you know about assertArrayEquals() in JUnit ? It's in version 4.5.

-Ulf






Re: First round of java.util.Objects for code review (bug 6797535)

2009-10-09 Thread Eamonn McManus

> The spec, you mention, refers to the instance method equals(), but here
> we are talking about static helpers.

The difference between Marek's suggestion and Joe's is what happens when
the equals(Object) method of a or b returns true for a null argument, and
that is what I was saying violates the spec.

Éamonn McManus · JMX Spec Lead · http://weblogs.java.net/blog/emcmanus


Ulf Zibis wrote:

Am 09.10.2009 10:56, Eamonn McManus schrieb:

Hi,

Marek Kozieł wrote:
>> +public static boolean equals(Object a, Object b) {
>> +return (a == b) || (a != null && a.equals(b));
>> +}
>
> Hello,
> I would suggest other implementation of equals method:
>
> public static boolean equals(Object a, Object b) {
> if (a == null) return (b == null ? true : b.equals(null));
> if (b == null) return a.equals(null);
> return a.equals(b);
> }
>
> This one could cover case when null is considered as object in object
> equals method [...]

An equals implementation that can return true when its argument is 
null violates its contract since
it is not symmetric.  In fact, the spec of Object.equals specifically 
says that the method must

return false when the argument is null.



The spec, you mention, refers to the instance method equals(), but here 
we are talking about static helpers.


But anyway, I don't understand the point of Marek's implementation.

-Ulf






Re: j.u.Objects follow-up: deepEquals(Object, Object)?

2009-10-09 Thread Eamonn McManus

Joseph D. Darcy wrote:
> What are scenarios where this method would be used?

I use a similar method fairly often in unit tests.  JUnit's assertEquals doesn't
do the right thing if its arguments happen to be arrays, so I use the following
simple if inefficient implementation:

static void deepEquals(Object x, Object y) {
return Arrays.deepEquals(new Object[] {x}, new Object[] {y});
}

What that shows of course is that the messy logic you mention is already 
present in
Arrays.deepEquals so you could factor it out.

Regards,
Éamonn McManus · JMX Spec Lead · http://weblogs.java.net/blog/emcmanus


Joseph D. Darcy wrote:
Another piece of functionality requested in the j.u.Objects thread was a 
deepEquals(Object a, Object b.) method that "did the right thing" if the 
arguments happened to dynamically be arrays.


I've been thinking a bit how this might be implemented.

The array-ness of a and b would need to be determined, after any 
up-front null-checks


boolean aIsArray = a.getClass().isArray();
boolean bIsArray = b.getClass().isArray();

followed various case-analyses.

if (aIsArray && bIsArray) {
Class aComponentType = a.getClass().getComponentType();
Class bComponentType = b.getClass().getComponentType();
   if (aComponentType == bComponentType) {
   // long case analysis to cast and call Arrays.deepEquals if 
ComponentType is a reference type
   // or the matching Arrays.equals(primitiveComponent[], 
primitiveComponent[]) method if

   // aComponentType.isPrimitive().
   } else
   return false;
} else
   return a.equals(b);

Certainly a bit messy internally.

What are scenarios where this method would be used?

-Joe




Re: Objects.toString [Re: What methods should go into a java.util.Objects class in JDK 7?]

2009-10-09 Thread Eamonn McManus

David Holmes - Sun Microsystems wrote:
> So to me: String toString(Object o, String useIfNull) is the only method
> that provides true utility in this case.

I agree with that, and would just suggest to the person specifying the method
to add a @see String#valueOf(Object).  I find that the behaviour of that method
is usually exactly what I want when the argument is null (and I find the idea of
representing null by an empty string aberrant, but there you go), so for people 
like
me but who might not have noticed String.valueOf this will save them from 
peppering
Objects.toString(x, "null") throughout their programs.

Éamonn McManus · JMX Spec Lead · http://weblogs.java.net/blog/emcmanus


David Holmes - Sun Microsystems wrote:

Joe,

Joseph D. Darcy said the following on 10/09/09 04:30:

System.out.println("" + referenceOfAnyType);

will print "null" if referenceOfAnyType is null.

This is what the platform has done since the beginning.


Yes because String concatenation can not tolerate null values appearing, 
so it is defined to replace null with "null" (or replace a null from 
o.toString() with "null"). And I suspect that "null" was chosen over "" 
because it is then obvious when an unexpected null was encountered.


But Objects.toString(o) is not concerned with string concatenation so 
doesn't have to mimic that behaviour.


Personally I'd only put in the two-arg variant because:

a) a one-arg that simply forwards to String.valueOf is redundant as 
already discussed


b) a one-arg that hardwires the response to null (whether that be to 
return null or "null") will simply force the programmer to either switch 
to a different API or else add their own null handling logic - which 
defeats the purpose of putting in these utility/convenience methods.


So to me: String toString(Object o, String useIfNull) is the only method 
that provides true utility in this case.


David




Re: First round of java.util.Objects for code review (bug 6797535)

2009-10-09 Thread Eamonn McManus

Hi,

Marek Kozieł wrote:
>> +public static boolean equals(Object a, Object b) {
>> +return (a == b) || (a != null && a.equals(b));
>> +}
>
> Hello,
> I would suggest other implementation of equals method:
>
>public static boolean equals(Object a, Object b) {
>if (a == null) return (b == null ? true : b.equals(null));
>if (b == null) return a.equals(null);
>return a.equals(b);
>}
>
> This one could cover case when null is considered as object in object
> equals method [...]

An equals implementation that can return true when its argument is null 
violates its contract since
it is not symmetric.  In fact, the spec of Object.equals specifically says that 
the method must
return false when the argument is null.

Regards,
Éamonn McManus · JMX Spec Lead · http://weblogs.java.net/blog/emcmanus


Marek Kozieł wrote:

2009/10/8 Joseph D. Darcy :

Hello.

Please code review the first-round of java.util.Objects; the patch is below:
http://cr.openjdk.java.net/~darcy/6797535.0/

-Joe

--- old/make/java/java/FILES_java.gmk   2009-10-08 11:04:03.0 -0700
+++ new/make/java/java/FILES_java.gmk   2009-10-08 11:04:02.0 -0700
@@ -258,6 +258,7 @@
   java/util/ServiceConfigurationError.java \
   java/util/Timer.java \
   java/util/TimerTask.java \
+java/util/Objects.java \
   java/util/UUID.java \
   java/util/concurrent/AbstractExecutorService.java \
   java/util/concurrent/ArrayBlockingQueue.java \
--- /dev/null   2009-07-06 20:02:10.0 -0700
+++ new/src/share/classes/java/util/Objects.java2009-10-08
11:04:03.0 -0700
@@ -0,0 +1,102 @@
+/*
+ * Copyright 2009 Sun Microsystems, Inc.  All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Sun designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Sun in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License
version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+package java.util;
+
+/**
+ * This class consists of {...@code static} utility methods for operating
+ * on objects.  These utilities include {...@code null}-safe or {...@code
+ * null}-tolerant methods for computing the hash code of an object,
+ * returning a string for an object, and comparing two objects.
+ *
+ * @since 1.7
+ */
+public class Objects {
+private Objects() {
+throw new AssertionError("No java.util.Objects instances for
you!");
+}
+
+/**
+ * Returns {...@code true} if the arguments are equal to each other
+ * and {...@code false} otherwise.
+ * Consequently, if both arguments are {...@code null}, {...@code true}
+ * is returned and if exactly one argument is {...@code null}, {...@code
+ * false} is returned.  Otherwise, equality is determined by using
+ * the {...@link Object#equals equals} method of the first
+ * argument.
+ *
+ * @return {...@code true} if the arguments are equal to each other
+ * and {...@code false} otherwise
+ * @see Object#equals(Object)
+ */
+public static boolean equals(Object a, Object b) {
+return (a == b) || (a != null && a.equals(b));
+}


Hello,
I would suggest other implementation of equals method:

public static boolean equals(Object a, Object b) {
if (a == null) return (b == null ? true : b.equals(null));
if (b == null) return a.equals(null);
return a.equals(b);
}

This one could cover case when null is considered as object in object
equals method, while in not time implementation result might depend
from arguments order:

class Boo {
public String   data;

@Override
public boolean equals(Object obj) {
if (obj == null) return (data == null);//
...
}
}






Re: Spec update for Class#getDeclaredMethods()

2009-07-16 Thread Eamonn McManus




Code that relies on Class.getDeclaredMethods() returning the methods
in source-code order is currently wrong not just in theory but in
practice. That the JDK appears to return them in that order is a
fragile coincidence, as I explained in this
blog entry. Apparently unrelated changes anywhere else in the app
can cause the order to change.

The proposed change could be made, but it is not just a spec
change. All JVMs that don't currently conform to the spec would need to
change, as Rémi Forax notes, and that includes the JDK and its
derivatives. I'm not a HotSpot expert but my guess is that there's some
pretty sensitive code involved that the HotSpot team might be reluctant
to go poking around in.

Éamonn McManus · JMX Spec Lead · http://weblogs.java.net/blog/emcmanus 



Florian Weimer wrote:

  I suggest to replace:

| The elements [methods] in the array returned are not sorted and are
| not in any particular order.

with:

| If the class is a compiled Java class, the elements in the array are
| sorted according to the order of declaration in the source code.
| Otherwise, the elements are not in any particular order.

There is a growing amount of code which relies on predictable method
order, so backwards compatibility concerns implicitly dictate the
ordering.  The proposed change makes this explicit.

The other reflection methods should be updated in a similar fashion.

  





hg: jdk7/tl/jdk: 6456269: Add a GenericMBeanException so clients don't have to have server's exception classes present

2008-12-10 Thread eamonn . mcmanus
Changeset: c8db1ddbdba4
Author:emcmanus
Date:  2008-12-10 11:59 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/c8db1ddbdba4

6456269: Add a GenericMBeanException so clients don't have to have server's 
exception classes present
Reviewed-by: jfdenise, dfuchs

! src/share/classes/javax/management/Descriptor.java
+ src/share/classes/javax/management/GenericMBeanException.java
! src/share/classes/javax/management/MBeanException.java
+ test/javax/management/interop/MBeanExceptionInteropTest.java
+ test/javax/management/openmbean/GenericMBeanExceptionTest.java



hg: jdk7/tl/jdk: 6780803: Wrong parameter name in description of EventClient::addListeners(); ...

2008-12-09 Thread eamonn . mcmanus
Changeset: 0b1c7f982cc0
Author:emcmanus
Date:  2008-12-09 18:30 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/0b1c7f982cc0

6780803: Wrong parameter name in description of EventClient::addListeners()
6470295: Misleading exception message says context classloader when it isn't
6714954: Description of MBeanPermission checking in MBeanServer javadoc is 
inaccurate
6732037: Event Service spec needs more detail about Executor use
6740900: Specify that listeners invoked via SendNotification should not block
6778436: Typo in @NotificationInfos spec
Reviewed-by: dfuchs

! src/share/classes/com/sun/jmx/mbeanserver/MBeanInstantiator.java
! src/share/classes/javax/management/MBeanRegistration.java
! src/share/classes/javax/management/MBeanServer.java
! src/share/classes/javax/management/event/EventClient.java
! src/share/classes/javax/management/event/FetchingEventRelay.java



hg: jdk7/tl/jdk: 6774918: @NotificationInfo is ineffective on MBeans that cannot send notifications

2008-12-09 Thread eamonn . mcmanus
Changeset: b4bf1806ee66
Author:emcmanus
Date:  2008-12-09 12:01 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/b4bf1806ee66

6774918: @NotificationInfo is ineffective on MBeans that cannot send 
notifications
Reviewed-by: jfdenise

! src/share/classes/com/sun/jmx/mbeanserver/MBeanInjector.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java
! src/share/classes/javax/management/NotificationInfo.java
! test/javax/management/Introspector/AnnotatedNotificationInfoTest.java



hg: jdk7/tl/jdk: 6776225: JMX.isNotificationSource wrong when DynamicWrapperMBean + SendNotification injection

2008-11-27 Thread eamonn . mcmanus
Changeset: 24a31530683d
Author:emcmanus
Date:  2008-11-27 15:44 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/24a31530683d

6776225: JMX.isNotificationSource wrong when DynamicWrapperMBean + 
SendNotification injection
Reviewed-by: dfuchs, jfdenise

! src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java
! src/share/classes/javax/management/JMX.java
! src/share/classes/javax/management/StandardEmitterMBean.java
! src/share/classes/javax/management/StandardMBean.java
! test/javax/management/MBeanServer/DynamicWrapperMBeanTest.java



hg: jdk7/tl/jdk: 6772779: @NotificationInfo does not create MBeanNotificationInfo in the MBean's MBeanInfo; ...

2008-11-20 Thread eamonn . mcmanus
Changeset: 098e456e860e
Author:emcmanus
Date:  2008-11-20 10:10 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/098e456e860e

6772779: @NotificationInfo does not create MBeanNotificationInfo in the MBean's 
MBeanInfo
6773593: CompositeDataSupport constructor javadoc is not in sync with the 
implementation
Reviewed-by: sjiang

! src/share/classes/com/sun/jmx/interceptor/DefaultMBeanServerInterceptor.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java
! src/share/classes/javax/management/openmbean/CompositeDataSupport.java
! test/javax/management/Introspector/AnnotatedNotificationInfoTest.java



hg: jdk7/tl/jdk: 6336968: Methods to convert AttributeList to/from Map; ...

2008-11-07 Thread eamonn . mcmanus
Changeset: 2410a0b48d06
Author:emcmanus
Date:  2008-11-07 19:19 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/2410a0b48d06

6336968: Methods to convert AttributeList to/from Map
6750008: Add JMX.getSpecificationVersion(MBeanServerConnection) and document 
interop
6750472: Add a way to convert a CompositeData into a Map
6752563: Allow CompositeDataSupport to have zero items
Summary: Small JMX RFEs
Reviewed-by: dfuchs

! src/share/classes/javax/management/AttributeList.java
! src/share/classes/javax/management/JMX.java
! src/share/classes/javax/management/MBeanServerConnection.java
! src/share/classes/javax/management/MBeanServerNotification.java
! src/share/classes/javax/management/QueryNotificationFilter.java
! src/share/classes/javax/management/openmbean/CompositeDataSupport.java
! src/share/classes/javax/management/package.html
+ test/javax/management/MBeanServer/AttributeListMapTest.java
+ test/javax/management/MBeanServer/AttributeListTypeSafeTest.java
+ test/javax/management/openmbean/CompositeDataToMapTest.java
+ test/javax/management/remote/mandatory/version/JMXSpecVersionTest.java



hg: jdk7/tl/jdk: 5072267: A way to communicate client context such as locale to the JMX server

2008-11-07 Thread eamonn . mcmanus
Changeset: 810a95940b99
Author:emcmanus
Date:  2008-11-07 11:48 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/810a95940b99

5072267: A way to communicate client context such as locale to the JMX server
Summary: Support for client contexts and also for localization of descriptions
Reviewed-by: dfuchs

! src/share/classes/com/sun/jmx/defaults/ServiceName.java
! src/share/classes/com/sun/jmx/event/EventParams.java
! src/share/classes/com/sun/jmx/event/LeaseManager.java
! src/share/classes/com/sun/jmx/interceptor/SingleMBeanForwarder.java
! src/share/classes/com/sun/jmx/mbeanserver/JmxMBeanServer.java
- src/share/classes/com/sun/jmx/namespace/JMXNamespaceUtils.java
! src/share/classes/com/sun/jmx/namespace/ObjectNameRouter.java
! src/share/classes/com/sun/jmx/namespace/RoutingConnectionProxy.java
! src/share/classes/com/sun/jmx/namespace/RoutingProxy.java
! src/share/classes/com/sun/jmx/namespace/RoutingServerProxy.java
! src/share/classes/com/sun/jmx/remote/util/EventClientConnection.java
+ src/share/classes/javax/management/ClientContext.java
! src/share/classes/javax/management/Descriptor.java
! src/share/classes/javax/management/JMX.java
! src/share/classes/javax/management/MBeanInfo.java
! src/share/classes/javax/management/MBeanServerNotification.java
! src/share/classes/javax/management/Notification.java
! src/share/classes/javax/management/event/EventClient.java
! src/share/classes/javax/management/event/EventClientDelegate.java
! src/share/classes/javax/management/event/EventClientDelegateMBean.java
! src/share/classes/javax/management/event/EventRelay.java
! src/share/classes/javax/management/event/package-info.java
! src/share/classes/javax/management/namespace/JMXNamespaces.java
! src/share/classes/javax/management/namespace/JMXRemoteNamespace.java
! src/share/classes/javax/management/remote/JMXConnectorFactory.java
! src/share/classes/javax/management/remote/JMXConnectorServer.java
! src/share/classes/javax/management/remote/JMXConnectorServerMBean.java
! src/share/classes/javax/management/remote/rmi/RMIConnector.java
! src/share/classes/javax/management/remote/rmi/RMIConnectorServer.java
! test/javax/management/Introspector/AnnotationTest.java
+ test/javax/management/context/ContextForwarderTest.java
+ test/javax/management/context/ContextTest.java
+ test/javax/management/context/LocaleAwareBroadcasterTest.java
+ test/javax/management/context/LocaleTest.java
+ test/javax/management/context/LocalizableTest.java
+ test/javax/management/context/RemoteContextTest.java
+ test/javax/management/context/localizable/MBeanDescriptions.properties
+ test/javax/management/context/localizable/MBeanDescriptions_fr.java
+ test/javax/management/context/localizable/Whatsit.java
+ test/javax/management/context/localizable/WhatsitMBean.java
! test/javax/management/eventService/CustomForwarderTest.java
! test/javax/management/eventService/EventClientExecutorTest.java
! test/javax/management/eventService/EventManagerTest.java
! test/javax/management/eventService/ListenerTest.java
! test/javax/management/eventService/NotSerializableNotifTest.java
! test/javax/management/eventService/UsingEventService.java
! test/javax/management/namespace/EventWithNamespaceControlTest.java
! test/javax/management/namespace/JMXNamespaceSecurityTest.java
! test/javax/management/namespace/JMXNamespaceViewTest.java
! test/javax/management/namespace/JMXRemoteTargetNamespace.java
! test/javax/management/namespace/NamespaceNotificationsTest.java
! test/javax/management/namespace/NullDomainObjectNameTest.java
! test/javax/management/namespace/NullObjectNameTest.java
! test/javax/management/openmbean/CompositeDataStringTest.java
! test/javax/management/remote/mandatory/connectorServer/ForwarderChainTest.java
! 
test/javax/management/remote/mandatory/connectorServer/StandardForwardersTest.java
! test/javax/management/remote/mandatory/provider/ProviderTest.java
! test/javax/management/remote/mandatory/subjectDelegation/SimpleStandard.java



hg: jdk7/tl/jdk: 6766173: Spec should say that createMBean wraps a constructor RuntimeException in a RuntimeMBeanException

2008-10-31 Thread eamonn . mcmanus
Changeset: 8d17cc67a857
Author:emcmanus
Date:  2008-10-31 17:34 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/8d17cc67a857

6766173: Spec should say that createMBean wraps a constructor RuntimeException 
in a RuntimeMBeanException
Summary: JMX spec clarification
Reviewed-by: dfuchs

! src/share/classes/javax/management/MBeanServerConnection.java
! src/share/classes/javax/management/namespace/MBeanServerSupport.java
! test/javax/management/MBeanServer/MBeanExceptionTest.java



hg: jdk7/tl/jdk: 6450848: make it easier to get the ObjectName of a JMX Proxy

2008-10-30 Thread eamonn . mcmanus
Changeset: cdfb6f963a60
Author:emcmanus
Date:  2008-10-30 18:19 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/cdfb6f963a60

6450848: make it easier to get the ObjectName of a JMX Proxy
Summary: Rework proxy javadoc to explain how to do this.
Reviewed-by: sjiang

! src/share/classes/javax/management/JMX.java



hg: jdk7/tl/jdk: 6252609: Two different default descriptor forms defined for ModelMBeanInfoSupport; ...

2008-10-30 Thread eamonn . mcmanus
Changeset: 8dcde0b16199
Author:emcmanus
Date:  2008-10-30 17:46 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/8dcde0b16199

6252609: Two different default descriptor forms defined for 
ModelMBeanInfoSupport
6253137: Documentation for NotificationListener's handback parameter is 
confusing
6368691: javadoc for JMX Descriptors has bugs and is very hard to navigate.
6602699: support for async notification of mbeaninfo update
6759612: [javadoc] EventClient.NOTIFS_LOST has a garbled href to 
addEventClientListener
6759619: Clarify what EventClient.getEventClientNotificationInfo does
6759622: Clarify what EventClient.getListeners list does
Summary: Documentation fixes, plus simple bugfix for 6759619.
Reviewed-by: dfuchs

! src/share/classes/javax/management/Descriptor.java
! src/share/classes/javax/management/MBeanInfo.java
! src/share/classes/javax/management/MBeanServer.java
! src/share/classes/javax/management/NotificationListener.java
! src/share/classes/javax/management/event/EventClient.java
! src/share/classes/javax/management/modelmbean/DescriptorSupport.java
! src/share/classes/javax/management/modelmbean/ModelMBeanAttributeInfo.java
! src/share/classes/javax/management/modelmbean/ModelMBeanConstructorInfo.java
! src/share/classes/javax/management/modelmbean/ModelMBeanInfo.java
! src/share/classes/javax/management/modelmbean/ModelMBeanNotificationInfo.java
! src/share/classes/javax/management/modelmbean/ModelMBeanOperationInfo.java
! test/javax/management/eventService/CustomForwarderTest.java
! test/javax/management/mxbean/TypeNameTest.java



hg: jdk7/tl/jdk: 6763051: MXBean: Incorrect type names for parametrized dealing with arrays (openType); ...

2008-10-28 Thread eamonn . mcmanus
Changeset: 58e52eb46bd3
Author:emcmanus
Date:  2008-10-28 18:21 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/58e52eb46bd3

6763051: MXBean: Incorrect type names for parametrized dealing with arrays 
(openType)
6713777: developer diagnosability of errors in uncompliant mxbean interfaces
Reviewed-by: dfuchs

! src/share/classes/com/sun/jmx/mbeanserver/DefaultMXBeanMappingFactory.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanIntrospector.java
! src/share/classes/javax/management/MBeanServerInvocationHandler.java
+ test/javax/management/mxbean/ExceptionDiagnosisTest.java
! test/javax/management/mxbean/TypeNameTest.java



hg: jdk7/tl/jdk: 6763639: Remove "rawtypes" warnings from JMX code

2008-10-27 Thread eamonn . mcmanus
Changeset: 76ecb928e83a
Author:emcmanus
Date:  2008-10-27 14:02 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/76ecb928e83a

6763639: Remove "rawtypes" warnings from JMX code
Reviewed-by: dfuchs

! make/netbeans/jmx/build.xml
! src/share/classes/com/sun/jmx/event/LeaseManager.java
! src/share/classes/com/sun/jmx/event/LeaseRenewer.java
! src/share/classes/com/sun/jmx/interceptor/DefaultMBeanServerInterceptor.java
! src/share/classes/com/sun/jmx/mbeanserver/ClassLoaderRepositorySupport.java
! src/share/classes/com/sun/jmx/mbeanserver/ConvertingMethod.java
! src/share/classes/com/sun/jmx/mbeanserver/DefaultMXBeanMappingFactory.java
! src/share/classes/com/sun/jmx/mbeanserver/Introspector.java
! src/share/classes/com/sun/jmx/mbeanserver/JmxMBeanServer.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanInstantiator.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanIntrospector.java
! src/share/classes/com/sun/jmx/mbeanserver/ObjectInputStreamWithLoader.java
! src/share/classes/com/sun/jmx/mbeanserver/SecureClassLoaderRepository.java
! src/share/classes/com/sun/jmx/mbeanserver/Util.java
! src/share/classes/com/sun/jmx/mbeanserver/WeakIdentityHashMap.java
! src/share/classes/com/sun/jmx/remote/internal/ArrayNotificationBuffer.java
! src/share/classes/com/sun/jmx/remote/internal/ClientNotifForwarder.java
! src/share/classes/com/sun/jmx/remote/internal/ProxyInputStream.java
! src/share/classes/com/sun/jmx/remote/internal/ServerNotifForwarder.java
! src/share/classes/com/sun/jmx/remote/internal/Unmarshal.java
! src/share/classes/com/sun/jmx/remote/security/FileLoginModule.java
! src/share/classes/com/sun/jmx/remote/security/JMXPluggableAuthenticator.java
! 
src/share/classes/com/sun/jmx/remote/security/MBeanServerFileAccessController.java
! src/share/classes/com/sun/jmx/remote/util/ClassLoaderWithRepository.java
! src/share/classes/com/sun/jmx/remote/util/ClassLogger.java
! src/share/classes/com/sun/jmx/remote/util/EnvHelp.java
! src/share/classes/com/sun/jmx/remote/util/EventClientConnection.java
! src/share/classes/com/sun/jmx/remote/util/OrderClassLoaders.java
! src/share/classes/javax/management/AttributeList.java
! src/share/classes/javax/management/DefaultLoaderRepository.java
! src/share/classes/javax/management/JMRuntimeException.java
! src/share/classes/javax/management/JMX.java
! src/share/classes/javax/management/MBeanAttributeInfo.java
! src/share/classes/javax/management/MBeanConstructorInfo.java
! src/share/classes/javax/management/MBeanInfo.java
! src/share/classes/javax/management/MBeanOperationInfo.java
! src/share/classes/javax/management/MBeanServerFactory.java
! src/share/classes/javax/management/MBeanServerInvocationHandler.java
! src/share/classes/javax/management/StandardMBean.java
! src/share/classes/javax/management/event/EventClientDelegate.java
! src/share/classes/javax/management/event/EventSubscriber.java
! src/share/classes/javax/management/loading/DefaultLoaderRepository.java
! src/share/classes/javax/management/loading/MLet.java
! src/share/classes/javax/management/loading/MLetObjectInputStream.java
! src/share/classes/javax/management/modelmbean/DescriptorSupport.java
! src/share/classes/javax/management/modelmbean/ModelMBeanConstructorInfo.java
! src/share/classes/javax/management/modelmbean/RequiredModelMBean.java
! src/share/classes/javax/management/openmbean/ArrayType.java
! 
src/share/classes/javax/management/openmbean/CompositeDataInvocationHandler.java
! src/share/classes/javax/management/openmbean/CompositeType.java
! 
src/share/classes/javax/management/openmbean/OpenMBeanAttributeInfoSupport.java
! 
src/share/classes/javax/management/openmbean/OpenMBeanParameterInfoSupport.java
! src/share/classes/javax/management/openmbean/OpenType.java
! src/share/classes/javax/management/openmbean/SimpleType.java
! src/share/classes/javax/management/openmbean/TabularDataSupport.java
! src/share/classes/javax/management/openmbean/TabularType.java
! src/share/classes/javax/management/relation/MBeanServerNotificationFilter.java
! src/share/classes/javax/management/relation/RelationService.java
! src/share/classes/javax/management/relation/RelationSupport.java
! src/share/classes/javax/management/relation/Role.java
! src/share/classes/javax/management/relation/RoleList.java
! src/share/classes/javax/management/relation/RoleResult.java
! src/share/classes/javax/management/relation/RoleUnresolved.java
! src/share/classes/javax/management/relation/RoleUnresolvedList.java
! src/share/classes/javax/management/remote/JMXConnectorFactory.java
! src/share/classes/javax/management/remote/rmi/NoCallStackClassLoader.java
! src/share/classes/javax/management/remote/rmi/RMIConnection.java
! src/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
! src/share/classes/javax/management/remote/rmi/RMIConnector.java
! src/share/classes/javax/management/remote/rmi/RMIConnectorServer.java
! src/share/classes/javax/management/remote/rmi/RMIServerImpl.java
! src/share/clas

Re: Maintenance Reviews was: [PATCH] Enhance ServiceLoader to understand factory methods

2008-10-22 Thread Eamonn McManus




Jaroslav,
I can act as sponsor for this change.

My understanding from looking at the code is that the change is as
follows. Today, you can supply an implementation of the SPI
com.example.Foo by putting a jar file in the classpath (etc) that
contains a file called META-INF/services/com.example.Foo which might
contain lines like:

com.example.providers.Bar

To instantiate this provider, ServiceLoader loads the class
com.example.providers.Bar and instantiates its public no-arg
constructor. The Bar class must be a subtype of com.example.Foo.

With your proposed change, the above will still be possible, but
additionally the file can contain lines like:

com.example.providers.Bar.baz()

To instantiate this provider, ServiceLoader loads the class
com.example.providers.Bar and calls its public static method baz(). The
Bar class does not have to be a subtype of com.example.Foo but the
return type of baz() does.

Can you say a bit more about the rationale here? I understand why
SPIs should be defined with abstract classes rather than interfaces,
but ServiceLoader already supports both classes and interfaces. I can
see why final classes might be of interest (so you can add new methods
without fearing a clash with an existing subclass method name), and I
guess you have in mind that such classes could have their behaviour
defined via constructor parameters, with each new version adding a new
constructor; but rather than guessing it would be nice to see an
example.

Regards,
Éamonn McManus · JMX Spec Lead · http://weblogs.java.net/blog/emcmanus 



Jaroslav Tulach wrote:

  Thanks for the clarification, Mark. I've heard about some umbrella JSR for 
each release of the JDK before and I was hoping that my proposed change
could be covered as part of it. Thanks for confirming that such aggregation
is possible.

  
  
With regard to this particular proposal, as the original author of ...

  
  
OK, I can promise that I will work hard on polishing my proposal. I can 
improve the code, I can add more documentation, I can write more tests, 
change the new API semantics, etc. I just need a sponsor to tell me what to 
change and where and then take and apply my final patch.

Thanks in advance for your help.
-jst

Dne Friday 17 October 2008 06:24:13 Mark Reinhold napsal(a):
  
  

  Date: Fri, 17 Oct 2008 05:56:13 +1000
From: [EMAIL PROTECTED]

If this is a modification to the API not just a patch for the
implementation then the short answer is No.
  

Well, not exactly.



         API changes have to be made
via a JSR.
  

... or in a Maintenance Review of an existing JSR.

Small API revisions and additions do not require their own JSR; that
would be ridiculously inefficient.  Instead we roll them up into a
series of Maintenance Reviews of the previous Platform JSR [1].

There have already been many small API changes in the JDK 7 codebase.
Suggested patches for further such changes are welcome, and will be
discussed and judged on their own technical merit.  All implemented
changes will, in due course, be aggregated into an appropriate JCP
Maintenance Review.

With regard to this particular proposal, as the original author of
the class in question I suppose I should have an opinion about it,
but that's a topic for a different message.

- Mark


[1]


  
  http://weblogs.java.net/blog/mreinhold/archive/2006/03/mustang_mainten.html


  





hg: jdk7/tl/jdk: 6757225: MXBean: Incorrect type names for parametrized types, dealing with arrays

2008-10-08 Thread eamonn . mcmanus
Changeset: 86799e45c230
Author:emcmanus
Date:  2008-10-08 18:38 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/86799e45c230

6757225: MXBean: Incorrect type names for parametrized types, dealing with 
arrays
Reviewed-by: sjiang

! src/share/classes/com/sun/jmx/mbeanserver/MXBeanIntrospector.java
! src/share/classes/javax/management/event/EventClient.java
! src/share/classes/javax/management/event/FetchingEventRelay.java
! src/share/classes/javax/management/monitor/Monitor.java
! src/share/classes/javax/management/remote/rmi/RMIConnector.java
+ test/javax/management/mxbean/TypeNameTest.java



hg: jdk7/tl/jdk: 6750935: The expected NotCompliantMBeanException is not thrown for the custom MXBeanMappingFactory; ...

2008-09-24 Thread eamonn . mcmanus
Changeset: 48a790c67659
Author:emcmanus
Date:  2008-09-24 15:19 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/48a790c67659

6750935: The expected NotCompliantMBeanException is not thrown for the custom 
MXBeanMappingFactory
6751872: MXBeanMappingFactory example says "implements" when it should be 
"extends"
Reviewed-by: dfuchs

! src/share/classes/com/sun/jmx/mbeanserver/Introspector.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanSupport.java
! src/share/classes/javax/management/openmbean/MXBeanMappingFactory.java
! test/javax/management/mxbean/CustomTypeTest.java



hg: jdk7/tl/jdk: 6747411: EventClient causes thread leaks

2008-09-12 Thread eamonn . mcmanus
Changeset: ebc38225b4a9
Author:emcmanus
Date:  2008-09-12 15:17 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/ebc38225b4a9

6747411: EventClient causes thread leaks
Summary: Reworked thread management in EventClient and related classes.
Reviewed-by: sjiang, dfuchs

! src/share/classes/com/sun/jmx/event/LeaseManager.java
! src/share/classes/com/sun/jmx/event/RepeatedSingletonJob.java
! src/share/classes/com/sun/jmx/remote/internal/ClientCommunicatorAdmin.java
! src/share/classes/javax/management/event/EventClient.java
! src/share/classes/javax/management/event/FetchingEventRelay.java
! src/share/classes/javax/management/event/RMIPushEventForwarder.java
! src/share/classes/javax/management/remote/rmi/RMIConnector.java
+ test/javax/management/eventService/EventClientThreadTest.java
! test/javax/management/eventService/SharingThreadTest.java



hg: jdk7/tl/jdk: 6746759: Fix for 6734813 introduced build break

2008-09-10 Thread eamonn . mcmanus
Changeset: b35ccd203a7e
Author:emcmanus
Date:  2008-09-10 14:56 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/b35ccd203a7e

6746759: Fix for 6734813 introduced build break
Reviewed-by: dfuchs

! src/share/classes/sun/management/ClassLoadingImpl.java
! src/share/classes/sun/management/CompilationImpl.java
! src/share/classes/sun/management/HotSpotDiagnostic.java
! src/share/classes/sun/management/HotspotInternal.java
! src/share/classes/sun/management/ManagementFactoryHelper.java
! src/share/classes/sun/management/MemoryImpl.java
! src/share/classes/sun/management/OperatingSystemImpl.java
! src/share/classes/sun/management/RuntimeImpl.java
! src/share/classes/sun/management/ThreadImpl.java



hg: jdk7/tl/jdk: 6734813: Provide a way to construct an ObjectName without checked exceptions; ...

2008-09-10 Thread eamonn . mcmanus
Changeset: 2b44dd8ed72d
Author:emcmanus
Date:  2008-09-10 13:36 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/2b44dd8ed72d

6734813: Provide a way to construct an ObjectName without checked exceptions
6746649: ObjectName constructors and methods declare unchecked exceptions in 
throws clauses
Reviewed-by: dfuchs

! src/share/classes/com/sun/jmx/interceptor/DefaultMBeanServerInterceptor.java
! src/share/classes/com/sun/jmx/mbeanserver/Repository.java
! src/share/classes/com/sun/jmx/mbeanserver/Util.java
! src/share/classes/com/sun/jmx/namespace/DomainInterceptor.java
! src/share/classes/java/lang/management/PlatformComponent.java
! src/share/classes/java/util/logging/Logging.java
! src/share/classes/javax/management/MBeanServerDelegate.java
! src/share/classes/javax/management/ObjectName.java
! src/share/classes/javax/management/QueryNotificationFilter.java
! src/share/classes/javax/management/event/EventClientDelegateMBean.java
! src/share/classes/javax/management/namespace/MBeanServerSupport.java
! src/share/classes/sun/management/Util.java
+ test/javax/management/ObjectName/ValueOfTest.java



hg: jdk7/tl/jdk: 6746196: Some JMX classes do not compile with Eclipse compiler

2008-09-09 Thread eamonn . mcmanus
Changeset: 1643868af837
Author:emcmanus
Date:  2008-09-09 14:57 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/1643868af837

6746196: Some JMX classes do not compile with Eclipse compiler
Reviewed-by: dfuchs
Contributed-by: [EMAIL PROTECTED]

! src/share/classes/com/sun/jmx/mbeanserver/DefaultMXBeanMappingFactory.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java



hg: jdk7/tl/jdk: 6744132: Spurious failures from test/javax/management/MBeanInfo/NotificationInfoTest.java

2008-09-03 Thread eamonn . mcmanus
Changeset: 00ea8fc81867
Author:emcmanus
Date:  2008-09-03 14:31 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/00ea8fc81867

6744132: Spurious failures from 
test/javax/management/MBeanInfo/NotificationInfoTest.java
Reviewed-by: dfuchs

! test/javax/management/MBeanInfo/NotificationInfoTest.java



hg: jdk7/tl/jdk: 6405862: Allow CompositeType to have zero items; ...

2008-09-02 Thread eamonn . mcmanus
Changeset: 1d1d66438d11
Author:emcmanus
Date:  2008-09-02 14:14 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/1d1d66438d11

6405862: Allow CompositeType to have zero items
6737133: Compilation failure of 
test/javax/management/eventService/LeaseManagerDeadlockTest.java
6737140: Javadoc of some throw clauses of MBeanServer and MBeanServerConnection 
is garbled
6737143: createMBean of MBeanServer should acquire 2 extra throw clauses 
present in MBeanServerConnection
Reviewed-by: dfuchs

! src/share/classes/javax/management/MBeanServer.java
! src/share/classes/javax/management/MBeanServerConnection.java
! src/share/classes/javax/management/openmbean/CompositeType.java
! test/javax/management/eventService/LeaseManagerDeadlockTest.java



hg: jdk7/tl/jdk: 6731410: JMXServiceURL cannot use @ConstructorProperties for compatibility reasons

2008-09-01 Thread eamonn . mcmanus
Changeset: 0a427d0e70a7
Author:emcmanus
Date:  2008-09-01 17:11 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/0a427d0e70a7

6731410: JMXServiceURL cannot use @ConstructorProperties for compatibility 
reasons
Reviewed-by: dfuchs

! src/share/classes/javax/management/MXBean.java
! src/share/classes/javax/management/remote/JMXServiceURL.java
! test/javax/management/mxbean/JMXServiceURLTest.java



hg: jdk7/tl/jdk: 5041784: (reflect) generic signature methods needlessly return generic arrays

2008-08-27 Thread eamonn . mcmanus
Changeset: 7afa7314d883
Author:emcmanus
Date:  2008-08-27 11:03 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/7afa7314d883

5041784: (reflect) generic signature methods needlessly return generic arrays
Reviewed-by: darcy

! src/share/classes/sun/reflect/generics/factory/CoreReflectionFactory.java
+ test/java/lang/reflect/Generics/TestPlainArrayNotGeneric.java



hg: jdk7/tl/jdk: 6610174: Improve CompositeDataSupport.toString when it includes arrays

2008-08-08 Thread eamonn . mcmanus
Changeset: 343d63bb2609
Author:emcmanus
Date:  2008-08-08 18:36 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/343d63bb2609

6610174: Improve CompositeDataSupport.toString when it includes arrays
Reviewed-by: dfuchs

! src/share/classes/javax/management/openmbean/CompositeDataSupport.java
+ test/javax/management/openmbean/CompositeDataStringTest.java



hg: jdk7/tl/jdk: 2 new changesets

2008-08-08 Thread eamonn . mcmanus
Changeset: e9de9ae8c214
Author:emcmanus
Date:  2008-08-08 15:08 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/e9de9ae8c214

6334663: TabularDataSupport should be able to return values in the insertion 
order
Reviewed-by: dfuchs

! src/share/classes/com/sun/jmx/mbeanserver/DefaultMXBeanMappingFactory.java
! src/share/classes/javax/management/openmbean/TabularDataSupport.java
+ test/javax/management/openmbean/TabularDataOrderTest.java

Changeset: 4fac95ca002a
Author:emcmanus
Date:  2008-08-08 15:10 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/4fac95ca002a

Merge




hg: jdk7/tl/jdk: 6717257: MBeanServer doesn't describe RuntimeException for methods inherited from MBeanServerConnection

2008-08-07 Thread eamonn . mcmanus
Changeset: afe18ad188a1
Author:emcmanus
Date:  2008-08-07 16:25 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/afe18ad188a1

6717257: MBeanServer doesn't describe RuntimeException for methods inherited 
from MBeanServerConnection
Reviewed-by: dfuchs

! src/share/classes/javax/management/MBeanServer.java



hg: jdk7/tl/jdk: 6734273: Minor updates to documentation of Custom MXBean Mappings

2008-08-06 Thread eamonn . mcmanus
Changeset: 13b8426bb0cd
Author:emcmanus
Date:  2008-08-06 18:28 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/13b8426bb0cd

6734273: Minor updates to documentation of Custom MXBean Mappings
Reviewed-by: dfuchs

! src/share/classes/javax/management/MXBean.java
! src/share/classes/javax/management/openmbean/MXBeanMapping.java
! src/share/classes/javax/management/openmbean/MXBeanMappingFactory.java



hg: jdk7/tl/jdk: 6733589: Intermittent failure of test/javax/management/eventService/SharingThreadTest.java

2008-08-05 Thread eamonn . mcmanus
Changeset: 00c40e393a75
Author:emcmanus
Date:  2008-08-05 10:49 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/00c40e393a75

6733589: Intermittent failure of 
test/javax/management/eventService/SharingThreadTest.java
Reviewed-by: sjiang

! test/javax/management/eventService/SharingThreadTest.java



hg: jdk7/tl/jdk: 5108776: Add reliable event handling to the JMX API; ...

2008-07-31 Thread eamonn . mcmanus
Changeset: 8f52c4d1d934
Author:sjiang
Date:  2008-07-31 15:31 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/8f52c4d1d934

5108776: Add reliable event handling to the JMX API
6218920: API bug - impossible to delete last MBeanServerForwarder on a connector
Reviewed-by: emcmanus

+ src/share/classes/com/sun/jmx/event/DaemonThreadFactory.java
+ src/share/classes/com/sun/jmx/event/EventBuffer.java
+ src/share/classes/com/sun/jmx/event/EventClientFactory.java
+ src/share/classes/com/sun/jmx/event/EventConnection.java
+ src/share/classes/com/sun/jmx/event/EventParams.java
+ src/share/classes/com/sun/jmx/event/LeaseManager.java
+ src/share/classes/com/sun/jmx/event/LeaseRenewer.java
+ src/share/classes/com/sun/jmx/event/ReceiverBuffer.java
+ src/share/classes/com/sun/jmx/event/RepeatedSingletonJob.java
+ src/share/classes/com/sun/jmx/interceptor/MBeanServerSupport.java
+ src/share/classes/com/sun/jmx/interceptor/SingleMBeanForwarder.java
! src/share/classes/com/sun/jmx/interceptor/package.html
! src/share/classes/com/sun/jmx/mbeanserver/MBeanInjector.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanSupport.java
+ src/share/classes/com/sun/jmx/mbeanserver/PerThreadGroupPool.java
! src/share/classes/com/sun/jmx/mbeanserver/Util.java
! src/share/classes/com/sun/jmx/remote/internal/ClientNotifForwarder.java
! src/share/classes/com/sun/jmx/remote/internal/ProxyInputStream.java
! src/share/classes/com/sun/jmx/remote/internal/ProxyRef.java
! src/share/classes/com/sun/jmx/remote/internal/ServerNotifForwarder.java
! src/share/classes/com/sun/jmx/remote/security/FileLoginModule.java
! src/share/classes/com/sun/jmx/remote/util/EnvHelp.java
+ src/share/classes/com/sun/jmx/remote/util/EventClientConnection.java
! src/share/classes/com/sun/jmx/snmp/tasks/ThreadService.java
! src/share/classes/javax/management/ImmutableDescriptor.java
! src/share/classes/javax/management/MBeanServer.java
! src/share/classes/javax/management/MBeanServerConnection.java
! src/share/classes/javax/management/MXBean.java
! src/share/classes/javax/management/QueryParser.java
! src/share/classes/javax/management/StringValueExp.java
+ src/share/classes/javax/management/event/EventClient.java
+ src/share/classes/javax/management/event/EventClientDelegate.java
+ src/share/classes/javax/management/event/EventClientDelegateMBean.java
+ src/share/classes/javax/management/event/EventClientNotFoundException.java
+ src/share/classes/javax/management/event/EventConsumer.java
+ src/share/classes/javax/management/event/EventForwarder.java
+ src/share/classes/javax/management/event/EventReceiver.java
+ src/share/classes/javax/management/event/EventRelay.java
+ src/share/classes/javax/management/event/EventSubscriber.java
+ src/share/classes/javax/management/event/FetchingEventForwarder.java
+ src/share/classes/javax/management/event/FetchingEventRelay.java
+ src/share/classes/javax/management/event/ListenerInfo.java
+ src/share/classes/javax/management/event/NotificationManager.java
+ src/share/classes/javax/management/event/RMIPushEventForwarder.java
+ src/share/classes/javax/management/event/RMIPushEventRelay.java
+ src/share/classes/javax/management/event/RMIPushServer.java
+ src/share/classes/javax/management/event/package-info.java
! src/share/classes/javax/management/loading/MLet.java
! src/share/classes/javax/management/modelmbean/ModelMBeanInfoSupport.java
! src/share/classes/javax/management/modelmbean/RequiredModelMBean.java
! src/share/classes/javax/management/relation/RelationService.java
+ src/share/classes/javax/management/remote/IdentityMBeanServerForwarder.java
! src/share/classes/javax/management/remote/JMXConnector.java
! src/share/classes/javax/management/remote/JMXConnectorServer.java
! src/share/classes/javax/management/remote/JMXConnectorServerFactory.java
! src/share/classes/javax/management/remote/JMXConnectorServerMBean.java
! src/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
! src/share/classes/javax/management/remote/rmi/RMIConnector.java
! src/share/classes/javax/management/remote/rmi/RMIConnectorServer.java
+ test/javax/management/MBeanServer/DynamicWrapperMBeanTest.java
+ test/javax/management/MBeanServer/OldMBeanServerTest.java
+ test/javax/management/eventService/AddRemoveListenerTest.java
+ test/javax/management/eventService/CustomForwarderTest.java
+ test/javax/management/eventService/EventClientExecutorTest.java
+ test/javax/management/eventService/EventDelegateSecurityTest.java
+ test/javax/management/eventService/EventManagerTest.java
+ test/javax/management/eventService/FetchingTest.java
+ test/javax/management/eventService/LeaseManagerDeadlockTest.java
+ test/javax/management/eventService/LeaseTest.java
+ test/javax/management/eventService/ListenerTest.java
+ test/javax/management/eventService/MyFetchingEventForwarder.java
+ test/javax/management/eventService/NotSerializableNotifTest.java
+ test/javax/management/eventService/PublishTest.java
+ test/javax/management/eventService/Reco

hg: jdk7/tl/jdk: 6323980: Annotations to simplify MBean development

2008-07-09 Thread eamonn . mcmanus
Changeset: afa8b71365aa
Author:emcmanus
Date:  2008-07-09 10:36 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/afa8b71365aa

6323980: Annotations to simplify MBean development
Reviewed-by: jfdenise, dfuchs

! src/share/classes/com/sun/jmx/interceptor/DefaultMBeanServerInterceptor.java
! src/share/classes/com/sun/jmx/mbeanserver/DynamicMBean2.java
! src/share/classes/com/sun/jmx/mbeanserver/Introspector.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanAnalyzer.java
+ src/share/classes/com/sun/jmx/mbeanserver/MBeanInjector.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanSupport.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanIntrospector.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanSupport.java
+ src/share/classes/com/sun/jmx/mbeanserver/NotifySupport.java
! src/share/classes/com/sun/jmx/mbeanserver/Repository.java
! src/share/classes/com/sun/jmx/mbeanserver/StandardMBeanIntrospector.java
! src/share/classes/com/sun/jmx/mbeanserver/StandardMBeanSupport.java
! src/share/classes/com/sun/jmx/mbeanserver/Util.java
! src/share/classes/javax/management/BinaryRelQueryExp.java
+ src/share/classes/javax/management/Description.java
! src/share/classes/javax/management/Descriptor.java
+ src/share/classes/javax/management/DescriptorFields.java
! src/share/classes/javax/management/DescriptorKey.java
+ src/share/classes/javax/management/DynamicWrapperMBean.java
+ src/share/classes/javax/management/Impact.java
! src/share/classes/javax/management/JMX.java
+ src/share/classes/javax/management/MBean.java
! src/share/classes/javax/management/MBeanOperationInfo.java
! src/share/classes/javax/management/MBeanRegistration.java
! src/share/classes/javax/management/MBeanServer.java
! src/share/classes/javax/management/MBeanServerConnection.java
! src/share/classes/javax/management/MXBean.java
+ src/share/classes/javax/management/ManagedAttribute.java
+ src/share/classes/javax/management/ManagedOperation.java
! src/share/classes/javax/management/NotQueryExp.java
! src/share/classes/javax/management/NotificationBroadcasterSupport.java
+ src/share/classes/javax/management/NotificationInfo.java
+ src/share/classes/javax/management/NotificationInfos.java
+ src/share/classes/javax/management/SendNotification.java
! src/share/classes/javax/management/StandardEmitterMBean.java
! src/share/classes/javax/management/StandardMBean.java
! src/share/classes/javax/management/modelmbean/RequiredModelMBean.java
! src/share/classes/javax/management/monitor/package.html
! src/share/classes/javax/management/package.html
+ test/javax/management/Introspector/AnnotatedMBeanTest.java
+ test/javax/management/Introspector/AnnotatedNotificationInfoTest.java
+ test/javax/management/Introspector/MBeanDescriptionTest.java
+ test/javax/management/Introspector/ParameterNameTest.java
+ test/javax/management/Introspector/ResourceInjectionTest.java
+ test/javax/management/Introspector/annot/Name.java



hg: jdk7/tl/jdk: 6601652: MXBeans: no IllegalArgumentException in the ex. chain for SortedSet/Map with a non-null comparator()

2008-07-04 Thread eamonn . mcmanus
Changeset: a031e88c72ec
Author:emcmanus
Date:  2008-07-04 18:55 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/a031e88c72ec

6601652: MXBeans: no IllegalArgumentException in the ex. chain for 
SortedSet/Map with a non-null comparator()
Summary: Forward-port this bug fix from JDK 6
Reviewed-by: dfuchs, lmalvent

! src/share/classes/com/sun/jmx/mbeanserver/DefaultMXBeanMappingFactory.java
+ test/javax/management/mxbean/ComparatorExceptionTest.java
! test/javax/management/mxbean/MXBeanTest.java
+ test/javax/management/mxbean/SameObjectTwoNamesTest.java



hg: jdk7/tl/jdk: 2 new changesets

2008-06-05 Thread eamonn . mcmanus
Changeset: b715e82ef7e1
Author:emcmanus
Date:  2008-06-05 13:40 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/b715e82ef7e1

6701498: Change JMX query language to use * and ? as wildcards rather than % 
and _
Reviewed-by: dfuchs

! src/share/classes/javax/management/MatchQueryExp.java
! src/share/classes/javax/management/ObjectName.java
! src/share/classes/javax/management/Query.java
! src/share/classes/javax/management/QueryNotificationFilter.java
! src/share/classes/javax/management/QueryParser.java
! test/javax/management/query/QueryExpStringTest.java
! test/javax/management/query/QueryParseTest.java

Changeset: af0a68f46dde
Author:emcmanus
Date:  2008-06-05 13:42 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/af0a68f46dde

6562936: Support custom type mappings in MXBeans
Reviewed-by: dfuchs

! src/share/classes/com/sun/jmx/mbeanserver/ConvertingMethod.java
+ src/share/classes/com/sun/jmx/mbeanserver/DefaultMXBeanMappingFactory.java
! src/share/classes/com/sun/jmx/mbeanserver/Introspector.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanAnalyzer.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanSupport.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanIntrospector.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanLookup.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanProxy.java
! src/share/classes/com/sun/jmx/mbeanserver/MXBeanSupport.java
! src/share/classes/com/sun/jmx/mbeanserver/NotificationMBeanSupport.java
- src/share/classes/com/sun/jmx/mbeanserver/OpenConverter.java
! src/share/classes/com/sun/jmx/mbeanserver/PerInterface.java
! src/share/classes/com/sun/jmx/mbeanserver/StandardMBeanSupport.java
! src/share/classes/javax/management/JMX.java
! src/share/classes/javax/management/MBeanServerInvocationHandler.java
! src/share/classes/javax/management/MXBean.java
! src/share/classes/javax/management/StandardMBean.java
- src/share/classes/javax/management/ToQueryString.java
! 
src/share/classes/javax/management/openmbean/CompositeDataInvocationHandler.java
! src/share/classes/javax/management/openmbean/CompositeType.java
+ src/share/classes/javax/management/openmbean/MXBeanMapping.java
+ src/share/classes/javax/management/openmbean/MXBeanMappingClass.java
+ src/share/classes/javax/management/openmbean/MXBeanMappingFactory.java
+ src/share/classes/javax/management/openmbean/MXBeanMappingFactoryClass.java
! src/share/classes/javax/management/openmbean/OpenType.java
+ test/javax/management/mxbean/CustomTypeTest.java
+ test/javax/management/mxbean/customtypes/CustomLongMXBean.java
+ test/javax/management/mxbean/customtypes/CustomMXBean.java
+ test/javax/management/mxbean/customtypes/IntegerIsLongFactory.java
+ test/javax/management/mxbean/customtypes/IntegerIsStringFactory.java
+ test/javax/management/mxbean/customtypes/package-info.java



hg: jdk7/tl/jdk: 6703552: Missing files from changeset for 6701459

2008-05-16 Thread eamonn . mcmanus
Changeset: 1483094a7c17
Author:emcmanus
Date:  2008-05-16 11:34 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/1483094a7c17

6703552: Missing files from changeset for 6701459
Summary: Previous push missed a small number of files.
Reviewed-by: dfuchs

! 
src/share/classes/javax/management/openmbean/OpenMBeanOperationInfoSupport.java
! src/share/classes/javax/management/relation/RelationService.java
! src/share/classes/javax/management/timer/Timer.java
+ test/javax/management/relation/RelationNotificationSeqNoTest.java



Re: hg: jdk7/tl/jdk: 6701459: Synchronization bug pattern found in javax.management.relation.RelationService

2008-05-15 Thread Eamonn McManus




Rob,
You write:



  In the change sets some variable postfixes were renamed
Nbr  -> No

Example:
Long seqNbr = getNotificationSequenceNumber();
+ Long seqNo = atomicSeqNo.incrementAndGet();


The abbreviation No could also could mean:
 No as in 'Yes'/'No' ,
 No as in 'Notation',
 besides 'Number'.
Generally avoiding unclear naming seems desirable.
Comments ?


First of all, I'm very happy to see someone paying this much
attention to my changes!

You are probably right: I think seqNum is a better abbreviation than
seqNo. On the other hand, seqNo is the abbreviation used in the few
other places in the JMX source where there is a local variable or
parameter that is a sequence number. Google shows 313,000 hits for
"seqnum" and 5,480,000 for "seqno" so we might
conclude that both are fairly standard abbreviations with "seqno" being
commoner. "seqnbr" comes in a poor third at 10,100 so I think I was
justified in renaming it in any case. (Google Code Search shows about
equal numbers for seqno and seqnum.)

We could do a sweep over the JMX sources to rename the various seqNo
(and paramNo) variables to seqNum (paramNum). But there's a tension
here between slightly improved readability and the noise that would be
generated by such a change. However, I'll bear your suggestion in mind
for future changes, and this is also a reminder for me that I should
have sent the diffs to the public [EMAIL PROTECTED] list for
review.

Thanks,
-- 
Éamonn McManus   JMX Spec Lead   http://weblogs.java.net/blog/emcmanus/





hg: jdk7/tl/jdk: 6701459: Synchronization bug pattern found in javax.management.relation.RelationService

2008-05-14 Thread eamonn . mcmanus
Changeset: 94ded5c8cfba
Author:emcmanus
Date:  2008-05-14 18:38 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/94ded5c8cfba

6701459: Synchronization bug pattern found in 
javax.management.relation.RelationService
Summary: Fixed this and many other problems found by FindBugs.
Reviewed-by: dfuchs

! src/share/classes/com/sun/jmx/interceptor/DefaultMBeanServerInterceptor.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanInstantiator.java
! src/share/classes/com/sun/jmx/mbeanserver/Repository.java
! src/share/classes/com/sun/jmx/remote/internal/ClientNotifForwarder.java
! src/share/classes/com/sun/jmx/remote/internal/ServerNotifForwarder.java
! src/share/classes/com/sun/jmx/remote/security/FileLoginModule.java
! src/share/classes/com/sun/jmx/remote/security/JMXPluggableAuthenticator.java
! 
src/share/classes/com/sun/jmx/remote/security/MBeanServerFileAccessController.java
! src/share/classes/javax/management/NumericValueExp.java
! src/share/classes/javax/management/ObjectName.java
! src/share/classes/javax/management/StandardMBean.java
! src/share/classes/javax/management/loading/MLet.java
! src/share/classes/javax/management/loading/MLetParser.java
! src/share/classes/javax/management/modelmbean/DescriptorSupport.java
! src/share/classes/javax/management/modelmbean/ModelMBeanAttributeInfo.java
! src/share/classes/javax/management/modelmbean/ModelMBeanConstructorInfo.java
! src/share/classes/javax/management/modelmbean/ModelMBeanInfoSupport.java
! src/share/classes/javax/management/modelmbean/ModelMBeanNotificationInfo.java
! src/share/classes/javax/management/modelmbean/ModelMBeanOperationInfo.java
! src/share/classes/javax/management/modelmbean/RequiredModelMBean.java
! src/share/classes/javax/management/monitor/CounterMonitor.java
! src/share/classes/javax/management/monitor/GaugeMonitor.java
! src/share/classes/javax/management/monitor/Monitor.java
! src/share/classes/javax/management/openmbean/ArrayType.java
! src/share/classes/javax/management/openmbean/CompositeType.java
! 
src/share/classes/javax/management/openmbean/OpenMBeanAttributeInfoSupport.java
! 
src/share/classes/javax/management/openmbean/OpenMBeanConstructorInfoSupport.java
! src/share/classes/javax/management/openmbean/OpenMBeanInfoSupport.java
! src/share/classes/javax/management/openmbean/SimpleType.java
! src/share/classes/javax/management/openmbean/TabularType.java
! src/share/classes/javax/management/relation/RelationNotification.java
! src/share/classes/javax/management/relation/RelationService.java
! src/share/classes/javax/management/relation/RelationSupport.java
! src/share/classes/javax/management/remote/JMXConnectorFactory.java
! src/share/classes/javax/management/remote/JMXConnectorServerFactory.java
! src/share/classes/javax/management/remote/JMXServiceURL.java
! src/share/classes/javax/management/remote/rmi/RMIConnector.java
! src/share/classes/javax/management/remote/rmi/RMIConnectorServer.java
! src/share/classes/javax/management/timer/Timer.java



hg: jdk7/tl/jdk: 6692027: Custom subclasses of QueryEval don't serialize

2008-04-22 Thread eamonn . mcmanus
Changeset: 92ea0ac77d2f
Author:emcmanus
Date:  2008-04-22 18:58 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/92ea0ac77d2f

6692027: Custom subclasses of QueryEval don't serialize
Summary: Remove non-public superclass of QueryEval
Reviewed-by: dfuchs

! src/share/classes/javax/management/AndQueryExp.java
! src/share/classes/javax/management/BetweenQueryExp.java
! src/share/classes/javax/management/BinaryRelQueryExp.java
! src/share/classes/javax/management/NotQueryExp.java
! src/share/classes/javax/management/ObjectName.java
! src/share/classes/javax/management/OrQueryExp.java
! src/share/classes/javax/management/Query.java
! src/share/classes/javax/management/QueryEval.java
+ test/javax/management/query/CustomQueryTest.java



hg: jdk7/tl/jdk: 6610917: Define a generic NotificationFilter

2008-04-01 Thread eamonn . mcmanus
Changeset: 2965459a8ee7
Author:emcmanus
Date:  2008-04-01 14:45 +0200
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/2965459a8ee7

6610917: Define a generic NotificationFilter
Summary: Adds javax.management.QueryNotificationFilter
Reviewed-by: dfuchs

! src/share/classes/com/sun/jmx/interceptor/DefaultMBeanServerInterceptor.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanAnalyzer.java
! src/share/classes/com/sun/jmx/mbeanserver/MBeanIntrospector.java
+ src/share/classes/com/sun/jmx/mbeanserver/NotificationMBeanSupport.java
! src/share/classes/com/sun/jmx/mbeanserver/OpenConverter.java
! src/share/classes/com/sun/jmx/mbeanserver/Repository.java
! src/share/classes/com/sun/jmx/mbeanserver/Util.java
! src/share/classes/javax/management/ObjectName.java
+ src/share/classes/javax/management/QueryNotificationFilter.java
+ test/javax/management/query/QueryNotifFilterTest.java



hg: jdk7/tl/jdk: 6643627: JMX source code includes incorrect Java code

2008-03-21 Thread eamonn . mcmanus
Changeset: 01f7eeea81f1
Author:emcmanus
Date:  2008-03-21 18:07 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/01f7eeea81f1

6643627: JMX source code includes incorrect Java code
Summary: javac compiler bug accepts incorrect code; JMX code inadvertently has 
such code
Reviewed-by: dfuchs

! src/share/classes/com/sun/jmx/mbeanserver/OpenConverter.java
! src/share/classes/java/beans/MetaData.java



hg: jdk7/tl/jdk: 6649542: Document explicitly in registerMBean etc that MBeanServerNotification is emitted

2008-03-21 Thread eamonn . mcmanus
Changeset: 9a97ca4eb8b7
Author:emcmanus
Date:  2008-03-21 09:49 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/9a97ca4eb8b7

6649542: Document explicitly in registerMBean etc that MBeanServerNotification 
is emitted
Summary: Make spec more readable by adding cross-references.  Suggested by 
Andrew Haley.
Reviewed-by: dfuchs

! src/share/classes/javax/management/MBeanServer.java



hg: jdk7/tl/jdk: 6675768: NoSuchElementException thrown in RequiredModelMBean when tracing enabled

2008-03-19 Thread eamonn . mcmanus
Changeset: 0d4923ce2707
Author:emcmanus
Date:  2008-03-19 15:17 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/0d4923ce2707

6675768: NoSuchElementException thrown in RequiredModelMBean when tracing 
enabled
Summary: Rewrite logging in 
RequiredModelMBean.addAttributeChangeNotificationListener
Reviewed-by: dfuchs

! src/share/classes/javax/management/modelmbean/RequiredModelMBean.java
+ test/javax/management/modelmbean/LoggingExceptionTest.java



hg: jdk7/tl/jdk: 6670375: Missing unit test for 6607114 (Make JMXServiceURL reconstructible)

2008-03-03 Thread eamonn . mcmanus
Changeset: 302cbd0a8ace
Author:emcmanus
Date:  2008-03-03 15:44 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/302cbd0a8ace

6670375: Missing unit test for 6607114 (Make JMXServiceURL reconstructible)
Summary: Current setup doesn't allow two pushes with same CR number
Reviewed-by: dfuchs

! src/share/classes/javax/management/remote/JMXServiceURL.java
+ test/javax/management/mxbean/JMXServiceURLTest.java



hg: jdk7/tl/jdk: 2 new changesets

2008-03-03 Thread eamonn . mcmanus
Changeset: 10256bd4afcd
Author:emcmanus
Date:  2008-03-03 15:28 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/10256bd4afcd

6607114: Make JMXServiceURL reconstructible in MXBeans
Summary: Add @ConstructorProperties tag to JMXServiceURL
Reviewed-by: dfuchs

! src/share/classes/javax/management/remote/JMXServiceURL.java

Changeset: 613f2c906b9d
Author:emcmanus
Date:  2008-03-03 15:29 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/613f2c906b9d

Merge




hg: jdk7/tl/jdk: 6602310: Extensions to Query API for JMX 2.0; ...

2008-03-03 Thread eamonn . mcmanus
Changeset: 41d9c673dd9d
Author:emcmanus
Date:  2008-03-03 10:32 +0100
URL:   http://hg.openjdk.java.net/jdk7/tl/jdk/rev/41d9c673dd9d

6602310: Extensions to Query API for JMX 2.0
6604768: IN queries require their arguments to be constants
Summary: New JMX query language and support for dotted attributes in queries.
Reviewed-by: dfuchs

! src/share/classes/com/sun/jmx/mbeanserver/Introspector.java
! src/share/classes/javax/management/AndQueryExp.java
! src/share/classes/javax/management/AttributeValueExp.java
! src/share/classes/javax/management/BetweenQueryExp.java
! src/share/classes/javax/management/BinaryOpValueExp.java
! src/share/classes/javax/management/BinaryRelQueryExp.java
! src/share/classes/javax/management/BooleanValueExp.java
! src/share/classes/javax/management/InQueryExp.java
! src/share/classes/javax/management/MatchQueryExp.java
! src/share/classes/javax/management/NotQueryExp.java
! src/share/classes/javax/management/NumericValueExp.java
! src/share/classes/javax/management/ObjectName.java
! src/share/classes/javax/management/OrQueryExp.java
! src/share/classes/javax/management/QualifiedAttributeValueExp.java
! src/share/classes/javax/management/Query.java
! src/share/classes/javax/management/QueryEval.java
! src/share/classes/javax/management/QueryExp.java
+ src/share/classes/javax/management/QueryParser.java
! src/share/classes/javax/management/StringValueExp.java
+ src/share/classes/javax/management/ToQueryString.java
! src/share/classes/javax/management/monitor/Monitor.java
+ test/javax/management/query/QueryDottedAttrTest.java
! test/javax/management/query/QueryExpStringTest.java
+ test/javax/management/query/QueryParseTest.java



Re: jmx-dev [Fwd: Fix compiler problem]

2007-12-19 Thread Eamonn McManus




Thanks for bringing this to our attention, Alan. Roman is right -
the code in question should not compile and we should change it as he
suggests so that it is correct.

(For people interested in the gory details, the method
 T getAnnotation(Class
annotationClass)
returns T only if it is called on a properly generic variable, such as
a Constructor or Constructor or
Constructor. If it is called on a plain Constructor with no
type parameter, then that is a "raw type", and the return type is
"erased" to Annotation. So something like
ConstructorProperties annotation = constructor.getAnnotation(ConstructorProperties.class)
should not compile if constructor is
declared as Constructor rather than Constructor or whatever.)

Éamonn McManus   JMX Spec Lead   http://weblogs.java.net/blog/emcmanus/



Alan Bateman wrote:

I don't know if the JMX  team or the JavaBeans maintainers are on the
core-libs-dev mailing list.
  
  
  
  
  

  

Subject:

Fix compiler problem
  
  

From: 
Roman Kennke <[EMAIL PROTECTED]>
  
  

Date: 
Tue, 18 Dec 2007 15:32:06 +0100
  
  

To: 
Core-Libs-Dev 
  

  
  

  

To: 
Core-Libs-Dev 
  

  
  
  When trying to compile OpenJDK with the Eclipse compiler, I noticed two
compiler errors related to generics. It turned out that the code there
is invalid and only javac (incorrectly) accepts it. See the following
bug reports for details:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=212147
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6400189

The attached changeset fixes the problem. Could this be included?

/Roman