Re: Why isn't Object.notify() a synchronized method?
On Sun, May 31, 2015 at 02:31:25PM +1000, David Holmes wrote:
As I recently fell into the trap of forgetting the synchronized block
around a single notifyAll(), I believe, the current situation is just
errorprone.
How is it errorprone? You forgot to acquire the lock and you got an
IllegalMonitorStateException when you did notifyAll. That's pointing
out your error.
The reason for not making wait/notify synchronized is *not* that it would be
unnecessary because you typically already hold the lock anyway. The reason is
(as David Holms pointed out earlier) that it would be *meaningless* to make
them synchronized.
When you say it's errorprone it sounds like you first had
notifyAll();
and then fixed the IllegalMonitorStateException by doing
synchronized (this) {
notifyAll();
}
(and then wrote your original email asking why notifyAll didn't do this on the
inside).
If this is the case you have not understood the intention of using synchronized
here. In a nutshell wait and notify is all about thread communication, and for
that to work correctly you need to synchronize your execution. Let me try to
explain why wrapping *just* notifyAll() in a synchronized block (or relying on
making notifyAll synchronized) is broken: Suppose you have a producer /
consumer thing going on. In produce() you have something like
enqueue(value);
synchronized (this) {
notifyAll(); // because consumer may be waiting for a value
}
and in consume() you have something like
synchronized (this) {
while (noValueIsAvailable())
wait();
}
value = retrieve();
Suppose now that ProducerThread enters produce(), enqueues a value and before
reaching notifyAll, ConsumerThread comes along, enters consume(), consumes the
value, processes the value, calls consume *again*, sees that noValueIsAvailable
and calls wait. ProducerThread now continues it's execution and calls
notifyAll(). ConsumerThread is woken up, but at this point no value is
available despite there was a call to notify! (In the best case, this doesn't
crash the program, in worst case, ProducerThread assumed that the value should
be processed after notifyAll, in which case you may run into a deadlock. If
there were more variables involved you could also have memory visibility issues
involved and accidentally break class invariants by doing this.)
I've written an answer to a similar question here:
Why must wait() always be in synchronized block
http://stackoverflow.com/a/2779674/276052
best regards,
Andreas
Re: Optional.orElseChain ?
On Fri, Apr 17, 2015 at 03:01:29PM -0700, Steven Schlansker wrote:
On Apr 17, 2015, at 2:37 PM, Remi Forax fo...@univ-mlv.fr wrote:
As you can see the code is not bad but the code of chain() could be
simplified
if there was a way on Optional to call a Supplier of Optional if an
Optional is empty.
Currently, orElse() takes a value, orElseGet takes a lambda that will
return a value
and there is no method that takes a lambda and return an Optional
(like flatMap but but with a supplier that will be called if the Optional
is empty).
If we add the method orElseChain(Supplier? extends OptionalT supplier)
perhaps with a better name ?, then the code of chain is better:
public default TypeProvider chain(TypeProvider provider) {
return name - loadType(name).orElseChain(() - provider.loadType(name));
}
Am i the only one to think that this method is missing ?
We actually ran into the exact same problem, and wrote the following helper
method:
public static X OptionalX unlessOpt(@Nonnull OptionalX first,
SupplierOptionalX second) {
return first.isPresent() ? first : second.get();
}
I don't think it's precisely the same as your solution, but it definitely
indicates a missing method.
There are similar discussion here:
http://stackoverflow.com/questions/2456/get-value-from-one-optional-or-another
and here:
http://stackoverflow.com/questions/28818506/java-8-optional-orelse-optional
-- Andreas
Re: 8050820: Please add java.util.Optional.stream() to convert OptionalT to StreamT
On Tue, Jan 20, 2015 at 06:59:52PM +, Paul Sandoz wrote:
Hi,
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8050820-Optional-stream/webrev/
This is one of those cases where i think adding a new method to Optional
carries enough weight.
It can be really awkward to use Optional with Stream.flatMap to map to a
stream of present values.
It's not at all obvious that one can do
op.map(Stream::of).orElse(Stream.empty())) so often one sees some contorted
code of there in the wild. Hopefully adding Optional.stream (and to the
primitive variants) will help reduce such contortions.
A CCC will be filed.
Thanks,
Paul.
Hi Paul,
My name is Andreas Lundblad, and I joined the langtools team a little more than
a year ago. I'm not a Reviewer or anything but I casually read through your
patch out of curiosity.
You don't seem to be a fan of the ?: operator. I would have written
return isPresent() ? Stream.of(value) : Stream.empty();
Any reason for if/else in this case? Also, I find it strange to include { ... }
only on the else-branch:
+if (!isPresent())
+return Stream.empty();
+else {
+return Stream.of(value);
+}
It's in all of the if-statements in the patch so perhaps it's intentional. I
think it looks a bit peculiar.
-- Andreas
Re: JDK 9 RFR of 8067669: Documentation for methods in Number incomplete regarding too large values.
On Fri, Jan 16, 2015 at 02:55:28PM -0800, Brian Burkhalter wrote: On Jan 8, 2015, at 7:13 AM, Andreas Lundblad andreas.lundb...@oracle.com wrote: Although I was the one who brought it up, I think the whole ComparatorNumber discussion is slightly off topic. I still think that explicitly mentioning rounding and truncation is a bit confusing, as even the JDK implementations resort to other methods. I don’t think we quite agreed on the resolution of this issue. I don’t have a strong opinion about it, but I do think it should be resolved either as Fixed or Not An Issue. One alternative which removes the seemingly contentious verbiage is here: http://cr.openjdk.java.net/~bpb/8067669/webrev.01/ Note that the change at line 40 should be made even if the other diffs are rejected. This patch is an improvement in my opinion since it does not indicate that any effort is made to round or truncate the number. The obvious place to look for further documentation would be in the implementing class. I am however still in favour of making this explicit by adding The specific semantics of the conversion is defined by the subclass in question. which is also in line with the class level documentation. -- Andreas
Re: JDK 9 RFR of 8067669: Documentation for methods in Number incomplete regarding too large values.
On the matter of writing a ComparatorNumber, the basic problem is the Number interface is not strong enough to allow you to write such functionality. First, the Number type basically only means convertible to a primitive. There are no strict requirements on consistency between the different conversion methods and there is not even a requirement that calling the same Number method on the same object returns the same value (since Numbers are not required to be immutable). Even just writing a semantically correct ComparatorNumber for Number subclasses in the JDK is nontrivial. Multiple Number subclasses (BigInteger and BigDecimal) have values outside of the set of values that can be represented with a primitive type. So a general ComparatorNumber is not really well-defined. Right. This is something I discovered in the process. (Feel free to look at how for I got [1].) Although I was the one who brought it up, I think the whole ComparatorNumber discussion is slightly off topic. I still think that explicitly mentioning rounding and truncation is a bit confusing, as even the JDK implementations resort to other methods. [1] http://stackoverflow.com/a/4192900/276052
Re: JDK 9 RFR of 8067669: Documentation for methods in Number incomplete regarding too large values.
On Mon, Jan 05, 2015 at 05:43:26PM -0800, Joseph D. Darcy wrote: Hello, Getting back to this issue in the new year, taking a closer look at the existing class-level documentation, I don't really see a compelling case for an edit this large. I for one would be content if the bug were closed as not an issue. HTH, -Joe I originally brought this up when I was trying to create a ComparatorNumber. In my use case I was fine with rounding and truncation, so I thougth I could safely use longValue. I was then surprised when I discovered that the method could basically return arbitrary long values. In retrospect I realize that one could interpret [...] which may involve rounding or truncation as may involve rounding or truncation or something else. Nevertheless I find the current formulation a bit confusing. Maybe it's just me. -- Andreas
Re: Documentation of methods in java.lang.Number
On Tue, Dec 16, 2014 at 03:18:49PM -0800, Brian Burkhalter wrote:
Andreas,
Doesn’t the class documentation of Number [1] provide sufficient clarity, to
wit:
The specific semantics of the conversion from the numeric value of a
particular Number implementation to a given primitive type is defined by the
Number implementation in question.”
and
“[…] conversions may lose information about the overall magnitude of a
numeric value, may lose precision, and may even return a result of a
different sign than the input. See the documentation of a given Number
implementation for conversion details.”
Agree. I see no way of improving the class level documentation.
That said, it might not however hurt slightly to modify the descriptions of
{float,double,int,long}Value() to read something like, for longValue() for
example:
Returns the value of the specified number as a {@code long}, which may
involve rounding, truncation, or some other narrowing conversion.
Right. As it stands now the class level description says the semantics of the
conversion is defined by the Number implementation in question, and then the
documentation of longValue narrows this down to rounding or truncation. By
adding or some other narrowing conversion we make room for the
BigDecimal/BigInteger implementations, but I would argue that it's still more
constrained than the class level description.
Another suggestion would be:
Returns the value of the specified number as a {@code long}. The specific
semantics of the conversion is defined by the subclass in question.
-- Andreas
Documentation of methods in java.lang.Number
I've noticed that the documentation of Number.longValue says: Returns the value of the specified number as a long, which may involve rounding or truncation. BigInteger and BigDecimal does not seem to honor this contract since they implement this method by masking out the lower bits of the number (and I've found no definition of rounding or truncation that encompass this behavuor). The documentation of these classes even state that [...] this conversion can lose information about the overall magnitude of the BigInteger value as well as return a result with the opposite sign. I suggest the documentation of Number.byteValue/shortValue/intValue/longValue should be updated to give room for this type of implementation. (I filed an issue on this and got a confirmation email with the following link: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7000825 Since then this page has gone missing. Where did the bug report go, and why didn't I get any notification about the removal?) -- Andreas Lundblad
Re: RFR 8065998: Avoid use of _ as a one-character identifier
On Mon, Dec 01, 2014 at 01:10:29PM +0100, Jan Lahoda wrote: Hi, In a preparation for JDK-8061549, I'd like to rename all uses of '_' as a one-character identifier in the jaxp and jdk repositories. All the uses I was able to find are in tests, and the identifier is used as a name of a catch parameter. The proposed new name is ignore, but if a different name would be more appropriate, I'll be happy to use it. To me ignore signals I don't care if this exception occurred. In tests, when an exception *should* occurr, I usually name the variable expected. Ignore is a bit shorter though, so in the end it's a matter of taste I guess. -- Andreas
Re: RFR: 8027470: AnnotationSupport uses == rather than .equals to compare Class objects
On Thu, Nov 14, 2013 at 07:21:38PM -0800, Joseph Darcy wrote: Hello, Catching up on email, the specification of java.lang.Class does not explicitly promise that its notion of equality must be identity for all time. Therefore, while not required for today's implementations, I would prefer that new code we write in the JDK use equals rather than == when comparing classes. Thank you for getting back on this matter. First of all, I agree with you that 'equals' is in a sense more future proof than plain '=='. Strictly speaking though, can't one say that todays docs implicitly specify that j.l.Class.equals *must* be identity-based (since it does not explicitly refine the spec of Object.equals)? If so, wouldn't it be a breaking change to add a j.l.Class.equals which refines this by specifying something else? (Side note, At the moment I neighter consider this review as accepted nor rejected, and assume that the patch won't make it into 8 regardless.) -- Andreas
RFR: 8006730: remove workaround tests when jtreg updated
Hi,
Please review the fix for JDK-8006730 below.
Description:
Several tests have been added in test/tools/doclint/{html,tidy,tool} making the
dummy-tests, AAA.java obsolete.
This patch removes the AAA.java tests.
Link to web review:
http://cr.openjdk.java.net/~alundblad/8006730
Link to bug reports:
http://bugs.openjdk.java.net/browse/JDK-8006730
-- Andreas Lundblad
RFR: 8027470: AnnotationSupport uses == rather than .equals to compare Class objects
Hi, Please review the fix for JDK-8027470 below. Description: AnnotationSupport compared Class-instances using '==' where it should be using '.equals'. Fixed in this patch. Link to web review: http://cr.openjdk.java.net/~alundblad/8027470 Link to bug reports: http://bugs.openjdk.java.net/browse/JDK-8027470 -- Andreas Lundblad
RFR: 8016725: TEST_BUG: java/lang/reflect/Method/DefaultMethodModeling.java failing intermittently
Hi, Please review the fix for JDK-8016725 below. Description: DefaultMethodModeling.java and Equals.java in jdk/test/java/lang/reflect/Method interfered with each other since both tests defines a class named 'A'. In this patch DefaultMethodModeling.java is moved to its own subdirectory. Link to web review: http://cr.openjdk.java.net/~alundblad/8016725/webrev.00 Link to bug report: https://bugs.openjdk.java.net/browse/JDK-8016725 -- Andreas Lundblad
Re: RFR: 8004912: Repeating annotations - getAnnotationsByType is not working as expected
On 10/22/2013 03:20 PM, Peter Levart wrote: I think the problem could be solved in two ways: - by explicitly scanning the inheritance chain for the 1st class that has the annotations of type T present directly or indirectly (by containment). - by canonicalizing the representation of the repeating annotations in the class file attributes. By this I mean that each repeating annotation is placed inside it's container at compile-time even if it is a single annotation of a particular type. This would mean that some features like specifying different @Targets or @Retentions for repeating annotation types and their containers would be prohibited and the specification would have to be changed. But I think this way the inheritance aspect of repeating annotations would be consistent even if not looking through containers (by using the old JDK7 API)... The canonicalization could be performed at runtime though, and I think there were such attempts already in the past. What happened to them? Regards, Peter Hi Peter, A new patch is available for review here: http://cr.openjdk.java.net/~alundblad/inherited-associated/ This includes a new test based on your code. The test passes after applying the patch. best regards, Andreas Lundblad
Re: RFR: 8004912: Repeating annotations - getAnnotationsByType is not working as expected
On Tue 22 Oct 2013 12:21:36 PM CEST, Joel Borggrén-Franck wrote:
Hi Andreas,
A few nits:
Class.java:
import java.util.Collection;
+import java.util.Collections;
import java.util.HashSet;
unused import.
Right. Thanks.
AnnotationSupport.java:
+/**
+ * Equivalent to calling {@code getDirectlyAndIndirectlyPresentAnnotations(
+ * annotations, annoClass, false)}.
+ */
I think it is equivalent to annotations, annoClass, true
Absolutely. Good catch.
Otherwise looks good. I can sponsor this fix.
cheers
/Joel
Thanks Joel.
-- Andreas
Re: RFR: 8004912: Repeating annotations - getAnnotationsByType is not working as expected
Hi, New revision up for review: http://aoeu.se/webrevs/8019420-and-8004912/webrev.01 The following has been addressed since webrev.00: - Order of directly / indirectly present annotations now respects the order of the keys in the given map of annotations. - A new test has been added to test the above behavior. best regards, Andreas - Original Message - From: andreas.lundb...@oracle.com To: core-libs-dev@openjdk.java.net Sent: Wednesday, October 16, 2013 4:00:08 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: RFR: 8004912: Repeating annotations - getAnnotationsByType is not working as expected Hi, Please review the fix for JDK-8004912 and JDK-8019420 below. Description: The behavior of Class.get[Declared]AnnotationsByType was wrong. These methods delegate to sun.reflect.annotation.AnnotationSupport which has been rewritten. NonInheritableContainee.java is added and contains the test referred to in JDK-8019420. RepeatedUnitTest.java have been updated to include the test cases in JDK-8004912. There are more tests available in tl/langtools/test/tools/javac/annotations/repeatingAnnotations/combo/ReflectionTest.java (NB. this file is in the langtools repo) Link to web review: http://cr.openjdk.java.net/~alundblad/8019420-and-8004912/ Link to bug reports: https://bugs.openjdk.java.net/browse/JDK-8004912 https://bugs.openjdk.java.net/browse/JDK-8019420 -- Andreas Lundblad
RFR: 8004912: Repeating annotations - getAnnotationsByType is not working as expected
Hi, Please review the fix for JDK-8004912 and JDK-8019420 below. Description: The behavior of Class.get[Declared]AnnotationsByType was wrong. These methods delegate to sun.reflect.annotation.AnnotationSupport which has been rewritten. NonInheritableContainee.java is added and contains the test referred to in JDK-8019420. RepeatedUnitTest.java have been updated to include the test cases in JDK-8004912. There are more tests available in tl/langtools/test/tools/javac/annotations/repeatingAnnotations/combo/ReflectionTest.java (NB. this file is in the langtools repo) Link to web review: http://cr.openjdk.java.net/~alundblad/8019420-and-8004912/ Link to bug reports: https://bugs.openjdk.java.net/browse/JDK-8004912 https://bugs.openjdk.java.net/browse/JDK-8019420 -- Andreas Lundblad
