RFR: 8178889: Move creation of AbstractChronology comparators to call sites

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR: 8178889: Move creation of AbstractChronology comparators to call sites

Claes Redestad
Hi,

in AbstractChronology we define a number of comparators which could be
moved to their
respective call site and thus be lazily initialized when needed with no
performance
penalty.

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

Testing/analysis:
- naive micro to show this doesn't cause a regression in steady
state[1], for those in
doubt
- reduces executed bytecode by ~127K and drops 5 generated classes from
startup of
applications that depend on AbstractChronology but don't use any of
these comparators
- pass all java/time tests

Thanks!

/Claes

[1]
package org.openjdk;
import org.openjdk.jmh.annotations.Benchmark;
import java.time.chrono.*;
import java.lang.reflect.*;

public class ChronoComparatorBench {

     public static Method timeLineOrder;

     static {
         try {
             timeLineOrder =
ChronoLocalDate.class.getDeclaredMethod("timeLineOrder");
         } catch (Exception e) {
             throw new RuntimeException(e);
         }
     }

     @Benchmark
     public Object getComparator() throws Exception {
         return timeLineOrder.invoke(null);
     }
}

-f 1 -tu us -bm avgt

Benchmark                            Mode  Cnt  Score   Error Units
ChronoComparatorBench.getComparator  avgt   20 5.979 ± 0.067  ns/op  #
before
ChronoComparatorBench.getComparator  avgt   20 5.975 ± 0.054  ns/op  # after
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8178889: Move creation of AbstractChronology comparators to call sites

roger riggs
Hi Claes,

Looks fine

Roger

On 4/18/2017 8:28 AM, Claes Redestad wrote:

> Hi,
>
> in AbstractChronology we define a number of comparators which could be
> moved to their
> respective call site and thus be lazily initialized when needed with
> no performance
> penalty.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8178889
> Webrev: http://cr.openjdk.java.net/~redestad/8178889/jdk.01/
>
> Testing/analysis:
> - naive micro to show this doesn't cause a regression in steady
> state[1], for those in
> doubt
> - reduces executed bytecode by ~127K and drops 5 generated classes
> from startup of
> applications that depend on AbstractChronology but don't use any of
> these comparators
> - pass all java/time tests
>
> Thanks!
>
> /Claes
>
> [1]
> package org.openjdk;
> import org.openjdk.jmh.annotations.Benchmark;
> import java.time.chrono.*;
> import java.lang.reflect.*;
>
> public class ChronoComparatorBench {
>
>     public static Method timeLineOrder;
>
>     static {
>         try {
>             timeLineOrder =
> ChronoLocalDate.class.getDeclaredMethod("timeLineOrder");
>         } catch (Exception e) {
>             throw new RuntimeException(e);
>         }
>     }
>
>     @Benchmark
>     public Object getComparator() throws Exception {
>         return timeLineOrder.invoke(null);
>     }
> }
>
> -f 1 -tu us -bm avgt
>
> Benchmark                            Mode  Cnt  Score   Error Units
> ChronoComparatorBench.getComparator  avgt   20 5.979 ± 0.067 ns/op  #
> before
> ChronoComparatorBench.getComparator  avgt   20 5.975 ± 0.054 ns/op  #
> after

Loading...