groovy

Improve DefaultTypeTransformation.booleanUnbox performance

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 1.9-beta-4, 1.8.4, 1.7.11
  • Component/s: None
  • Labels:
    None
  • Number of attachments :
    1

Description

In profiling a Grails application, org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation.booleanUnbox(Object) is showing up as one of the top "blockers" (groovy.lang.ExpandoMetaClass.isInitialized() is actually causing the blocking since EMC gets always used in Grails).

The booleanUnbox/castToBoolean method should be optimized for the case when the Object is already a boolean. Currently it goes threw a lot of unnecessary layers also for boolean values.

current implementation:

    public static boolean castToBoolean(Object object) {
        // null is always false
        if (object == null) {
            return false;
        }

        // if the object is not null, try to call an asBoolean() method on the object
        return (Boolean)InvokerHelper.invokeMethod(object, "asBoolean", InvokerHelper.EMPTY_ARGS);
    }

improved implementation:

    public static boolean castToBoolean(Object object) {
        // null is always false
        if (object == null) {
            return false;
        }
        if (object.getClass() == Boolean.class) {   // equality check is enough and faster than instanceof check, no need to check superclasses since Boolean is final
            return ((Boolean)object).booleanValue();
        }
        // if the object is not null, try to call an asBoolean() method on the object
        return (Boolean)InvokerHelper.invokeMethod(object, "asBoolean", InvokerHelper.EMPTY_ARGS);
    }

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Lari Hotari added a comment -

Screenshot of a YJP profiling session. It shows that 96% of thread blocking is caused by EMC.isInitialized() (GROOVY-3557) and of that 91% of is caused by DTT.booleanUnbox(Object).

Show
Lari Hotari added a comment - Screenshot of a YJP profiling session. It shows that 96% of thread blocking is caused by EMC.isInitialized() ( GROOVY-3557 ) and of that 91% of is caused by DTT.booleanUnbox(Object).
Hide
Permalink
blackdrag blackdrag added a comment -

the point though is mostly about... if a user provides an Boolean#asBoolean method, or wants to hook in the invocation of that method, is it legal for us to ignore that will and bypass it completely?

Show
blackdrag blackdrag added a comment - the point though is mostly about... if a user provides an Boolean#asBoolean method, or wants to hook in the invocation of that method, is it legal for us to ignore that will and bypass it completely?
Hide
Permalink
Lari Hotari added a comment -

I added btrace profiling to get statistics of booleanUnbox.

Here is the code for the btrace "script":

import com.sun.btrace.annotations.*;
import static com.sun.btrace.BTraceUtils.*;
import com.sun.btrace.AnyType;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;

@BTrace public class BooleanUnboxHistogram {
   private static Map<String, AtomicInteger> histo = Collections.newHashMap();
   private static AtomicInteger total = Atomic.newAtomicInteger(1);

    @OnMethod(  
        clazz="org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation",  
        method="booleanUnbox"
    )  
    public static void onBooleanUnbox(AnyType[] arg) {
	if (arg[0] != null) {
	    String cn = Reflective.name(classOf(arg[0]));
	    AtomicInteger ai = Collections.get(histo, cn);
	    if (ai == null) {
		ai = Atomic.newAtomicInteger(1);
		Collections.put(histo, cn, ai);
	    } else {
		Atomic.incrementAndGet(ai);
	    }
	    Atomic.incrementAndGet(total);
	}
    }

    @OnTimer(5000) 
    public static void print() {
        if (Collections.size(histo) != 0) {
            printNumberMap("\n\nArgument Histogram for booleanUnbox", histo);
            printNumber("Total", Atomic.get(total));
        }
    }
}

For a single real-world Grails app page refresh booleanUnbox was called 3092 times and 1995 of those were type of java.lang.Boolean.
That's 65%.

Show
Lari Hotari added a comment - I added btrace profiling to get statistics of booleanUnbox. Here is the code for the btrace "script": import com.sun.btrace.annotations.*; import static com.sun.btrace.BTraceUtils.*; import com.sun.btrace.AnyType; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; @BTrace public class BooleanUnboxHistogram { private static Map< String , AtomicInteger> histo = Collections.newHashMap(); private static AtomicInteger total = Atomic.newAtomicInteger(1); @OnMethod( clazz= "org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation" , method= "booleanUnbox" ) public static void onBooleanUnbox(AnyType[] arg) { if (arg[0] != null ) { String cn = Reflective.name(classOf(arg[0])); AtomicInteger ai = Collections.get(histo, cn); if (ai == null ) { ai = Atomic.newAtomicInteger(1); Collections.put(histo, cn, ai); } else { Atomic.incrementAndGet(ai); } Atomic.incrementAndGet(total); } } @OnTimer(5000) public static void print() { if (Collections.size(histo) != 0) { printNumberMap( "\n\nArgument Histogram for booleanUnbox" , histo); printNumber( "Total" , Atomic.get(total)); } } } For a single real-world Grails app page refresh booleanUnbox was called 3092 times and 1995 of those were type of java.lang.Boolean. That's 65%.
Hide
Permalink
Lari Hotari added a comment - - edited

I re-indexed the Grails Searchable index and looked at the statistics after that:
java.lang.Boolean instances: 118384 / 159677 = 74%.
java.lang.Number instances: 10356 / 159677 = 6,5%
java.lang.Collection instances: 10212 / 159677 = 6,4%
java.lang.String instances: 9271 / 159677 = 5,8%
Adding asBoolean bypass for all of these would give 93% coverage. (In the testcase I did the test for...)

Show
Lari Hotari added a comment - - edited I re-indexed the Grails Searchable index and looked at the statistics after that: java.lang.Boolean instances: 118384 / 159677 = 74%. java.lang.Number instances: 10356 / 159677 = 6,5% java.lang.Collection instances: 10212 / 159677 = 6,4% java.lang.String instances: 9271 / 159677 = 5,8% Adding asBoolean bypass for all of these would give 93% coverage. (In the testcase I did the test for...)
Hide
Permalink
Lari Hotari added a comment -

The old while(true) issue is related: GROOVY-3123 .

Show
Lari Hotari added a comment - The old while(true) issue is related: GROOVY-3123 .
Hide
Permalink
blackdrag blackdrag added a comment -

I was a bit unsure about if that change is legal, but I think it is now, so I committed the change. Thanks for finding this

Show
blackdrag blackdrag added a comment - I was a bit unsure about if that change is legal, but I think it is now, so I committed the change. Thanks for finding this

People

  • Assignee:
    blackdrag blackdrag
    Reporter:
    Lari Hotari
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: