1
\$\begingroup\$

The code below comes from the Arm_const class of my Android disassembler project:

// ARM condition code

public static final int ARM_CC_INVALID = 0;
public static final int ARM_CC_EQ = 1;
public static final int ARM_CC_NE = 2;
public static final int ARM_CC_HS = 3;
public static final int ARM_CC_LO = 4;
public static final int ARM_CC_MI = 5;
public static final int ARM_CC_PL = 6;
public static final int ARM_CC_VS = 7;
public static final int ARM_CC_VC = 8;
public static final int ARM_CC_HI = 9;
public static final int ARM_CC_LS = 10;
public static final int ARM_CC_GE = 11;
public static final int ARM_CC_LT = 12;
public static final int ARM_CC_GT = 13;
public static final int ARM_CC_LE = 14;
public static final int ARM_CC_AL = 15;

public static String getCCName(int cc)
{
    Class clazz=Arm_const.class;
    Field[] fields=clazz.getFields();
    for(Field f:fields)
    {
        String s=f.getName();
        if(s.contains("ARM_CC_"))
        try
        {
            if (((int)f.get(null))==cc)
            {
                return s.replace("ARM_CC_","");
            }
        }
        catch (IllegalAccessException e)
        {
            Log.e("arm","",e);
        }
        catch (IllegalArgumentException e)
        {}
    }
    return "";
}

The method getCCName returns the name of a constant from the declared public static final ints.

Examples:

  • 3 → "HS"

  • 12 → "LT"

Any suggestions to improve its performance (speed) are appreciated!

\$\endgroup\$
3
\$\begingroup\$

If performance is your concern I would implement getCCName as a lookup in a map that's filled when the class is loaded, e.g. like

private static final Map<Integer, String> _int2string;

static
{
    final Map<Integer, String> int2string = new HashMap<>();

    try
    {
        for (Field field: Arm_const.class.getFields())
        {
            final int mod = field.getModifiers();

            if (!int.class.equals(field.getType()))
                continue;

            if (!Modifier.isStatic(mod) || !Modifier.isPublic(mod))
                continue;

            if (!field.getName().startsWith("ARM_CC_"))
                continue;

            int2string.put(field.getInt(null),
                           field.getName().substring("ARM_CC_".length()));
        }
    }
    catch (IllegalAccessException l_e)
    {
        throw new RuntimeException(l_e); // should not occur
    }

    _int2string = Collections.unmodifiableMap(int2string);
}

public static String getCCName(int cc)
{
    return _int2string.get(cc);
}
\$\endgroup\$
  • \$\begingroup\$ Thanks! What is difference between normal map and unmodifiableMap? \$\endgroup\$ – KYHSGeekCode Sep 30 '18 at 22:29
  • \$\begingroup\$ "unmodifiableMap - Returns an unmodifiable view of the specified map. This method allows modules to provide users with "read-only" access to internal maps. Query operations on the returned map "read through" to the specified map, and attempts to modify the returned map, whether direct or via its collection views, result in an UnsupportedOperationException." docs.oracle.com/javase/8/docs/api/java/util/… \$\endgroup\$ – aventurin Sep 30 '18 at 22:37
2
\$\begingroup\$

Why are you using reflection at all? You should be taking advantage of enums. Also, the huge ARM_const class should be broken up into smaller classes. (Call ARMConstants.Condition.nameForCode(cc) instead of ARM_const.getCCName(cc).)

public class ARMConstants {
    private ARMConstants() {}

    public static enum Condition {
        INVALID, EQ, NE, HS, LO, MI, PL, VS, VC, HI, LS, GE, LT, GT, LE, AL;

        public int code() {
            return this.ordinal();
        }

        public static String nameForCode(int cc) {
            try {
                return values()[cc].name();
            } catch (ArrayIndexOutOfBoundsException noSuchCode) {
                Log.e("Disassembler", "", noSuchCode);
                return "";
            }
        }
    }

}
\$\endgroup\$
  • \$\begingroup\$ Good. But the reason why I was using public static final int was it was from an opensource project capstone(the content of link of arm_const.java is copied from there) Anyway, I didn't know this.ordinal(). Thank you for teaching that. \$\endgroup\$ – KYHSGeekCode Oct 1 '18 at 0:46
  • \$\begingroup\$ Doesn't value have performance hit? stackoverflow.com/a/27408871/8614565 I may have to cache it. \$\endgroup\$ – KYHSGeekCode Oct 1 '18 at 5:40
  • \$\begingroup\$ Feel free to cache the result of values() in a static initializer block. \$\endgroup\$ – 200_success Oct 1 '18 at 5:42
  • \$\begingroup\$ An enum valueOf is massive more performant than reflection. On could have a Map<String, Condition>, maybe as general solution: an interface FastEnumerable with a default method replacing valueOf. Not worth the trouble at this stage. \$\endgroup\$ – Joop Eggen Oct 1 '18 at 8:09
0
\$\begingroup\$

I tried performance test to determine which to accept as an answer and got a surprising result.

Exexution time gap gets smaller as I execute the test again.

Test code:

Main:

import java.util.*;
public class Main
{
    public static void main(String[] args)
    {
        long time;
        long lapse1,lapse2;
        time=System.currentTimeMillis();
        //lapse1=time;
        //System.out.println("time"+time);
        for(int i=0;i<15;++i)
            System.out.println(ARMConstants.Condition.nameForCode(i));
        lapse1=System.currentTimeMillis()-time;
        time=System.currentTimeMillis();
        //System.out.println("lapse1"+System.currentTimeMillis());
        for(int i=0;i<15;++i)
            System.out.println(MapConst.getCCName(i));
            lapse2=System.currentTimeMillis()-time;
        //System.out.println("time"+System.currentTimeMillis());
        System.out.println("lapse1: "+lapse1+"lapse2:"+lapse2);
    }
}

ARMConstants:

public class ARMConstants {
    private ARMConstants() {}

    public static enum Condition {
        INVALID, EQ, NE, HS, LO, MI, PL, VS, VC, HI, LS, GE, LT, GT, LE, AL;

        public int code() {
            return this.ordinal();
        }

        public static String nameForCode(int cc) {
            try {
                return values()[cc].name();
            } catch (ArrayIndexOutOfBoundsException noSuchCode) {
               // Log.e("Disassembler", "", noSuchCode);
                return "";
            }
        }
    }

}

MapConst:

import java.lang.reflect.*;
import java.util.*;

public class MapConst
{
    private static final Map<Integer, String> _int2string;
    public static final int ARM_CC_INVALID = 0;
    public static final int ARM_CC_EQ = 1;
    public static final int ARM_CC_NE = 2;
    public static final int ARM_CC_HS = 3;
    public static final int ARM_CC_LO = 4;
    public static final int ARM_CC_MI = 5;
    public static final int ARM_CC_PL = 6;
    public static final int ARM_CC_VS = 7;
    public static final int ARM_CC_VC = 8;
    public static final int ARM_CC_HI = 9;
    public static final int ARM_CC_LS = 10;
    public static final int ARM_CC_GE = 11;
    public static final int ARM_CC_LT = 12;
    public static final int ARM_CC_GT = 13;
    public static final int ARM_CC_LE = 14;
    public static final int ARM_CC_AL = 15;

    static
    {
        final Map<Integer, String> int2string = new HashMap<>();

        try
        {
            for (Field field: MapConst.class.getFields())
            {
                final int mod = field.getModifiers();

                if (!int.class.equals(field.getType()))
                    continue;

                if (!Modifier.isStatic(mod) || !Modifier.isPublic(mod))
                    continue;

                if (!field.getName().startsWith("ARM_CC_"))
                    continue;

                int2string.put(field.getInt(null),
                               field.getName().substring("ARM_CC_".length()));
            }
        }
        catch (IllegalAccessException l_e)
        {
            throw new RuntimeException(l_e); // should not occur
        }

        _int2string = Collections.unmodifiableMap(int2string);
    }

    public static String getCCName(int cc)
    {
        return _int2string.get(cc);
    }
}

Result:

enter image description here

enter image description here

enter image description here

Odd!

And more odd thing happened!

I added my code.

for(int i=0;i<15;++i)
            System.out.println(MapConst.getCCNameMine(i));
        lapse3=System.currentTimeMillis()-time;

        //System.out.println("time"+System.currentTimeMillis());
        System.out.println("lapse1: "+lapse1+"lapse2:"+lapse2+"lapse3:"+lapse3);

public static String getCCNameMine(int cc)
    {
        Class clazz=MapConst.class;
        Field[] fields=clazz.getFields();
        for(Field f:fields)
        {
            String s=f.getName();
            if(s.contains("ARM_CC_"))
                try
                {
                    if (((int)f.get(null))==cc)
                    {
                        return s.replace("ARM_CC_","");
                    }
                }
                catch (IllegalAccessException e)
                {
                    //Log.e("Disassembler","",e);
                }
                catch (IllegalArgumentException e)
                {
                    //Log.e("Disassembler","",e);
                }
        }
        return "";
    }

enter image description here

Mine is fastest...

Edit

When I change System. currentTimeMillis to nanoTime:

lapse1: 5432129lapse2:3326416lapse3:9307861

So using enum is fastest.

Nono, I didn't check working performance nicely.

I may have test a lot more times;;

100 iteration:

lapse1: 293518068 lapse2:281768798 lapse3:433898927

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.