Re: Define JNIEXPORT as visibility default with GCC?

[David Holmes]

I've bcc'd jdk8-dev and re-added core-libs and hotspot-runtime as that 
is where the original thread was.


David

On 9/04/2013 11:49 AM, Coleen Phillimore wrote:


Hi Martin,

I'm sorry, I lost track of this and thought it was already pushed.  The
jni_md.h changes look good but I don't really understand why the awt
changes were made, or what they do.   Since the jdk doesn't usually push
using JPRT, I'm afraid to push this directly myself without a review
from someone from jdk8-...@openjdk.com.  I have cc'ed them.   I think
someone from the tools and libraries group should review and push this.

Thanks,
Coleen

On 4/8/2013 7:35 PM, Martin Buchholz wrote:

friendly ping.  I'd like to have an approve to push this (or have
someone jprt for me).


On Mon, Mar 11, 2013 at 4:57 PM, Martin Buchholz mailto:marti...@google.com>> wrote:

The latest version of my webrev is here:
http://cr.openjdk.java.net/~martin/webrevs/openjdk8/JNIEXPORT/

It includes this line:

Reviewed-by: coleenp, ddehaven, dcubed


Ok to push?



On Fri, Mar 8, 2013 at 10:31 AM, Coleen Phillmore
mailto:coleen.phillim...@oracle.com>> wrote:


The hotspot definitions of JNIEXPORT don't match in all the
files to the JDK definition.   I think a hotspot bug should be
filed to fix the jni_.h definitions which now none of
them match. After someone in core-libs checks this in, we'll
update the hotspot files to match the final version and retest
-fvisibility=hidden.

I don't remember why the JDK version wasn't fixed with the
original -fvisibility=hidden work.

Coleen


On 2/28/2013 3:56 PM, Daniel D. Daugherty wrote:

On 2/28/13 11:57 AM, David DeHaven wrote:

Has a bug been filed for this? -DrD-


As mentioned earlier in this thread...

Dan



On 2/19/13 5:21 PM, Daniel D. Daugherty wrote:

I couldn't find a 'jdk' repo relevant bug for this
issue so I filed:

8008509: 6588413 changed JNIEXPORT visibility for
GCC on HSX, jdk's
 jni_md.h needs similar change

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8008509
https://jbs.oracle.com/bugs/browse/JDK-8008509

Coleen did the original work on 6588413 so I added her
to the "interest
list" for the new bug. The need for an update to the
jdk repo's jni_md.h
file was raised during the code review for 6588413,
but that detail appears
to have been dropped.

Dan

Re: RFR: 8004518 & 8010122 : Default methods on Map

[David Holmes]

Hi Mike,

Looking only at Map itself for now.

On 9/04/2013 4:07 AM, Mike Duigou wrote:

Hello all;

This is a combined review for the new default methods on the java.util.Map 
interface being added for the JSR-335 lambda libraries. The reviews are being 
combined because they share a common unit test.

http://cr.openjdk.java.net/~mduigou/JDK-8010122/0/webrev/


General style issues:

- spaces after keyword ie "if (x == null)" not "if(x == null)"
- comparisons against constants/null put the constant/null on the right 
ie "if (x == null)" not "if (null == x)"


(where has our style guide gone? I can't find it on internal or external 
wikis :( )



8004518: Add in-place operations to Map
  forEach()
  replaceAll()


Both of those contain the boilerplate text:

  * The default implementation makes no guarantees about
  * synchronization or atomicity properties of this method. Any
  * class overriding this method must specify its concurrency
  * properties. In particular, all implementations of
  * subinterface {@link java.util.concurrent.ConcurrentMap}
  * must ensure that this operation is performed atomically.

but these methods are not, and can not be, atomic in ConcurrentMap

forEach and replaceAll are very similar in terms of taking and applying 
a "operation" to each entry, yet their descriptions use a completely 
different style. forEach describes thing in general terms while 
replaceAll talks about calling the functions' apply method with the 
current entry's key and value. I would suggest for replaceAll:


"Replaces each entry's value with the result of invoking the given 
function on that entry, in the order entries are returned by an entry 
set iterator, until all entries have been processed or the function 
throws an exception."


This is also makes "replace" the subject rather than "apply".

forEach doesn't declare the IllegalStateException that getKey and 
getValue can throw.


Some/many of the @throws text has obviously been copied from the 
Map.Entry methods eg:


 * @throws ClassCastException if the class of the specified value
 * prevents it from being stored in the backing map

but when put into Map itself they should not be referring to "the 
backing map" as there is no "backing map". Further we have inconsistent 
terminology being used, eg getOrDefault has:


 * @throws ClassCastException if the key is of an inappropriate type for
 * this map

and then there is a third variant in other methods:

  * @throws ClassCastException if the class of the specified key or value
  * prevents it from being stored in this map
  * (optional)

These should all have the same basic wording, differing only in key/value.


8010122: Add atomic operations to Map


Meaning "backport some operations from ConcurrentMap" - they aren't 
actually atomic in Map.



  getOrDefault()


No comment


  putIfAbsent()  *


The default implementation throws ConcurrentModificationException but 
this is not declared in the spec.



  remove(K, V)
  replace(K, V)
  replace(K, V, V)


No comments


  compute()  *
  merge()*
  computeIfAbsent()  *
  computeIfPresent() *


The following generally apply to this group of methods.

As Peter already stated the spec:

 * If the function returns {@code null}, the mapping is removed (or
 * remains absent if initially absent).

is somewhat unclear. The parenthesized part is not connected to the 
function returning null or otherwise; as the function won't be called.


I find the spec for these rather confusing from a concurrency 
perspective - this non-concurrent interface seems to be trying to say 
too much about how a concurrent interface should specify behaviour. Why 
does it need to say:


  * In concurrent contexts, the default implementation may retry
  * these steps when multiple threads attempt updates.

? Note computeIfAbsent does not say the same thing.

The @implSpec does not match the actual implementation. It looks to me 
like these implementations are trying to cater for concurrent situations 
- hence the loop. That's okay but then the implSpec should identify that 
fact.



980  * be of use when combining multiple mapped values for a key.  For
981  * example. to either create or append a {@code String msg} to a

Period after example should be a comma.

Cheers,
David


The * operations treat null values as being absent. (ie. the same as there 
being no mapping for the specified key).

The default implementations provided in Map are overridden in HashMap for 
performance purposes, in Hashtable for atomicity and performance purposes and 
in Collections for atomicity.

Mike

Re: RFR: 8004518 & 8010122 : Default methods on Map

[Ulf Zibis]

Hi Mike,

Comments for j.u.Map:

To my savour the variant belongs to the left hand side of a comparison, e.g.:
   if (v = get(key) != null)

Instead
 501 return (null != (v = get(key)))
 502 ? v
 503 : containsKey(key) ? null : defaultValue;
I would code
 501 return ((v = get(key) != null) || containsKey(key)) ? v : 
defaultValue;

Use consistent formatted code examples in javadoc. I like the style from lines 558 to 561, but e.g. 
from line 601 to 605:

- 2 spaces before 
- indentation should be 4
- line break before }

Why you use both, {@code...} and ... ?

For performance reasons, I think you should reverse the order of the if 
expressions here:
 673 if (!Objects.equals(get(key), value) || !containsKey(key))
... because otherwise map lookup too often becomes executed twice, via 
contains() + get().

Not negate the comparison term, e.g.:
1053 if (value == null)
1054 return null;
1055 if ((oldValue = putIfAbsent(key, value)) == null)
1056 return value;


-Ulf


Am 08.04.2013 20:07, schrieb Mike Duigou:

Hello all;

This is a combined review for the new default methods on the java.util.Map 
interface being added for the JSR-335 lambda libraries. The reviews are being 
combined because they share a common unit test.

http://cr.openjdk.java.net/~mduigou/JDK-8010122/0/webrev/

8004518: Add in-place operations to Map
  forEach()
  replaceAll()

8010122: Add atomic operations to Map
  getOrDefault()
  putIfAbsent()  *
  remove(K, V)
  replace(K, V)
  replace(K, V, V)
  compute()  *
  merge()*
  computeIfAbsent()  *
  computeIfPresent() *

The * operations treat null values as being absent. (ie. the same as there 
being no mapping for the specified key).

The default implementations provided in Map are overridden in HashMap for 
performance purposes, in Hashtable for atomicity and performance purposes and 
in Collections for atomicity.

Mike
  

hg: jdk8/tl/jdk: 6298888: Add toGenericString to j.l.Class and getTypeName to j.l.reflect.Type; ...

[joe . darcy]

Changeset: 3e5a18c3e599
Author:darcy
Date:  2013-04-08 17:06 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3e5a18c3e599

629: Add toGenericString to j.l.Class and getTypeName to j.l.reflect.Type
6992705: Include modifiers in Class.toGenericString()
Summary: Class.toGenericString and supporting changes; additional reviews by 
Peter Levart
Reviewed-by: alanb

! src/share/classes/java/lang/Class.java
! src/share/classes/java/lang/reflect/Constructor.java
! src/share/classes/java/lang/reflect/Executable.java
! src/share/classes/java/lang/reflect/Field.java
! src/share/classes/java/lang/reflect/Method.java
! src/share/classes/java/lang/reflect/Modifier.java
! src/share/classes/java/lang/reflect/Parameter.java
! src/share/classes/java/lang/reflect/Type.java
+ test/java/lang/Class/GenericStringTest.java

Re: Looking for a sponsor to review changes made to two unit tests under jdk/test

[David Holmes]

On 9/04/2013 9:41 AM, Mani Sarkar wrote:

Is there no better way of waiting rather than using the Thread.sleep(n)
method, I think you will agree that using sleep isn't the most elegant
way to do things.


No it isn't elegant but there is no explicit way to synchronize with the 
GC activity, so generally all you can do is add a sleep to allow 
"sufficient" time for the things to happen. For finalization you can do 
more because you can synchronize with the finalizer itself.


David


I did run the patch without sleep, and it executed perfectly well - just
curious about the use of Thread.sleep(n) in general, nothing specific to
this change itself.

When should/can it be used and when not?

Cheers,
mani

On Tue, Apr 9, 2013 at 12:28 AM, David Holmes mailto:david.hol...@oracle.com>> wrote:

On 8/04/2013 9:59 PM, Mani Sarkar wrote:

Thanks Alan, David for your feedback.

So effectively you are saying the Thread.sleep(10) is fine in
the test
and does not need to be re-written using any of the concurrency
library
methods.


As I wrote back in one of my earliest emails:

"that aside the latch is not needed. The fork() method starts a
thread and joins it. So when createNoise() returns we already know
for certain that the "noise has been created". What the sleep is
doing is giving the GC a chance to run. "

The sleep has nothing to do with synchronizing with the "noise"
thread. And synchronization with the "noise" thread is already
handled perfectly correctly.

David
-


--
*Twitter:* @theNeomatrix369
*Blog:* http://neomatrix369.wordpress.com
*JUG activity:* LJC Advocate (@adoptopenjdk & @adoptajsr programs)
*Meet-a-Project:* https://github.com/MutabilityDetector
*Devoxx UK 2013 was a grand success:*
http://www.devoxx.com/display/UK13/Home
*/Don't chase success, rather aim for "Excellence", and success will
come chasing after you!/*

Throwable.addSuppressed error conditions -- use the exception as the cause?

[Steven Schlansker]

Today I encountered "java.lang.IllegalArgumentException: Self-suppression not 
permitted" from Throwable.addSuppressed.

My first surprise is that the try-with-resources block can throw this 
exception; it is very confusing to have auto-generated code throw exceptions.  
But beyond that, it is impossible to figure out *which* exception caused the 
problem.  If you have multiple resources acquired in the try-with-resources, it 
could be any of them.

Would it be reasonable to change

public final synchronized void addSuppressed(Throwable exception) {
if (exception == this)
throw new IllegalArgumentException(SELF_SUPPRESSION_MESSAGE);

to

public final synchronized void addSuppressed(Throwable exception) {
if (exception == this)
throw new IllegalArgumentException(SELF_SUPPRESSION_MESSAGE, this);

so that when you get this exception it at least points to one of the throw 
sites that actually caused the problem?  Otherwise the only stack trace you 
have is in auto-generated code and you are left scratching your head, wondering 
where it came from.  I believe this would increase the debuggability of the 
try-with-resources construct and there's no immediately obvious downside.

I'm not the only one who was confused by this behavior:
http://stackoverflow.com/questions/12103126/what-on-earth-is-self-suppression-not-permitted-and-why-is-javac-generating-co

hg: jdk8/tl/jdk: 8010849: (str) Optimize StringBuilder.append(null)

[martinrb]

Changeset: 3db793b080d8
Author:martin
Date:  2013-04-08 16:37 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3db793b080d8

8010849: (str) Optimize StringBuilder.append(null)
Summary: Append 4 chars instead of the string "null"
Reviewed-by: mduigou, forax, jgish

! src/share/classes/java/lang/AbstractStringBuilder.java

Re: RFR: Optimize StringBuilder.append(null)

[Martin Buchholz]

On Thu, Mar 28, 2013 at 10:47 AM, Jim Gish  wrote:

> Thanks, Martin,
>
> We've all heard of code "smells" -- I hereby dub this a code "chuckle"  :-)
>
> The change looks good to me otherwise as well.
>

Committed - thanks for the reviews!

Re: RFR: Optimize StringBuilder.append(null)

[Martin Buchholz]

On Wed, Mar 27, 2013 at 11:32 PM, Remi Forax  wrote:

> On 03/28/2013 07:29 AM, Martin Buchholz wrote:
>
>   The declaration saves 50 bytes of bytecode.
>>
>
> I don't understand ?
>

Copying a field into a method local results in fewer bytes of bytecode,
which may tickle hotspot into inlining the method for you.  Although it's
hard to demonstrate this.

Re: Looking for a sponsor to review changes made to two unit tests under jdk/test

[Mani Sarkar]

Is there no better way of waiting rather than using the Thread.sleep(n)
method, I think you will agree that using sleep isn't the most elegant way
to do things.

I did run the patch without sleep, and it executed perfectly well - just
curious about the use of Thread.sleep(n) in general, nothing specific to
this change itself.

When should/can it be used and when not?

Cheers,
mani

On Tue, Apr 9, 2013 at 12:28 AM, David Holmes wrote:

> On 8/04/2013 9:59 PM, Mani Sarkar wrote:
>
>> Thanks Alan, David for your feedback.
>>
>> So effectively you are saying the Thread.sleep(10) is fine in the test
>> and does not need to be re-written using any of the concurrency library
>> methods.
>>
>
> As I wrote back in one of my earliest emails:
>
> "that aside the latch is not needed. The fork() method starts a thread and
> joins it. So when createNoise() returns we already know for certain that
> the "noise has been created". What the sleep is doing is giving the GC a
> chance to run. "
>
> The sleep has nothing to do with synchronizing with the "noise" thread.
> And synchronization with the "noise" thread is already handled perfectly
> correctly.
>
> David
> -
>
>
>> --
*Twitter:* @theNeomatrix369
*Blog:* http://neomatrix369.wordpress.com
*JUG activity:* LJC Advocate (@adoptopenjdk & @adoptajsr programs)
*Meet-a-Project:* https://github.com/MutabilityDetector
*Devoxx UK 2013 was a grand success:*
http://www.devoxx.com/display/UK13/Home
*Don't chase success, rather aim for "Excellence", and success will come
chasing after you!*

Re: Looking for a sponsor to review changes made to two unit tests under jdk/test

[David Holmes]

On 8/04/2013 9:59 PM, Mani Sarkar wrote:

Thanks Alan, David for your feedback.

So effectively you are saying the Thread.sleep(10) is fine in the test
and does not need to be re-written using any of the concurrency library
methods.


As I wrote back in one of my earliest emails:

"that aside the latch is not needed. The fork() method starts a thread 
and joins it. So when createNoise() returns we already know for certain 
that the "noise has been created". What the sleep is doing is giving the 
GC a chance to run. "


The sleep has nothing to do with synchronizing with the "noise" thread. 
And synchronization with the "noise" thread is already handled perfectly 
correctly.


David
-



Cheers,
mani

On Mon, Apr 8, 2013 at 12:20 PM, David Holmes mailto:david.hol...@oracle.com>> wrote:

Mani,

Please go back to my original response. As Alan has just re-stated
we do not need a latch or a semaphore here because we already do a
join on the thread. As I have said the sleep is to allow the GC a
chance to run (eg finalizer and/or reference processor thread).

David


On 8/04/2013 8:53 PM, Mani Sarkar wrote:

We initially introduced CountdownLatch and now Semaphore, to
replace the
Thread.sleep(10) which took place before - what appears to be a
forced GC
(am I right?):

   System.err.println("GC " + i);
   System.gc();
   System.runFinalization();

As the threads join at the ends of the other, then we can do
away with the
Semaphore but how would we simulate the 10ms pause before the
forced GC -
is that necessary? Can we still use Semaphores to implement pauses?

Cheers,
mani

On Mon, Apr 8, 2013 at 11:07 AM, Alan Bateman
mailto:alan.bate...@oracle.com>>__wrote:

On 08/04/2013 10:39, Mani Sarkar wrote:

Hi David,

Here's the version of
*jdk8_tl/jdk/**test/java/lang/__**ref/Basic.java*implemented
using a
Semaphore:

   Hi Mani,


Is there a handshake really needed here? From a quick look
at the test
then it looks to me that fork (used by createNoise) does a
Thread.join so
it waiting until the task is complete before it returns.

-Alan







--
*Twitter:* @theNeomatrix369
*Blog:* http://neomatrix369.wordpress.com
*JUG activity:* LJC Advocate (@adoptopenjdk & @adoptajsr programs)
*Meet-a-Project:* https://github.com/MutabilityDetector
*Devoxx UK 2013 was a grand success:*
http://www.devoxx.com/display/UK13/Home
*/Don't chase success, rather aim for "Excellence", and success will
come chasing after you!/*

Re: Looking for a sponsor to review changes made to two unit tests under jdk/test

[Martin Buchholz]

Some very high level comments.

I think it's perfectly fine to use CountDownLatch to wait for events, as is
done in many jsr166 tests.

For the tricky problem of waiting for GC, see GcFinalization
https://code.google.com/p/guava-libraries/source/browse/guava-testlib/src/com/google/common/testing/GcFinalization.java

Re: RFR JDK-8011200 (was 7143928) : (coll) Optimize for Empty ArrayList and HashMap

[Martin Buchholz]

Style: spaces around "="

+private static final int DEFAULT_CAPACITY= 10;

---

It's a matter of taste whether to remove the temp var oldCapacity.  I
believe it was coded intentionally.

 public void trimToSize() {
 modCount++;
-int oldCapacity = elementData.length;
-if (size < oldCapacity) {
+if (size < elementData.length) {
 elementData = Arrays.copyOf(elementData, size);
 }

---

 754 throws java.io.IOException, ClassNotFoundException { 755
elementData = EMPTY_ELEMENTDATA;


Your indentation is off.

---

Because "threshold" is a serialized field, its javadoc is part of the
public interface of this class, and hence cannot refer to implementation
details like EMPTY_TABLE.

 161 /** 162  * The next size value at which to resize
(capacity * load factor). If 163  * table == EMPTY_TABLE then this
is the initial capacity at which the 164  * table will be created
when inflated.
 165  * @serial
 166  */
 167 int threshold;**

*
http://docs.oracle.com/javase/7/docs/api/serialized-form.html#java.util.HashMap
*
*
*

---

Consider making roundUpToPowerOf2 public.

Best Implementation is not obvious.  I would probably write a loop with core
x = x & (x - 1)
until get to zero.

 274 private static int roundUpToPowerOf2(int number) { 275
 int rounded = number >= MAXIMUM_CAPACITY 276 ?
MAXIMUM_CAPACITY 277 : (rounded =
Integer.highestOneBit(number)) != 0 278 ?
(Integer.bitCount(number) > 1) ? rounded << 1 : rounded 279
 : 1; 280  281 return rounded; 282 }



---

I think it's a mistake to "optimize" for the empty case in code like this:

 694 public boolean containsValue(Object value) { 695 if
(isEmpty()) { 696 return false; 697 } 698

Re: RFR: 8004518 & 8010122 : Default methods on Map

[Peter Levart]

Hi Mike,

It's unfortunate that getOrDefault() is specified so that it can't be 
implemented atomically in terms of existent (non-default) ConcurrentMap 
methods, so platform ConcurrentMap implementations will have atomic 
implementation and others will have to catch-up first. I hope this will 
not be a cause of any concurrency bugs.


The javadoc for computeIfPresent seems to be inconsistent:

 826  * If the value for the specified key is present and non-null,
 827  * attempts to compute a new mapping given the key and its current
 828  * mapped value.
 829  *
 830  * If the function returns {@code null}, the mapping is removed 
(*or*
 831  **remains absent if initially absent*).  If the function itself 
throws an
 832  * (unchecked) exception, the exception is rethrown, and the current 
mapping
 833  * is left unchanged.

I think the situation described *in bold* is non-existent.


Regards, Peter


On 04/08/2013 08:07 PM, Mike Duigou wrote:

Hello all;

This is a combined review for the new default methods on the java.util.Map 
interface being added for the JSR-335 lambda libraries. The reviews are being 
combined because they share a common unit test.

http://cr.openjdk.java.net/~mduigou/JDK-8010122/0/webrev/

8004518: Add in-place operations to Map
  forEach()
  replaceAll()

8010122: Add atomic operations to Map
  getOrDefault()
  putIfAbsent()  *
  remove(K, V)
  replace(K, V)
  replace(K, V, V)
  compute()  *
  merge()*
  computeIfAbsent()  *
  computeIfPresent() *

The * operations treat null values as being absent. (ie. the same as there 
being no mapping for the specified key).

The default implementations provided in Map are overridden in HashMap for 
performance purposes, in Hashtable for atomicity and performance purposes and 
in Collections for atomicity.

Mike
  

ReviewRequest 8011172: JJSR 310: DateTime API Updates II,

[Xueming Shen]

Hi,

JSR 310 has continued to refine and update the java.time API.
Please help review the proposed changeset as showed in webrev:

http://cr.openjdk.java.net/~sherman/8011172/webrev/

In addition to general javadoc improvements, the changes include:

java.time

* Duration - added a static from(temporalAmount) method to simplify
  conversions from other amounts
* Renamed the toString(Formatter) method to format(Formatter) in all classes
* Period - added a static from(temporalAmount) method to simplify conversions
* ZoneId -
   - Added getAvailableZoneIds method, a simpler mechanism than going
 to the provider
   - Added normalized() method to ease conversion to a fixed offset
   - renamed predefined static fields of timezone names

java.time.chrono

* ChronoLocalDate, ChronoLocalDateTime, ChonoZonedDateTime
   - changed xxx_COMPARATORs to static methods returning the Time Line
 Order comparators
   - Added a from(TemporalAcessor) method to ease conversions
* Chronology
- Added method to create a Date from EpochDay (And in each
  calendar subclass)
- Added resolveDate to allow resolving date components by the Chronology
- Serialization fixes
- Replaced raw return types with wildcard type
* Era
- Removed factory methods and getChronology - they did not work
  correctly in all cases
- Declared Era as a functional interface
* Hijrah calendar variations -
- Supporting the Umm alQura calendar
* Added HijrahEra, IsoEra, JapaneseEra, MinguEra, ThaiBuddhistEra
making the enums public
* MinguoDate, ThaiBuddhistDate, HijrahDate - Added getEra method
  to return the concrete Era type.

java.time.format

* DateTimeFormatter -
- Added fields for the predefined formatters
  (moved from DateTimeFormatters class)
- Updated patterns to be CLDR compatible
- Moved documentation for the pattern letters to the class javadoc
- Added support for Zone default and conversion
* DateTimeFormatterBuilder
- Updated documentation of patterns and corresponding equivalents
  to builder methods.
- Added a method to append the localized offset.

java.time.temporal

* Adjusters - class removed; all static adjusters moved to static methods
  in TemporalAdjuster
* ChronoField -
- Renamed EPOCH_MONTH to PROLEPTIC_MONTH
- Added getDisplayName - for locale specific field name
* Queries - class removed; all static query method moved to static methods
  in TemporalQuery
* TemporalField - added getDisplayName method
* UnsupportedTemporalTypeException - new subtype of DateTimeException
  to reflect no support for a unit or field
* WeekFields - Added fields for week and year of week-Based-Years to match
  CLDR fields "Y", "W"

Re: sun.awt.X11 logs still using String + (waste)

[Mandy Chung]

Peter, Laurent,

Peter's idea is a good one to add a couple of convenient methods to take 
Object parameters that will avoid the creation of array instances.  I'd 
also like to know what variants being used in the jdk to determine the 
pros and cons of the alternatives (this issue also applies to 
java.util.logging).


It was intentional to have PlatformLogger define only the useful methods 
necessary for jdk use.   The goal of PlatformLogger was to provide a 
light-weight utility for jdk to log debugging messages and eliminate the 
dependency to java.util.logging and avoid the unnecessary j.u.logging 
initialization (as disabled by default).   It was not a goal for 
PlatformLogger to be an exact mirror as java.util.logging.Logger.  My 
advice is to tune PlatformLogger to provide API that helps the platform 
implementation while avoid bloating the API.


Since you are touching some jdk files that use java.util.logging, would 
you like to contribute to convert them to use PlatformLogger:

   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7054233

It's perfectly fine to convert only some of them if not all.

Jim Gish is the owner of logging and will check with him if he has 
cycles to work with you on this.


Thanks
Mandy

On 4/8/13 3:06 AM, Laurent Bourgès wrote:

Peter, Mandy,

I think it would be great to make PlatformLogger very similar to 
Logger API:
I mean JUL Logger already has only 1 convenient method with 1 param so 
it may be great to have the same API in PlatformLogger too: maybe 
extend it to 2 or 3 params ...


Peter, could you look at the API differences between Logger / 
PlatformLogger to make PlatformLogger API more similar to JUL Logger ?


Laurent

2013/4/8 Peter Levart >


On 04/07/2013 07:11 PM, Laurent Bourgès wrote:


Hi

I started fixing All PlatformlLogger "bad" usages and I am fixing
also many jul Logger usages without isLoggable ...
That represents a lot of changes and a very boring job.

I think sending a webrew tomorrow.



Hi Laurent,

Since you're already deep in the task, you might have a rough
feeling what part of such unguarded log statements are of the
following type:

logger.[fine,info,...]("a message with {0} and {1} placeholders",
someArg1, someArg2);

where someArg1, ... are some values that are already present in
the context of the logging statement and don't have to be computed
just for satisfying the logging (not even boxing). Such usages
could be effectively optimized by adding some overloaded methods
to PlatformLogger that take 1, 2, 3, ... arguments:

class PlatformLogger {

...

public void finest(String msg, Object param1) {
if (isLoggable(Level.FINEST)) {
loggerProxy.doLog(Level.FINEST, msg, param1);
}
}

public void finest(String msg, Object param1, Object param2) {
if (isLoggable(Level.FINEST)) {
loggerProxy.doLog(Level.FINEST, msg, param1, param2);
}
}

public void finest(String msg, Object param1, Object param2,
Object param3) {
if (isLoggable(Level.FINEST)) {
loggerProxy.doLog(Level.FINEST, msg, param1, param2,
param3);
}
}

...

This would effectively guard creation of the arguments array with
an isLoggable check for some common usages, eliminating the need
to explicitly guard such statements. Of course, the user would
have to be aware of when such unguarded logging statement is
without overhead and when not...

How do you feel about such API extension?


Regards, Peter

hg: jdk8/tl/jdk: 8006036: (process) cleanup code in java/lang/Runtime/exec/WinCommand.java

[lance . andersen]

Changeset: 04617e462512
Author:lancea
Date:  2013-04-08 15:29 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/04617e462512

8006036: (process) cleanup code in java/lang/Runtime/exec/WinCommand.java
Reviewed-by: lancea
Contributed-by: Jim Gish 

! test/java/lang/Runtime/exec/WinCommand.java

hg: jdk8/tl/langtools: 8011677: EndPosTables should avoid hidden references to Parser

[jonathan . gibbons]

Changeset: 3f3cc8d3f13c
Author:jjg
Date:  2013-04-08 11:57 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/3f3cc8d3f13c

8011677: EndPosTables should avoid hidden references to Parser
Reviewed-by: mcimadamore

! src/share/classes/com/sun/tools/javac/parser/JavacParser.java

hg: jdk8/tl/langtools: 8011676: Instances of Tokens.Comment should not be defined in inner classes

[jonathan . gibbons]

Changeset: b402b93cbe38
Author:jjg
Date:  2013-04-08 11:54 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/b402b93cbe38

8011676: Instances of Tokens.Comment should not be defined in inner classes
Reviewed-by: mcimadamore

! src/share/classes/com/sun/tools/javac/parser/JavaTokenizer.java
! src/share/classes/com/sun/tools/javac/parser/JavadocTokenizer.java

RFR: 8004518 & 8010122 : Default methods on Map

[Mike Duigou]

Hello all;

This is a combined review for the new default methods on the java.util.Map 
interface being added for the JSR-335 lambda libraries. The reviews are being 
combined because they share a common unit test.

http://cr.openjdk.java.net/~mduigou/JDK-8010122/0/webrev/

8004518: Add in-place operations to Map
 forEach()
 replaceAll()

8010122: Add atomic operations to Map
 getOrDefault()   
 putIfAbsent()  *
 remove(K, V)
 replace(K, V)
 replace(K, V, V)
 compute()  *
 merge()*
 computeIfAbsent()  *
 computeIfPresent() *

The * operations treat null values as being absent. (ie. the same as there 
being no mapping for the specified key).

The default implementations provided in Map are overridden in HashMap for 
performance purposes, in Hashtable for atomicity and performance purposes and 
in Collections for atomicity.

Mike
 

Re: [OpenJDK 2D-Dev] sun.java2D.pisces big memory usage (waste ?)

[Andrea Aime]

On Fri, Apr 5, 2013 at 4:32 PM, Laurent Bourgès
wrote:

> Dear all,
>
> Here is my first pisces (beta) patch (webrev):
> http://jmmc.fr/~bourgesl/share/java2d-pisces/webrev-1/
>
> I succeed in fixing memory usage by having only 1 pisces instance
> (Renderer, stroker, iterator ...) per RendererContext (GC friendly).
>
> Impressive results between small and large drawings:
>

Good stuff. Is there anything I can do to help? I do have an up to date
version of JDK 8 sources on my disk, maybe I could
try to apply the patch and take it for a spin in a real GeoServer and see
how it goes.

By the way, wondering, is there any other benchmark to try out?
A possibly interesting one is this, where the author re-coded some selected
methods of Graphics2D for maximum performance
(but sometimes lower antialiasing quality) and created a applet to compare
the results in a number of synthetic test cases:
http://www.randelshofer.ch/oop/graphics/

Cheers
Andrea


-- 
==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for more
information.
==

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39  339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

---

Re: JAX-WS update coming soon

[Miroslav Kos]

On 04/08/2013 05:05 PM, Alan Bateman wrote:

On 21/03/2013 11:27, Alan Bateman wrote:


Just a heads-up that there is a JAX-WS update coming for jdk8. 
Miroslav Kos will be sending a webrev soon with the changes that 
update what we have in jdk8 from 2.2.7-b09 to 2.2.9-b13922.

Miroslav has put a webrev with the changes here:

http://cr.openjdk.java.net/~mkos/8010393/webrev.01/

Miroslav - can you briefly summarize the changes so that folks know 
what is coming?


The change set contains all the changes between versions 2.2.7-b09 and 
the current development version, which is 2.2.9-b12941. There are no big 
changes, it contains just bug fixes required by upper stacks (metro, 
glassfish, weblogic) and minor refactorings. Attached is a list of fixed 
issues.


Thanks
Miran




One minor update is that I needed to modify a make file to ensure that 
a new .xml gets packaged into resources.jar.


-Alan


diff --git a/makefiles/BuildJaxws.gmk b/makefiles/BuildJaxws.gmk
--- a/makefiles/BuildJaxws.gmk
+++ b/makefiles/BuildJaxws.gmk
@@ -55,7 +55,8 @@
 BIN:=$(JAXWS_OUTPUTDIR)/jaxws_classes,\
 COPY:=.xsd,\
COPY_FILES:=$(JAXWS_TOPDIR)/src/share/jaxws_classes/com/sun/tools/internal/xjc/runtime/JAXBContextFactory.java 
\
- 
$(JAXWS_TOPDIR)/src/share/jaxws_classes/com/sun/tools/internal/xjc/runtime/ZeroOneBooleanAdapter.java,\
+ 
$(JAXWS_TOPDIR)/src/share/jaxws_classes/com/sun/tools/internal/xjc/runtime/ZeroOneBooleanAdapter.java 
\
+ 
$(JAXWS_TOPDIR)/src/share/jaxws_classes/com/sun/xml/internal/ws/assembler/jaxws-tubes-default.xml,\

 ADD_JAVAC_FLAGS=-cp $(OUTPUT_ROOT)/jaxp/dist/lib/classes.jar))

 $(JAXWS_OUTPUTDIR)/jaxws_classes/META-INF/services/com.sun.tools.internal.ws.wscompile.Plugin: 
