5
\$\begingroup\$

The following code will generate a sequence of N mac addresses based on the input from user. The input must be atleast one octet to generate the sequence , ie :

input  : A0 , 5
output : A0:00:00:00:00:01 - A0:00:00:00:00:05

My actual code is working fine and I wanted to make it optimized if it could be :

public static List<String> getMacSequences(final String macAddress, final Integer count)
    {
        List<String> macList = new ArrayList<String>();
        final List<Integer> macOctets = new ArrayList<Integer>();
        String [] macSplit = macAddress.split(":");

        int incPos = macAddress.split(":").length; // need to throw exception when the length is 0
        if  (incPos >= 1)
        {
            for (int bp=0; bp <6; bp++)
            {
                if (bp >= incPos)
                {
                    macOctets.add(0);
                }
                else 
                {
                    macOctets.add(Integer.parseInt(macSplit[bp], 16));
                }
            }

            for (int i = 0 ; i<count; i++)
            {
                int lastOctet = macOctets.get(5);
                macOctets.set(5, ++lastOctet);
                for (int j = 5; j >= 0; j--)
                {               
                    if (j == 0 && macOctets.get(j) > 255)
                    {
                        Collections.fill(macOctets, 0);
                    }

                    if (macOctets.get(j) > 255)
                    {
                        macOctets.set(j, 0);
                        Integer macValue = macOctets.get(j - 1);
                        macOctets.set(j - 1, ++macValue);
                    }
                    else{
                    break;
                    }
                }
                macList.add(String.format("%02X:%02X:%02X:%02X:%02X:%02X",  macOctets.toArray()));
            }
        }
        return macList;
    }

I have done micro-benchmarking to determine the performance for generating 100 mac addresses in sequence and the result of average value for 10 sampling (100 itr/sampling) as follows (in ms) :

7.62 7.43 6.87 7.4 7.24 7.21 7.16 7.59 7.67 8.13

and the overall sampling average is : 7.432 ms

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Your code defines in the second loop: for (int j = 5; j >= 0; j--) and later you use j in this statement: macOctets.set(j - 1, ++macValue); If j==0 however, you will get an IndexOutOfBounceException as -1 is not a valid index. You avoid the exception by checking explicitely for j==0 and set the value to 0, though this check will be done for every segment of the mac address. This check therefore can be refactored out of the loop and to a later position.

Next, you split the input string twice. Why? Something like

String [] macSplit = macAddress.split(":");

int incPos = macAddress.split(":").length; // need to throw exception when the length is 0
if  (incPos >= 1) {
    ...
}

can be replaced with this:

String[] macSplit = macAddress.split(":");
if (macSplit.length >= 1) {
    ...
}

This will avoid splitting the input string multiple times.

I'd also initially fill the macOctet array with 0 values and then simply set the respective defined mac value like this:

for(int i=0; i<macSplit.length; i++)
{
    macOctets[i] = Integer.parseInt(macSplit[i], 16);
}

If I execute the below code and compare it to your original code:

public static List<String> getMacSequences(final String macAddress, final Integer count)
{
    List<String> macList = new ArrayList<String>();
    Integer[] macOctet = new Integer[] { 0, 0, 0, 0, 0, 0};
    String[] macSplit = macAddress.split(":");
    if (macSplit.length > 0) 
    {
        for (int i=0; i<macSplit.length; i++)
        {
            macOctet[i] = Integer.parseInt(macSplit[i], 16);
        }
    }
    for (int i=0 ; i<count; i++)
    {
        macOctets[5]++;
        for (int j=5; j>0; j--)
        {
            if (macOctets[j] > 255)
            {
                macOctets[j] = 0;
                macOctets[j-1]++;
            }
            else
            {
                break;
            }
        }
        if (macOctets[0] > 255)
        {
            macOctets[0] = 0;
        }
        macList.add(String.format("%02X:%02X:%02X:%02X:%02X:%02X",  macOctets));
    }
    return macList;
}

I'll get the following times for 100000 iterations on 260 generated MAC addresses:

Original - Total: 71341.440878 ms, Average: 0.71341440878 ms
Modified - Total: 70220.091388 ms, Average: 0.70220091388 ms

While not a big speed improvement, at least a little faster then the original one.

Further speed improvements

I used Integer[] macOctets = new Integer[] { ... } over a simple int[] macOctets = new int[] { ... } in the sample code above as the string formatter has some issues with int arrays for some reasons with my version of Java (Oracle Java 8 Update 101).

I then implemented my own version of MAC string converter

private static String toHexString(int[] macOctets)
{
    StringBuilder sb = new StringBuilder();
    for (int i=0; i<macOctets.length; i++)
    {
        sb.append(Integer.toHexString(0x100 | macOctets[i]).substring(1).toUpperCase());
        if (i != macOctets.length-1)
        {
            sb.append(":");
        }
    }
    return sb.toString();
}

and changed the initialization of the macOctets array to a simple

int[] macOctets = new int[6];

which internally will initialize each element with its default value, which is 0.

When re-running the comparison test described above, the following output is returned:

Original - Total: 71901.357327 ms, Average: 0.71901357327 ms
Modified - Total: 10287.282422 ms, Average: 0.10287282422 ms

The custom method makes use of a tiny hack in order to print the two byte pairs of a MAC segment.

It seems that the algorithm spends most of the time within the array to hex string formating. I'd therefore highly suggest to change from the String formatter to a custom one as this only requires a fraction of the time the Java native function requires.

For completeness I'll post the complete performance test main method so you can compare for yourself

public static void main(String ... args)
{
    int repetitions = 100000;
    String initialSequene = "A0";
    int macsToGenerate = 260;

    long startTime = System.nanoTime();
    for (int i=0; i<repetitions; i++)
    {
        getMacSequences(initialSequene, macsToGenerate);
    }
    long endTime = System.nanoTime();

    long duration = (endTime - startTime);
    double totalDurationInMs = (duration / 1000000.);
    System.out.println("Original - Total: " + totalDurationInMs + " ms, Average: " + (totalDurationInMs / repetitions) + " ms");

    startTime = System.nanoTime();
    for (int i=0; i<repetitions; i++)
    {
        getMacSequences2(initialSequene, macsToGenerate);
    }
    endTime = System.nanoTime();

    duration = (endTime - startTime);
    totalDurationInMs = (duration / 1000000.);
    System.out.println("Modified - Total: " + totalDurationInMs + " ms, Average: " + (totalDurationInMs / repetitions) + " ms");
}

where getMacSequences(...) is your version and getMacSequences2(...) is my modified version.

\$\endgroup\$
1
  • \$\begingroup\$ Kudos for the suggestion. It has reduced the execution time better than my code. \$\endgroup\$ Commented Aug 7, 2016 at 4:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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