Thanks Mandy!
On 13/09/16 15:56, Mandy Chung wrote:
The new webrev.05 looks good.
On Sep 13, 2016, at 5:45 AM, Daniel Fuchs wrote:
Hi Mandy,
Here is a new webrev with your feedback integrated.
Finally I managed to get rid of the InternalError in MethodHandle
by using:
PrivilegedA
The new webrev.05 looks good.
> On Sep 13, 2016, at 5:45 AM, Daniel Fuchs wrote:
>
> Hi Mandy,
>
> Here is a new webrev with your feedback integrated.
>
> Finally I managed to get rid of the InternalError in MethodHandle
> by using:
> PrivilegedAction pa = () ->
> cus
Hi Mandy,
Here is a new webrev with your feedback integrated.
Finally I managed to get rid of the InternalError in MethodHandle
by using:
PrivilegedAction pa = () ->
customLevel.getClass().getClassLoader();
instead of
PrivilegedAction pa =
> On Aug 29, 2016, at 6:10 AM, Daniel Fuchs wrote:
>
> Here is a new webrev that uses ClassLoaderValue as Peter suggested.
>
> http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.04/
>
Looks good in general.
553 // KnowLevelCLV is used to register custom level instances with
t
Hi Peter, Mandy,
On 20/08/16 21:17, Mandy Chung wrote:
It is a good observation. Pinning on a class loader is a good solution. On the
other hand, the Level API isn’t well designed for customization (so does
LogManager or Logger) that should be re-examined at some point in the future
(which i
It is a good observation. Pinning on a class loader is a good solution. On the
other hand, the Level API isn’t well designed for customization (so does
LogManager or Logger) that should be re-examined at some point in the future
(which is one cause of the current complicated implementation).
Hi Peter,
On 17/08/16 12:20, Peter Levart wrote:
Hi Daniel,
Thinking of this patch from the compatibility standpoint, aren't you
afraid that someone might be using the following idiom:
public class MyLevel extends java.util.logging.Level {
static {
new MyLevel("LEVEL1", 1);
On 08/17/2016 01:20 PM, Peter Levart wrote:
You could use a ClassLoaderValue for this purpose, like in the
following addition to your patch (see KnownLevel.add):
http://cr.openjdk.java.net/~plevart/jdk9-dev/Level.known/webrev.01/
Note that the above also contains the functional-style findLe
On 08/17/2016 01:16 PM, Daniel Fuchs wrote:
Now that you have various findByXXX methods return Optional, you
could make methods that use them more Optional-API-powered. For
example,
findLevel could be written as a single expression:
Right. But do you mind if I leave that for another day?
I'
Hi Daniel,
Thinking of this patch from the compatibility standpoint, aren't you
afraid that someone might be using the following idiom:
public class MyLevel extends java.util.logging.Level {
static {
new MyLevel("LEVEL1", 1);
new MyLevel("LEVEL2", 2);
...
}
Hi Peter,
Now that you have various findByXXX methods return Optional, you
could make methods that use them more Optional-API-powered. For example,
findLevel could be written as a single expression:
Right. But do you mind if I leave that for another day?
I'm not too keen on multi-line code sta
Hi Peter,
Rereading the comment and looking at the code I see
that the comment is actually right. There are two methods
with similar names:
Level.getLocalizedName() which can be overridden by subclasses
and Level.getLocalizedLevelName() which cannot.
By default Level.getLocalizedName() calls Le
Hi Mandy,
On 17/08/16 05:05, Mandy Chung wrote:
On Aug 16, 2016, at 5:42 AM, Daniel Fuchs wrote:
>
> Hi Mandy,
>
> I added an additional selector parameter to the find methods.
> This made it possible to return Optional instead of
> KnownLevel - and it does simply the parse() method.
>
> http:/
> On Aug 16, 2016, at 5:42 AM, Daniel Fuchs wrote:
>
> Hi Mandy,
>
> I added an additional selector parameter to the find methods.
> This made it possible to return Optional instead of
> KnownLevel - and it does simply the parse() method.
>
> http://cr.openjdk.java.net/~dfuchs/webrev_6543126/w
Hi Daniel,
Now that you have various findByXXX methods return Optional, you
could make methods that use them more Optional-API-powered. For example,
findLevel could be written as a single expression:
static Level findLevel(String name) {
return KnownLevel
.findByName(O
On 08/16/2016 03:14 PM, Daniel Fuchs wrote:
Hi Peter,
On 16/08/16 13:52, Peter Levart wrote:
Ah, I see. Right. Of course, my bad.
In that case, there is a possible race that could lead to exception
here:
476 // add new Level.
477 Level levelObject = new Level(nam
Hi Peter,
On 16/08/16 13:52, Peter Levart wrote:
Ah, I see. Right. Of course, my bad.
In that case, there is a possible race that could lead to exception here:
476 // add new Level.
477 Level levelObject = new Level(name, x);
478 return KnownLevel.findByV
On 16/08/16 13:59, Peter Levart wrote:
Hi Daniel,
Another place that is not clear to me is the following (in old code):
585 // Returns a KnownLevel with the given localized name matching
586 // by calling the Level.getLocalizedLevelName() method (i.e. found
587 // fro
Hi Daniel,
Another place that is not clear to me is the following (in old code):
585 // Returns a KnownLevel with the given localized name matching
586 // by calling the Level.getLocalizedLevelName() method (i.e. found
587 // from the resourceBundle associated with the
On 08/16/2016 02:30 PM, Daniel Fuchs wrote:
Hi Peter,
On 16/08/16 13:05, Peter Levart wrote:
Hi Daniel,
Passing Stream returning extractors to Stream::flatMap is one way
of doing it. The other would be to make findByXXX methods return
Optional and then use Optional::map method on the result
Hi Peter,
On 16/08/16 13:05, Peter Levart wrote:
Hi Daniel,
Passing Stream returning extractors to Stream::flatMap is one way
of doing it. The other would be to make findByXXX methods return
Optional and then use Optional::map method on the result to
extract the needed property, so instead of:
Hi Daniel,
Passing Stream returning extractors to Stream::flatMap is one way
of doing it. The other would be to make findByXXX methods return
Optional and then use Optional::map method on the result to
extract the needed property, so instead of:
Optional level = findByName(name, KnownLev
Hi Mandy,
I added an additional selector parameter to the find methods.
This made it possible to return Optional instead of
KnownLevel - and it does simply the parse() method.
http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.02
best regards,
-- daniel
On 11/08/16 20:12, Mandy Chung wr
> On Aug 11, 2016, at 2:29 AM, Daniel Fuchs wrote:
>
> On 10/08/16 17:21, Mandy Chung wrote:
>>> On Jul 29, 2016, at 4:54 AM, Daniel Fuchs wrote:
>>> >
>>> > http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.01/
>> This looks pretty good.
>>
>> Since KnownLevel is now a Reference, I sug
Hi Peter,
On 10/08/16 22:06, Peter Levart wrote:
static synchronized void add(Level l) {
purge();
KnownLevel o = new KnownLevel(l);
nameToLevels.computeIfAbsent(l.name, n -> new ArrayList<>()).add(o);
intToLevels.computeIfAbsent(l.value, v -> new ArrayList<>()).add(o);
}
I agre
On 10/08/16 17:21, Mandy Chung wrote:
On Jul 29, 2016, at 4:54 AM, Daniel Fuchs wrote:
>
> http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.01/
This looks pretty good.
Since KnownLevel is now a Reference, I suggest to change KnownLevel::findByName,
findByValue and findByLocalizedLevel
Hi Daniel,
This is not strictly necessary, but here are a couple of things you
might consider doing while changing this code:
Using new JDK 8 Map API, the following could be simplified:
580 static synchronized void add(Level l) {
581 purge();
582 // the mi
> On Jul 29, 2016, at 4:54 AM, Daniel Fuchs wrote:
>
> http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.01/
This looks pretty good.
Since KnownLevel is now a Reference, I suggest to change
KnownLevel::findByName, findByValue and findByLocalizedLevelName to return
Optional instead suc
On 29/07/16 12:54, Daniel Fuchs wrote:
Hi,
Here is the new webrev with Chris & James feedback
taken in.
http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.01/
This looks good to me Daniel.
-Chris.
best regards,
-- daniel
On 28/07/16 22:55, Daniel Fuchs wrote:
On 28/07/16 21:31, Ja
Hi,
Here is the new webrev with Chris & James feedback
taken in.
http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.01/
best regards,
-- daniel
On 28/07/16 22:55, Daniel Fuchs wrote:
On 28/07/16 21:31, James Perkins wrote:
I just happened to take a glance at this and noticed the remov
On 28/07/16 21:31, James Perkins wrote:
I just happened to take a glance at this and noticed the remove method
on the KnownLevels doesn't quite look right.
Using Optional.of(nameToLevels.get(mirroredLevel.name)).ifPresent((x) ->
x.remove(this)); will produce an NPE if the level is not present. If
I just happened to take a glance at this and noticed the remove method on
the KnownLevels doesn't quite look right.
Using Optional.of(nameToLevels.get(mirroredLevel.name)).ifPresent((x) ->
x.remove(this)); will produce an NPE if the level is not present. If the
intention is that the levels may not
Hi Chris,
On 27/07/16 11:17, Chris Hegarty wrote:
Hi Daniel,
On 25/07/16 19:10, Daniel Fuchs wrote:
Hi,
Please find below a fix for:
6543126: Level.known can leak memory
https://bugs.openjdk.java.net/browse/JDK-6543126
webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.00
Si
Hi Daniel,
On 25/07/16 19:10, Daniel Fuchs wrote:
Hi,
Please find below a fix for:
6543126: Level.known can leak memory
https://bugs.openjdk.java.net/browse/JDK-6543126
webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.00
Since mirroredLevel is a strong reference to the same
Hi,
Please find below a fix for:
6543126: Level.known can leak memory
https://bugs.openjdk.java.net/browse/JDK-6543126
webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.00
This is a fix for a long standing issue: Level maintains a list
of know levels which has strong references
35 matches
Mail list logo