\

@@ -98,7 +99,7 @@

 $(eval $(call SetupArchive,ARCHIVE_JAXWS,$(BUILD_JAXWS) $(BUILD_JAF) 
$(TARGET_PROP_FILES),\
 SRCS:=$(JAXWS_OUTPUTDIR)/jaxws_classes 
$(JAXWS_OUTPUTDIR)/jaf_classes,\

-SUFFIXES:=.class .properties .xsd .java \
+SUFFIXES:=.class .properties .xsd .xml .java \
   com.sun.mirror.apt.AnnotationProcessorFactory \
   com.sun.tools.internal.xjc.Plugin,\
 JAR:=$(JAXWS_OUTPUTDIR)/dist/lib/classes.jar))



Displaying issues *1* to *71* of *71* matching issues. as at: *08/Apr/1303:52 
PM*  

*Key*   *Summary*
   Make JAX-WS run on J2SE 5.0 

   httpproxy username and password 
not supported  
   wsgen fails to resolve all 
'service implementation bean' methods  
   jaxws should pass encoding 
option to jaxb  
   wsimport authFile URL 
containing passwords should support encoded/escaped characters... 
 
   Allow wild card matching to 
allow the same user:password for all urls with the same host name 
 
 
com.sun.xml.ws.api.model.wsdl.WSDLModel.WSDLParser.parse error in parsering 
wsdl:message/wsdl:part defines "type" (not "element") 
 
 IllegalAnnotationsException: 2 counts 
of IllegalAnnotationExceptions: MemberSubmissionEndpointReference$Address and 
W3CEndpointReference$Address  
   Basic Authentication with 
wsimport does not allow @ in username  
   wsa:to header of ack message 
changed to annonymous when the request message has no replyTo 
 
   Back Compatible issue, for 
method: com.sun.xml.ws.server.EndpointFactory.verifyImplementorClass 
 
   HttpAdapter send empty response 
message even when the message is null given that the endpointAddress of Packet 
is not null  
   unable to delete .war file 
after wsimport completed  
   Error listenerStart Sep 27, 
2012 12:02:48 AM org.apache.catalina.core.StandardContext start 
 
   consider updating dependencies 
 
   move build from ant to maven 

Re: JAX-WS update coming soon

[Alan Bateman]

On 21/03/2013 11:27, Alan Bateman wrote:


Just a heads-up that there is a JAX-WS update coming for jdk8. 
Miroslav Kos will be sending a webrev soon with the changes that 
update what we have in jdk8 from 2.2.7-b09 to 2.2.9-b13922.

Miroslav has put a webrev with the changes here:

http://cr.openjdk.java.net/~mkos/8010393/webrev.01/

Miroslav - can you briefly summarize the changes so that folks know what 
is coming?


One minor update is that I needed to modify a make file to ensure that a 
new .xml gets packaged into resources.jar.


-Alan


diff --git a/makefiles/BuildJaxws.gmk b/makefiles/BuildJaxws.gmk
--- a/makefiles/BuildJaxws.gmk
+++ b/makefiles/BuildJaxws.gmk
@@ -55,7 +55,8 @@
 BIN:=$(JAXWS_OUTPUTDIR)/jaxws_classes,\
 COPY:=.xsd,\
 
COPY_FILES:=$(JAXWS_TOPDIR)/src/share/jaxws_classes/com/sun/tools/internal/xjc/runtime/JAXBContextFactory.java 
\
-
$(JAXWS_TOPDIR)/src/share/jaxws_classes/com/sun/tools/internal/xjc/runtime/ZeroOneBooleanAdapter.java,\
+
$(JAXWS_TOPDIR)/src/share/jaxws_classes/com/sun/tools/internal/xjc/runtime/ZeroOneBooleanAdapter.java 
\
+  
$(JAXWS_TOPDIR)/src/share/jaxws_classes/com/sun/xml/internal/ws/assembler/jaxws-tubes-default.xml,\

 ADD_JAVAC_FLAGS=-cp $(OUTPUT_ROOT)/jaxp/dist/lib/classes.jar))

 
$(JAXWS_OUTPUTDIR)/jaxws_classes/META-INF/services/com.sun.tools.internal.ws.wscompile.Plugin:
 \
@@ -98,7 +99,7 @@

 $(eval $(call SetupArchive,ARCHIVE_JAXWS,$(BUILD_JAXWS) $(BUILD_JAF) 
$(TARGET_PROP_FILES),\
 SRCS:=$(JAXWS_OUTPUTDIR)/jaxws_classes 
$(JAXWS_OUTPUTDIR)/jaf_classes,\

-SUFFIXES:=.class .properties .xsd .java \
+SUFFIXES:=.class .properties .xsd .xml .java \
   com.sun.mirror.apt.AnnotationProcessorFactory \
   com.sun.tools.internal.xjc.Plugin,\
 JAR:=$(JAXWS_OUTPUTDIR)/dist/lib/classes.jar))

hg: jdk8/tl/langtools: 5 new changesets

[maurizio . cimadamore]

Changeset: b71a61d39cf7
Author:mcimadamore
Date:  2013-04-08 15:51 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/b71a61d39cf7

8010922: Cleanup: add support for ad-hoc method check logic
Summary: Support pluggable method checkers
Reviewed-by: jjg

! src/share/classes/com/sun/tools/javac/comp/Attr.java
! src/share/classes/com/sun/tools/javac/comp/DeferredAttr.java
! src/share/classes/com/sun/tools/javac/comp/Infer.java
! src/share/classes/com/sun/tools/javac/comp/Resolve.java

Changeset: b54122b9372d
Author:mcimadamore
Date:  2013-04-08 15:52 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/b54122b9372d

8010823: DefaultMethodTest.testReflectCall fails with new lambda VM
Summary: Fix lambda test
Reviewed-by: jjg

! test/tools/javac/lambdaShapes/org/openjdk/tests/vm/DefaultMethodsTest.java

Changeset: e9d986381414
Author:mcimadamore
Date:  2013-04-08 15:53 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/e9d986381414

8010404: Lambda debugging: redundant LineNumberTable entry for lambda capture
Summary: Ignore indy entries in LineNumberTable
Reviewed-by: jjg

! src/share/classes/com/sun/tools/javac/jvm/Code.java
! src/share/classes/com/sun/tools/javac/jvm/Gen.java
! test/tools/javac/lambda/TestInvokeDynamic.java

Changeset: 94a202228ec2
Author:mcimadamore
Date:  2013-04-08 15:57 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/94a202228ec2

8009131: Overload: javac should discard methods that lead to errors in lambdas 
with implicit parameter types
Summary: Lambdas that have errors in their bodies should make enclosing 
overload resolution fail
Reviewed-by: jjg

! src/share/classes/com/sun/tools/javac/comp/Attr.java
! src/share/classes/com/sun/tools/javac/resources/compiler.properties
+ test/tools/javac/diags/examples/BadArgTypesInLambda.java
! test/tools/javac/lambda/BadRecovery.out
! test/tools/javac/lambda/TargetType01.java
- test/tools/javac/lambda/TargetType01.out
! test/tools/javac/lambda/TargetType43.out
+ test/tools/javac/lambda/TargetType66.java
+ test/tools/javac/lambda/TargetType66.out

Changeset: c635a966ce84
Author:mcimadamore
Date:  2013-04-08 15:59 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/c635a966ce84

8010822: Intersection type cast for functional expressions does not follow spec 
EDR
Summary: Remove support for marker interfaces; redefine intersection type casts 
to be order-independent
Reviewed-by: jjg

! src/share/classes/com/sun/tools/javac/code/Type.java
! src/share/classes/com/sun/tools/javac/code/Types.java
! src/share/classes/com/sun/tools/javac/comp/Attr.java
! src/share/classes/com/sun/tools/javac/resources/compiler.properties
! src/share/classes/com/sun/tools/javac/util/RichDiagnosticFormatter.java
+ test/tools/javac/diags/examples/NotAnInterfaceComponent.java
- test/tools/javac/diags/examples/SecondaryBoundMustBeMarkerIntf.java
! test/tools/javac/lambda/Intersection01.java
- test/tools/javac/lambda/Intersection01.out
! test/tools/javac/lambda/intersection/IntersectionTargetTypeTest.java

Re: RFR JDK-8011200 (was 7143928) : (coll) Optimize for Empty ArrayList and HashMap

[Alan Bateman]

On 06/04/2013 23:02, Mike Duigou wrote:

Hello all;

Another, and hopefully the final, update to the webrev for this issue. The 
revised webrev is here:

http://cr.openjdk.java.net/~mduigou/JDK-8011200/1/webrev/

The important changes in this revision:

- I've removed the serialData change in HashMap. The implementation now reads 
the capacity and gracefully handles non-power of 2 values.

- I'm not entirely convinced that having serialization emulate clone() for 
capacity handling is the best answer. I might also want to change clone() to 
size it's result based upon the number of mappings in the source rather its the 
capacity. Anybody have strong feelings about this to suggest one behaviour is 
obviously better?

Any other final thoughts?

Mike


I think this is the webrev:

http://cr.openjdk.java.net/~mduigou/JDK-8011200/2/webrev/

It's impossible to predict what the usage will be after you 
reconstitute. Personally I think it's better to leave it as is, meaning 
the rounded-up size as otherwise you might reconstitute to a capacity 
that is much more than you might ever need.


I didn't notice in the previous revisions but roundUpToPowerOf2 can 
assign "rounded" twice (it probably doesn't happen when it gets compiled 
at runtime but still looks a bit odd).


The test still have the bug issue number.

-Alan

Re: JDK-8011653: Upgrade to JAXP 1.5

[Alan Bateman]

On 08/04/2013 08:39, huizhe wang wrote:

Hi Lance, Alan,

As I mentioned, I'd like to propose an integration of JAXP 1.5 into 
JDK8.  JAXP 1.5 adds a new feature to control connections.


Here's the webrev:
http://cr.openjdk.java.net/~joehw/jdk8/8011653/webrev/

Thanks,
Joe
I've done a first pass over the spec/javadoc changes (not the 
implementation yet).


Are you planning to add @since to each of the new constants in XMLConstants?

For the new properties then it specifies that a "a runtime exception" 
will be thrown. Can this be more specific?


The URI scheme is specified in terms of the obsolete RFC 2396 whereas I 
think it just needs to be a String that is equal (ignoring case) to the 
protocol of the connection that is attempting.


For jaxp.properties then it reads "and property XXX is specified". I'd 
probably change this to "the property".


For the existing FEATURE_SECURE_PROCESSING then "accordance with the 
letter" is a bit unusual, I would be "the letter" could be dropped.


For each of the factories then it specifies that all implementation are 
required to support the new property but this would seem to invalidate 
all existing provider implementations. I don't think providers are 
versioned so all I can suggest is that this be worded to make it clear 
that all implementations that implement JAXP 1.5 or newer support this 
property.


A small comment is that there seems to have been previous attempts to 
keep some of the files at 80 or thereabouts columns. The new javadoc 
requires a bit of horizontal scrolling.


-Alan.

Re: Looking for a sponsor to review changes made to two unit tests under jdk/test

[Mani Sarkar]

Thanks Alan, David for your feedback.

So effectively you are saying the Thread.sleep(10) is fine in the test and
does not need to be re-written using any of the concurrency library methods.

Cheers,
mani

On Mon, Apr 8, 2013 at 12:20 PM, David Holmes wrote:

> Mani,
>
> Please go back to my original response. As Alan has just re-stated we do
> not need a latch or a semaphore here because we already do a join on the
> thread. As I have said the sleep is to allow the GC a chance to run (eg
> finalizer and/or reference processor thread).
>
> David
>
>
> On 8/04/2013 8:53 PM, Mani Sarkar wrote:
>
>> We initially introduced CountdownLatch and now Semaphore, to replace the
>> Thread.sleep(10) which took place before - what appears to be a forced GC
>> (am I right?):
>>
>>   System.err.println("GC " + i);
>>   System.gc();
>>   System.runFinalization();
>>
>> As the threads join at the ends of the other, then we can do away with the
>> Semaphore but how would we simulate the 10ms pause before the forced GC -
>> is that necessary? Can we still use Semaphores to implement pauses?
>>
>> Cheers,
>> mani
>>
>> On Mon, Apr 8, 2013 at 11:07 AM, Alan Bateman **
>> wrote:
>>
>>  On 08/04/2013 10:39, Mani Sarkar wrote:
>>>
>>>  Hi David,

 Here's the version of
 *jdk8_tl/jdk/**test/java/lang/ref/Basic.java*implemented using a
 Semaphore:

   Hi Mani,

>>>
>>> Is there a handshake really needed here? From a quick look at the test
>>> then it looks to me that fork (used by createNoise) does a Thread.join so
>>> it waiting until the task is complete before it returns.
>>>
>>> -Alan
>>>
>>>
>>
>>
>>


-- 
*Twitter:* @theNeomatrix369
*Blog:* http://neomatrix369.wordpress.com
*JUG activity:* LJC Advocate (@adoptopenjdk & @adoptajsr programs)
*Meet-a-Project:* https://github.com/MutabilityDetector
*Devoxx UK 2013 was a grand success:*
http://www.devoxx.com/display/UK13/Home
*Don't chase success, rather aim for "Excellence", and success will come
chasing after you!*

Re: Proxy.isProxyClass scalability

[Peter Levart]

Hi Alan,

Is this still being considered? Recently there were some changes in the 
j.l.r.Proxy (the @CallerSensitive annotation), so my final webrev of the 
patch will have to be rebased. Do you still think we should not add new 
fields to ClassLoader? I have some ideas how to do it without adding a 
field to ClassLoader but for the price of some additional space overhead 
for each Proxy class produced. On the other hand I think that in the 
future, more platform-internal data structures would want to be 
"attached" to ClassLoaders so perhaps a single ConcurrentHashMap per 
ClassLoader could be re-used for different purposes by using distinct 
(never-equal) keys for each purpose (fox example, the lambda metafactory 
currently does not do any caching for it's own spun SAM proxy classes or 
CallSite objects, but could benefit quite a bit from doing so).


Regards, Peter

On 01/28/2013 05:13 PM, Peter Levart wrote:

Hi Alan,

I prepared the variant with lazy initialization of ConcurrentHashMaps 
per ClassLoader and performance measurements show no differences. So 
here's this variant:


* http://dl.dropbox.com/u/101777488/jdk8-tl/proxy/webrev.03/index.html

I also checked the ClassLoader layout and as it happens the additional 
pointer slot increases the ClassLoader object size by 8 bytes in both 
addressing modes: 32bit and 64bit. But that's a small overhead 
compared to for example the deep-size of AppClassLoader at the 
beginning of the main method: 14648 bytes (32bit) / 22432 bytes (64bit).


Regards, Peter

On 01/28/2013 01:57 PM, Peter Levart wrote:

On 01/28/2013 12:49 PM, Alan Bateman wrote:

On 25/01/2013 17:55, Peter Levart wrote:


:

The solution is actually very simple. I just want to validate my 
reasoning before jumping to implement it:


- for solving scalability of getProxyClass cache, a field with a 
reference to ConcurrentHashMap, ClassProxy>> is added to j.l.ClassLoader
- for solving scalability of isProxyClass, a field with a reference 
to ConcurrentHashMap, Boolean> is added to 
j.l.ClassLoader
I haven't had time to look very closely as your more recent changes 
(you are clearly doing very good work here). The only thing I wonder 
if whether it would be possible to avoid adding to ClassLoader. I 
can't say what percentage of frameworks and applications use proxies 
but it would be nice if the impact on applications that don't use 
proxies is zero.

Hi Alan,

Hm, well. Any application that uses run-time annotations, is 
implicitly using Proxies. But I agree that there are applications 
that don't use either. Such applications usually don't use many 
ClassLoaders either. Applications that use many ClassLoaders are 
typically application servers or applications written for modular 
systems (such as OSGI or NetBeans) and all those applications are 
also full of runtime annotations nowadays. So a typical application 
that does not use neither Proxies nor runtime annotations is composed 
of bootstrap classloader, AppClassLoader and ExtClassLoader. The 
ConcurrentHashMap for the bootstrap classloader is hosted by 
j.l.r.Proxy class and is only initialized when the j.l.r.Proxy class 
is initialized - so in this case never. The overhead for such 
applications is therefore an empty ConcurrentHashMap instance plus 
the overhead for a pointer slot in the ClassLoader object multiplied 
by the number of ClassLoaders (typically 2). An empty 
ConcurrentHashMap in JDK8 is only pre-allocating a single internal 
Segment:


java.util.concurrent.ConcurrentHashMap@642b6fc7(48 bytes) {
  keySet: null
  values: null
  hashSeed: int
  segmentMask: int
  segmentShift: int
  segments: 
java.util.concurrent.ConcurrentHashMap$Segment[16]@8e1dfb1(80 bytes) {

java.util.concurrent.ConcurrentHashMap$Segment@2524e205(40 bytes) {
  sync: 
java.util.concurrent.locks.ReentrantLock$NonfairSync@17feafba(32 
bytes) {

exclusiveOwnerThread: null
head: null
tail: null
state: int
  }->(32 deep bytes)
  table: 
java.util.concurrent.ConcurrentHashMap$HashEntry[2]@1c3aacb4(24 bytes) {

null
null
  }->(24 deep bytes)
  count: int
  modCount: int
  threshold: int
  loadFactor: float
}->(96 deep bytes)
null
null
null
null
null
null
null
null
null
null
null
null
null
null
null
  }->(176 deep bytes)
  keySet: null
  entrySet: null
  values: null
}->(224 deep bytes)

...therefore the overhead is approx. 224+4 = 228 bytes (on 32 bit 
pointer environments) per ClassLoader. In typical application (with 2 
ClassLoader objects) this amounts to approx. 456 bytes.


Is 456 bytes overhead too much?

If it is, I could do lazy initialization of per-classloader CHM 
instances, but then the fast-path would incur a little additional 
penalty (not to be taken seriously though).


Regards, Peter

P.S. I was inspecting the ClassValue internal implementation. This is 
a very nice piece of Java code. Without usin

Re: sun.awt.X11 logs still using String + (waste)

[Laurent Bourgès]

Anthony,

here is an updated patch concerning both PlatformLogger and Logger 'bad'
usages:
http://jmmc.fr/~bourgesl/share/webrev-8010297.3/

It impacts now awt, swing, JMX, security, network, and apache xml.

Thanks for the review,
Laurent

2013/4/8 Laurent Bourgès 

> Peter, Mandy,
>
> I think it would be great to make PlatformLogger very similar to Logger
> API:
> I mean JUL Logger already has only 1 convenient method with 1 param so it
> may be great to have the same API in PlatformLogger too: maybe extend it to
> 2 or 3 params ...
>
> Peter, could you look at the API differences between Logger /
> PlatformLogger to make PlatformLogger API more similar to JUL Logger ?
>
> Laurent
>
>
> 2013/4/8 Peter Levart 
>
>>  On 04/07/2013 07:11 PM, Laurent Bourgès wrote:
>>
>> Hi
>>
>> I started fixing All PlatformlLogger "bad" usages and I am fixing also
>> many jul Logger usages without isLoggable ...
>> That represents a lot of changes and a very boring job.
>>
>> I think sending a webrew tomorrow.
>>
>>
>> Hi Laurent,
>>
>> Since you're already deep in the task, you might have a rough feeling
>> what part of such unguarded log statements are of the following type:
>>
>> logger.[fine,info,...]("a message with {0} and {1} placeholders",
>> someArg1, someArg2);
>>
>> where someArg1, ... are some values that are already present in the
>> context of the logging statement and don't have to be computed just for
>> satisfying the logging (not even boxing). Such usages could be effectively
>> optimized by adding some overloaded methods to PlatformLogger that take 1,
>> 2, 3, ... arguments:
>>
>> class PlatformLogger {
>>
>> ...
>>
>> public void finest(String msg, Object param1) {
>> if (isLoggable(Level.FINEST)) {
>> loggerProxy.doLog(Level.FINEST, msg, param1);
>> }
>> }
>>
>> public void finest(String msg, Object param1, Object param2) {
>> if (isLoggable(Level.FINEST)) {
>> loggerProxy.doLog(Level.FINEST, msg, param1, param2);
>> }
>> }
>>
>> public void finest(String msg, Object param1, Object param2, Object
>> param3) {
>> if (isLoggable(Level.FINEST)) {
>> loggerProxy.doLog(Level.FINEST, msg, param1, param2, param3);
>> }
>> }
>>
>> ...
>>
>> This would effectively guard creation of the arguments array with an
>> isLoggable check for some common usages, eliminating the need to explicitly
>> guard such statements. Of course, the user would have to be aware of when
>> such unguarded logging statement is without overhead and when not...
>>
>> How do you feel about such API extension?
>>
>>
>> Regards, Peter
>>
>>
>>
>>  Laurent
>> Le 4 avr. 2013 14:08, "Laurent Bourgès"  a
>> écrit :
>>
>>> Ok, I can provide an updated patch after finding / fixing all usages.
>>>
>>> Probably in 1 or 2 days,
>>>
>>> Laurent
>>>
>>> 2013/4/4 Anthony Petrov 
>>>
 Yes, this makes sense. Do you want to update your fix for 8010297 to
 include these changes as well?

 --
 best regards,
 Anthony


 On 04/04/13 15:47, Laurent Bourgès wrote:

> Dear all,
>
>  I just realized there is another problem with PlatformLogger log
> statements:
> XBaseWindow:
>  public boolean grabInput() {
>  grabLog.fine("Grab input on {0}", this);
> ...
> }
>
> This calls the PlatformLogger.fine( varargs):
>  public void fine(String msg, Object... params) {
>  logger.doLog(FINE, msg, params);
>  }
>
> Doing so, the JVM creates a new Object[] instance to provide params as
> varargs.
>
> I would recommend using isLoggable() test to avoid such waste if the
> log
> is disabled (fine, finer, finest ...).
>
> Do you want me to provide a new patch related to this problem ?
>
> Does somebody have an idea to automatically analyze the JDK code and
> detect missing isLoggable statements ...
>
> Regards,
> Laurent
>
> 2013/4/3 Laurent Bourgès   >
>
>
> Anthony,
>
> could you tell me once it is in the OpenJDK8 repository ?
> I would then perform again profiling tests to check if there is no
> more missing isLoggable() statements.
>
> Once JMX and other projects switch to PlatformLogger, I could check
> again.
>
> Maybe I could write a small java code checker (pmd rule) to test if
> there is missing isLoggable() statements wrapping PlatformLogger
> log
> statements. Any idea about how to reuse java parser to do so ?
>
> Regards,
>
> Laurent
>
> 2013/4/2 Anthony Petrov   >
>
>
> Looks fine to me as well. Thanks for fixing this, Laurent.
>
> Let's wait a couple more days in case Net or Swing folks want
> to
> review the 

Re: Looking for a sponsor to review changes made to two unit tests under jdk/test

[David Holmes]

On 8/04/2013 7:39 PM, Mani Sarkar wrote:

Hi David,

Here's the version of
*jdk8_tl/jdk/**test/java/lang/ref/Basic.java*implemented using a
Semaphore:

x-
diff -r 16f63a94c231 test/java/lang/ref/Basic.java
--- a/test/java/lang/ref/Basic.java Fri Apr 05 18:20:12 2013 -0700
+++ b/test/java/lang/ref/Basic.java Sun Apr 07 11:06:17 2013 +0100
@@ -29,7 +29,7 @@

  import java.lang.ref.*;
  import java.util.Vector;
-
+import java.util.concurrent.Semaphore;

  public class Basic {

@@ -71,10 +71,11 @@
  });
  }

-static void createNoise() throws InterruptedException {
+static void createNoise(final Semaphore complete) throws
InterruptedException {
  fork(new Runnable() {
  public void run() {
  keep.addElement(new PhantomReference(new Object(), q2));
+complete.release();
  }
  });
  }
@@ -101,9 +102,12 @@
  for (int i = 1;; i++) {
  Reference r;

-createNoise();
-System.err.println("GC " + i);
-Thread.sleep(10);
+final Semaphore noiseComplete = new Semaphore(1);
+noiseComplete.acquire();
+
+createNoise(noiseComplete);


This is wrong. You would want:

   final Semaphore noiseComplete = new Semaphore(0);
   createNoise(noiseComplete);
   noiseComplete.acquire();

but again this is not needed at all.

David
--


+
+System.err.println("GC " + i);
  System.gc();
  System.runFinalization();
  x-

After running the test the first time I added temporary logging statements
between the blocks of code that dealt with the Semaphore and got the below
results:

*--System.out:(12/444)--*
*> Semaphore created.*
*> Semaphore acquired.*
*>> Inside createnoise(): before adding an element.*
*>> Inside createnoise(): after adding an element.*
*>> Inside createnoise(): Semaphore released.*
*> After createnoise() is called.*
*> Semaphore created.*
*> Semaphore acquired.*
*>> Inside createnoise(): before adding an element.*
*>> Inside createnoise(): after adding an element.*
*>> Inside createnoise(): Semaphore released.*
*> After createnoise() is called.*

What is the principle difference between CountdownLatch and Semaphore and
why is a later more applicable in this context than the former? The
mehanism by which they handle the two threads appears the same to me, could
you please explain to satisfy my concurrency curiosity.

Regards,
mani

On Mon, Apr 8, 2013 at 3:00 AM, David Holmes wrote:


On 7/04/2013 9:37 AM, Mani Sarkar wrote:


Hi David,

Sorry for not getting back earlier. Here's the changes to
/jdk8_tl/jdk/test/java/lang/**ref/Basic.java that you suggested earlier.



Okay but what about my comment that the latch usage is completely
unnecessary in the first place ??

David
-

  ---x--**---


diff -r 16f63a94c231 test/java/lang/ref/Basic.java
--- a/test/java/lang/ref/Basic.**javaFri Apr 05 18:20:12 2013 -0700
+++ b/test/java/lang/ref/Basic.**javaSun Apr 07 00:27:55 2013 +0100

@@ -29,7 +29,7 @@
   import java.lang.ref.*;
   import java.util.Vector;
-
+import java.util.concurrent.**CountDownLatch;
   public class Basic {
@@ -71,10 +71,11 @@
   });
   }
-static void createNoise() throws InterruptedException {
+static void createNoise(final CountDownLatch complete) throws
InterruptedException {
   fork(new Runnable() {
   public void run() {
   keep.addElement(new PhantomReference(new Object(), q2));
+complete.countDown();
   }
   });
   }
@@ -101,9 +102,11 @@
   for (int i = 1;; i++) {
   Reference r;
-createNoise();
++   CountDownLatch noiseComplete = new CountDownLatch(1);
++   createNoise(noiseComplete);
+
   System.err.println("GC " + i);
-Thread.sleep(10);
+noiseComplete.await();
   System.gc();
   System.runFinalization();
---x--**---

Its still implemented with CountdownLatch, but as you suggest we
implement the above via Semaphore it will have to be the next version
from me - CountdownLatch was suggested by many at the TestFest last
month but personally I benefit from getting exposed to different
techniques. I'll back to you with a solution applied using Semaphore.

Cheers,
mani

On Fri, Apr 5, 2013 at 2:40 AM, David Holmes mailto:david.holmes@oracle.**com >> wrote:

 On 5/04/2013 11:27 AM, Mani Sarkar wrote:

 Hi David,

 I'll reply in more detail later but to respond to your comment on:

   > I would not add the extra methods around the cdl.await() and
 cdl.countDown() as they just obscure things
 In general its meant to do the opposite. We come across a lot of
 code
 that does not express intent, a

Re: Looking for a sponsor to review changes made to two unit tests under jdk/test

[David Holmes]

Mani,

Please go back to my original response. As Alan has just re-stated we do 
not need a latch or a semaphore here because we already do a join on the 
thread. As I have said the sleep is to allow the GC a chance to run (eg 
finalizer and/or reference processor thread).


David

On 8/04/2013 8:53 PM, Mani Sarkar wrote:

We initially introduced CountdownLatch and now Semaphore, to replace the
Thread.sleep(10) which took place before - what appears to be a forced GC
(am I right?):

  System.err.println("GC " + i);
  System.gc();
  System.runFinalization();

As the threads join at the ends of the other, then we can do away with the
Semaphore but how would we simulate the 10ms pause before the forced GC -
is that necessary? Can we still use Semaphores to implement pauses?

Cheers,
mani

On Mon, Apr 8, 2013 at 11:07 AM, Alan Bateman wrote:


On 08/04/2013 10:39, Mani Sarkar wrote:


Hi David,

Here's the version of
*jdk8_tl/jdk/**test/java/lang/**ref/Basic.java*implemented using a
Semaphore:

  Hi Mani,


Is there a handshake really needed here? From a quick look at the test
then it looks to me that fork (used by createNoise) does a Thread.join so
it waiting until the task is complete before it returns.

-Alan

Re: Looking for a sponsor to review changes made to two unit tests under jdk/test

[Mani Sarkar]

We initially introduced CountdownLatch and now Semaphore, to replace the
Thread.sleep(10) which took place before - what appears to be a forced GC
(am I right?):

 System.err.println("GC " + i);
 System.gc();
 System.runFinalization();

As the threads join at the ends of the other, then we can do away with the
Semaphore but how would we simulate the 10ms pause before the forced GC -
is that necessary? Can we still use Semaphores to implement pauses?

Cheers,
mani

On Mon, Apr 8, 2013 at 11:07 AM, Alan Bateman wrote:

> On 08/04/2013 10:39, Mani Sarkar wrote:
>
>> Hi David,
>>
>> Here's the version of
>> *jdk8_tl/jdk/**test/java/lang/**ref/Basic.java*implemented using a
>> Semaphore:
>>
>>  Hi Mani,
>
> Is there a handshake really needed here? From a quick look at the test
> then it looks to me that fork (used by createNoise) does a Thread.join so
> it waiting until the task is complete before it returns.
>
> -Alan
>



-- 
*Twitter:* @theNeomatrix369
*Blog:* http://neomatrix369.wordpress.com
*JUG activity:* LJC Advocate (@adoptopenjdk & @adoptajsr programs)
*Meet-a-Project:* https://github.com/MutabilityDetector
*Devoxx UK 2013 was a grand success:*
http://www.devoxx.com/display/UK13/Home
*Don't chase success, rather aim for "Excellence", and success will come
chasing after you!*

Re: Looking for a sponsor to review changes made to two unit tests under jdk/test

[Alan Bateman]

On 08/04/2013 10:39, Mani Sarkar wrote:

Hi David,

Here's the version of
*jdk8_tl/jdk/**test/java/lang/ref/Basic.java*implemented using a
Semaphore:


Hi Mani,

Is there a handshake really needed here? From a quick look at the test 
then it looks to me that fork (used by createNoise) does a Thread.join 
so it waiting until the task is complete before it returns.


-Alan

Re: sun.awt.X11 logs still using String + (waste)

[Laurent Bourgès]

Peter, Mandy,

I think it would be great to make PlatformLogger very similar to Logger
API:
I mean JUL Logger already has only 1 convenient method with 1 param so it
may be great to have the same API in PlatformLogger too: maybe extend it to
2 or 3 params ...

Peter, could you look at the API differences between Logger /
PlatformLogger to make PlatformLogger API more similar to JUL Logger ?

Laurent

2013/4/8 Peter Levart 

>  On 04/07/2013 07:11 PM, Laurent Bourgès wrote:
>
> Hi
>
> I started fixing All PlatformlLogger "bad" usages and I am fixing also
> many jul Logger usages without isLoggable ...
> That represents a lot of changes and a very boring job.
>
> I think sending a webrew tomorrow.
>
>
> Hi Laurent,
>
> Since you're already deep in the task, you might have a rough feeling what
> part of such unguarded log statements are of the following type:
>
> logger.[fine,info,...]("a message with {0} and {1} placeholders",
> someArg1, someArg2);
>
> where someArg1, ... are some values that are already present in the
> context of the logging statement and don't have to be computed just for
> satisfying the logging (not even boxing). Such usages could be effectively
> optimized by adding some overloaded methods to PlatformLogger that take 1,
> 2, 3, ... arguments:
>
> class PlatformLogger {
>
> ...
>
> public void finest(String msg, Object param1) {
> if (isLoggable(Level.FINEST)) {
> loggerProxy.doLog(Level.FINEST, msg, param1);
> }
> }
>
> public void finest(String msg, Object param1, Object param2) {
> if (isLoggable(Level.FINEST)) {
> loggerProxy.doLog(Level.FINEST, msg, param1, param2);
> }
> }
>
> public void finest(String msg, Object param1, Object param2, Object
> param3) {
> if (isLoggable(Level.FINEST)) {
> loggerProxy.doLog(Level.FINEST, msg, param1, param2, param3);
> }
> }
>
> ...
>
> This would effectively guard creation of the arguments array with an
> isLoggable check for some common usages, eliminating the need to explicitly
> guard such statements. Of course, the user would have to be aware of when
> such unguarded logging statement is without overhead and when not...
>
> How do you feel about such API extension?
>
>
> Regards, Peter
>
>
>
>  Laurent
> Le 4 avr. 2013 14:08, "Laurent Bourgès"  a
> écrit :
>
>> Ok, I can provide an updated patch after finding / fixing all usages.
>>
>> Probably in 1 or 2 days,
>>
>> Laurent
>>
>> 2013/4/4 Anthony Petrov 
>>
>>> Yes, this makes sense. Do you want to update your fix for 8010297 to
>>> include these changes as well?
>>>
>>> --
>>> best regards,
>>> Anthony
>>>
>>>
>>> On 04/04/13 15:47, Laurent Bourgès wrote:
>>>
 Dear all,

  I just realized there is another problem with PlatformLogger log
 statements:
 XBaseWindow:
  public boolean grabInput() {
  grabLog.fine("Grab input on {0}", this);
 ...
 }

 This calls the PlatformLogger.fine( varargs):
  public void fine(String msg, Object... params) {
  logger.doLog(FINE, msg, params);
  }

 Doing so, the JVM creates a new Object[] instance to provide params as
 varargs.

 I would recommend using isLoggable() test to avoid such waste if the log
 is disabled (fine, finer, finest ...).

 Do you want me to provide a new patch related to this problem ?

 Does somebody have an idea to automatically analyze the JDK code and
 detect missing isLoggable statements ...

 Regards,
 Laurent

 2013/4/3 Laurent Bourgès >>>  >


 Anthony,

 could you tell me once it is in the OpenJDK8 repository ?
 I would then perform again profiling tests to check if there is no
 more missing isLoggable() statements.

 Once JMX and other projects switch to PlatformLogger, I could check
 again.

 Maybe I could write a small java code checker (pmd rule) to test if
 there is missing isLoggable() statements wrapping PlatformLogger log
 statements. Any idea about how to reuse java parser to do so ?

 Regards,

 Laurent

 2013/4/2 Anthony Petrov >>>  >


 Looks fine to me as well. Thanks for fixing this, Laurent.

 Let's wait a couple more days in case Net or Swing folks want to
 review the fix. After that I'll push it to the repository.

 --
 best regards,
 Anthony


 On 4/2/2013 15:35, Laurent Bourgès wrote:

 Here is the updated patch:
  http://jmmc.fr/~bourgesl/__share/webrev-8010297.2/
 


 Fixed inconsistencies between FINE / FINER log statements:

Re: Looking for a sponsor to review changes made to two unit tests under jdk/test

[Mani Sarkar]

Hi David,

Here's the version of
*jdk8_tl/jdk/**test/java/lang/ref/Basic.java*implemented using a
Semaphore:

x-
diff -r 16f63a94c231 test/java/lang/ref/Basic.java
--- a/test/java/lang/ref/Basic.java Fri Apr 05 18:20:12 2013 -0700
+++ b/test/java/lang/ref/Basic.java Sun Apr 07 11:06:17 2013 +0100
@@ -29,7 +29,7 @@

 import java.lang.ref.*;
 import java.util.Vector;
-
+import java.util.concurrent.Semaphore;

 public class Basic {

@@ -71,10 +71,11 @@
 });
 }

-static void createNoise() throws InterruptedException {
+static void createNoise(final Semaphore complete) throws
InterruptedException {
 fork(new Runnable() {
 public void run() {
 keep.addElement(new PhantomReference(new Object(), q2));
+complete.release();
 }
 });
 }
@@ -101,9 +102,12 @@
 for (int i = 1;; i++) {
 Reference r;

-createNoise();
-System.err.println("GC " + i);
-Thread.sleep(10);
+final Semaphore noiseComplete = new Semaphore(1);
+noiseComplete.acquire();
+
+createNoise(noiseComplete);
+
+System.err.println("GC " + i);
 System.gc();
 System.runFinalization();
 x-

After running the test the first time I added temporary logging statements
between the blocks of code that dealt with the Semaphore and got the below
results:

*--System.out:(12/444)--*
*> Semaphore created.*
*> Semaphore acquired.*
*>> Inside createnoise(): before adding an element.*
*>> Inside createnoise(): after adding an element.*
*>> Inside createnoise(): Semaphore released.*
*> After createnoise() is called.*
*> Semaphore created.*
*> Semaphore acquired.*
*>> Inside createnoise(): before adding an element.*
*>> Inside createnoise(): after adding an element.*
*>> Inside createnoise(): Semaphore released.*
*> After createnoise() is called.*

What is the principle difference between CountdownLatch and Semaphore and
why is a later more applicable in this context than the former? The
mehanism by which they handle the two threads appears the same to me, could
you please explain to satisfy my concurrency curiosity.

Regards,
mani

On Mon, Apr 8, 2013 at 3:00 AM, David Holmes wrote:

> On 7/04/2013 9:37 AM, Mani Sarkar wrote:
>
>> Hi David,
>>
>> Sorry for not getting back earlier. Here's the changes to
>> /jdk8_tl/jdk/test/java/lang/**ref/Basic.java that you suggested earlier.
>>
>
> Okay but what about my comment that the latch usage is completely
> unnecessary in the first place ??
>
> David
> -
>
>  ---x--**---
>>
>> diff -r 16f63a94c231 test/java/lang/ref/Basic.java
>> --- a/test/java/lang/ref/Basic.**javaFri Apr 05 18:20:12 2013 -0700
>> +++ b/test/java/lang/ref/Basic.**javaSun Apr 07 00:27:55 2013 +0100
>>
>> @@ -29,7 +29,7 @@
>>   import java.lang.ref.*;
>>   import java.util.Vector;
>> -
>> +import java.util.concurrent.**CountDownLatch;
>>   public class Basic {
>> @@ -71,10 +71,11 @@
>>   });
>>   }
>> -static void createNoise() throws InterruptedException {
>> +static void createNoise(final CountDownLatch complete) throws
>> InterruptedException {
>>   fork(new Runnable() {
>>   public void run() {
>>   keep.addElement(new PhantomReference(new Object(), q2));
>> +complete.countDown();
>>   }
>>   });
>>   }
>> @@ -101,9 +102,11 @@
>>   for (int i = 1;; i++) {
>>   Reference r;
>> -createNoise();
>> ++   CountDownLatch noiseComplete = new CountDownLatch(1);
>> ++   createNoise(noiseComplete);
>> +
>>   System.err.println("GC " + i);
>> -Thread.sleep(10);
>> +noiseComplete.await();
>>   System.gc();
>>   System.runFinalization();
>> ---x--**---
>>
>> Its still implemented with CountdownLatch, but as you suggest we
>> implement the above via Semaphore it will have to be the next version
>> from me - CountdownLatch was suggested by many at the TestFest last
>> month but personally I benefit from getting exposed to different
>> techniques. I'll back to you with a solution applied using Semaphore.
>>
>> Cheers,
>> mani
>>
>> On Fri, Apr 5, 2013 at 2:40 AM, David Holmes > > wrote:
>>
>> On 5/04/2013 11:27 AM, Mani Sarkar wrote:
>>
>> Hi David,
>>
>> I'll reply in more detail later but to respond to your comment on:
>>
>>   > I would not add the extra methods around the cdl.await() and
>> cdl.countDown() as they just obscure things
>> In general its meant to do the opposite. We come across a lot of
>> code
>> that does not express intent, and the purpose of encapsulating
>> such
>> blocks (even if its a single line)

Re: sun.awt.X11 logs still using String + (waste)

[Peter Levart]

On 04/07/2013 07:11 PM, Laurent Bourgès wrote:


Hi

I started fixing All PlatformlLogger "bad" usages and I am fixing also 
many jul Logger usages without isLoggable ...

That represents a lot of changes and a very boring job.

I think sending a webrew tomorrow.



Hi Laurent,

Since you're already deep in the task, you might have a rough feeling 
what part of such unguarded log statements are of the following type:


logger.[fine,info,...]("a message with {0} and {1} placeholders", 
someArg1, someArg2);


where someArg1, ... are some values that are already present in the 
context of the logging statement and don't have to be computed just for 
satisfying the logging (not even boxing). Such usages could be 
effectively optimized by adding some overloaded methods to 
PlatformLogger that take 1, 2, 3, ... arguments:


class PlatformLogger {

...

public void finest(String msg, Object param1) {
if (isLoggable(Level.FINEST)) {
loggerProxy.doLog(Level.FINEST, msg, param1);
}
}

public void finest(String msg, Object param1, Object param2) {
if (isLoggable(Level.FINEST)) {
loggerProxy.doLog(Level.FINEST, msg, param1, param2);
}
}

public void finest(String msg, Object param1, Object param2, Object 
param3) {

if (isLoggable(Level.FINEST)) {
loggerProxy.doLog(Level.FINEST, msg, param1, param2, param3);
}
}

...


This would effectively guard creation of the arguments array with an 
isLoggable check for some common usages, eliminating the need to 
explicitly guard such statements. Of course, the user would have to be 
aware of when such unguarded logging statement is without overhead and 
when not...


How do you feel about such API extension?


Regards, Peter



Laurent

Le 4 avr. 2013 14:08, "Laurent Bourgès" > a écrit :


Ok, I can provide an updated patch after finding / fixing all usages.

Probably in 1 or 2 days,

Laurent

2013/4/4 Anthony Petrov mailto:anthony.pet...@oracle.com>>

Yes, this makes sense. Do you want to update your fix for
8010297 to include these changes as well?

--
best regards,
Anthony


On 04/04/13 15:47, Laurent Bourgès wrote:

Dear all,

I just realized there is another problem with
PlatformLogger log statements:
XBaseWindow:
 public boolean grabInput() {
 grabLog.fine("Grab input on {0}", this);
...
}

This calls the PlatformLogger.fine( varargs):
 public void fine(String msg, Object... params) {
 logger.doLog(FINE, msg, params);
 }

Doing so, the JVM creates a new Object[] instance to
provide params as
varargs.

I would recommend using isLoggable() test to avoid such
waste if the log
is disabled (fine, finer, finest ...).

Do you want me to provide a new patch related to this
problem ?

Does somebody have an idea to automatically analyze the
JDK code and
detect missing isLoggable statements ...

Regards,
Laurent

2013/4/3 Laurent Bourgès mailto:bourges.laur...@gmail.com>
>>


Anthony,

could you tell me once it is in the OpenJDK8 repository ?
I would then perform again profiling tests to check if
there is no
more missing isLoggable() statements.

Once JMX and other projects switch to PlatformLogger,
I could check
again.

Maybe I could write a small java code checker (pmd
rule) to test if
there is missing isLoggable() statements wrapping
PlatformLogger log
statements. Any idea about how to reuse java parser to
do so ?

Regards,

Laurent

2013/4/2 Anthony Petrov mailto:anthony.pet...@oracle.com>
>>


Looks fine to me as well. Thanks for fixing this,
Laurent.

Let's wait a couple more days in case Net or Swing
folks want to
review the fix. After that I'll push it to the
repository.

--
best regards,
Anthony


On 4/2/2013 15:35, Laurent Bourgès wrote:

Here is the updated patch:
http://jmmc.fr/~bourgesl/__share/webrev-8010297.2/

Re: [OpenJDK 2D-Dev] sun.java2D.pisces big memory usage (waste ?)

[Laurent Bourgès]

Andrea,

Any feedback from anybody else ?

Here are J2D Bench results:
http://jmmc.fr/~bourgesl/share/java2d-pisces/j2DBench/

Depending on the test, performance gains varies from 20% to 100% !

I think it could be nice if you can perform tests (regression and
benchmarks using MapBench, J2DBench, Java2D demos).
I still have to enhance cleanup / tuning my code (stats, initial array
sizes ...) and cache eviction (memory cleanup) using Weak references or
manual cleanup using timestamps ...

To test regression, I propose you to enhance your MapBench class to save
image results (randomly in tests) and compare them after the test run
(image diff) even when multiple threads are in use.

If you apply my patch, take care of the following main settings:
useThreadLocal should be disabled in a web container (to avoid too much
memory waste): a RendererContext represents ~ 2Mb:
rowAARLE = new int[INITIAL_Y_ARRAY][INITIAL_ARRAY]; // ~2Mb +1 to avoid
recycling such shared arrays

PiscesConst:
/** use ThreadLocal or ConcurrentLinkedQueue to get the thread
RendererContext */
static final boolean useThreadLocal = true;

Initial RendererContext array capacity:
// +1 to avoid recycling such shared arrays
static final int INITIAL_ARRAY = 256 + 1;
static final int INITIAL_Y_ARRAY = 2048 + 1;
static final int INITIAL_LARGE_ARRAY = 16384 + 1; // large

Laurent

2013/4/7 Andrea Aime 

> On Fri, Apr 5, 2013 at 4:32 PM, Laurent Bourgès  > wrote:
>
>> Dear all,
>>
>> Here is my first pisces (beta) patch (webrev):
>> http://jmmc.fr/~bourgesl/share/java2d-pisces/webrev-1/
>>
>> I succeed in fixing memory usage by having only 1 pisces instance
>> (Renderer, stroker, iterator ...) per RendererContext (GC friendly).
>>
>> Impressive results between small and large drawings:
>>
>
> Good stuff. Is there anything I can do to help? I do have an up to date
> version of JDK 8 sources on my disk, maybe I could
> try to apply the patch and take it for a spin in a real GeoServer and see
> how it goes.
>
> By the way, wondering, is there any other benchmark to try out?
> A possibly interesting one is this, where the author re-coded some
> selected methods of Graphics2D for maximum performance
> (but sometimes lower antialiasing quality) and created a applet to compare
> the results in a number of synthetic test cases:
> http://www.randelshofer.ch/oop/graphics/
>
> Cheers
> Andrea
>
>
> --
> ==
> Our support, Your Success! Visit http://opensdi.geo-solutions.it for more
> information.
> ==
>
> Ing. Andrea Aime
> @geowolf
> Technical Lead
>
> GeoSolutions S.A.S.
> Via Poggio alle Viti 1187
> 55054  Massarosa (LU)
> Italy
> phone: +39 0584 962313
> fax: +39 0584 1660272
> mob: +39  339 8844549
>
> http://www.geo-solutions.it
> http://twitter.com/geosolutions_it
>
> ---
>

JDK-8011653: Upgrade to JAXP 1.5

[huizhe wang]

Hi Lance, Alan,

As I mentioned, I'd like to propose an integration of JAXP 1.5 into 
JDK8.  JAXP 1.5 adds a new feature to control connections.


Here's the webrev:
http://cr.openjdk.java.net/~joehw/jdk8/8011653/webrev/

Thanks,
Joe