Skip to main content
added 1340 characters in body
Source Link
Eric Stein
  • 6.7k
  • 14
  • 19

-- Working off of @rofl's other suggestion, this is 25% faster on my machine than your fast method, including the time to populate the map. It does consume more memory, and it eagerly maps numbers which may not be used. It is less elegant than @rofl's suggestion, and possibly slower, but I find it easier to read.

private static final String[] PAD_ZEROS =
        new String[] {
            "00000000",
            "0000000",
            "000000",
            "00000",
            "0000",
            "000",
            "00",
            "0",
            ""
        };

private static final Map<String, String> MAP =
        new HashMap<String, String>(256);

private static void populateMap() {
    for (int i = 0; i < 256; i++) {
        final String binaryString = Integer.toBinaryString(i);
        MAP.put(
            Integer.toString(i), 
            PAD_ZEROS[binaryString.length()] + binaryString);
    }
}

public static String faster(final String ip) {
    final StringBuilder result = new StringBuilder(32);
    final String octals[] = ip.split("\\.");
    for (final String octal: octals) {
        result.append(MAP.get(octal));
    }
    return result.toString();
}

-- Working off of @rofl's other suggestion, this is 25% faster on my machine than your fast method, including the time to populate the map. It does consume more memory, and it eagerly maps numbers which may not be used. It is less elegant than @rofl's suggestion, and possibly slower, but I find it easier to read.

private static final String[] PAD_ZEROS =
        new String[] {
            "00000000",
            "0000000",
            "000000",
            "00000",
            "0000",
            "000",
            "00",
            "0",
            ""
        };

private static final Map<String, String> MAP =
        new HashMap<String, String>(256);

private static void populateMap() {
    for (int i = 0; i < 256; i++) {
        final String binaryString = Integer.toBinaryString(i);
        MAP.put(
            Integer.toString(i), 
            PAD_ZEROS[binaryString.length()] + binaryString);
    }
}

public static String faster(final String ip) {
    final StringBuilder result = new StringBuilder(32);
    final String octals[] = ip.split("\\.");
    for (final String octal: octals) {
        result.append(MAP.get(octal));
    }
    return result.toString();
}
added 162 characters in body
Source Link
Eric Stein
  • 6.7k
  • 14
  • 19

I don't have access to your test file, but try:

private static final String[] PAD_ZEROS= 
    new String[] {
        "00000000", 
        "0000000", 
        "000000", 
        "00000", 
        "0000", 
        "000", 
        "00", 
        "0", 
        ""
    };

   public static String fast(final String ip) {
        final StringBuilder result = new StringBuilder();
        final String octals[] = ip.split("\\.");
        for (final String octal: octals) {    
            final String binaryString = Integer.toBinaryString(Integer.parseInt(octal));
            result.append(PAD_ZEROS[binaryString.length()]);
            result.append(binaryString);
        }
        return result.toString();
    }

In general, I'd suggest avoiding hungarian notation (bStringBuilder), unnecessary abbreviations(binString), and magic numbers (if you keep the 8, it should be in a private static final class-level variable).

Note that your performance testing is flawed. The problem is the VM performs internal optimizations as it runs which can improve the performance of the system. You should test either fast or slow, but not both, in one execution. Don't test the same string over and over again, but pull in your routing file. Then, unless your whole application is to run this one method on the file and then stop, you should run for a while, throw out that data, then start measuring. Maybe do 2 executions of the file, but only time the second one. That imitates the method being called from a live system that's been running a while, instead of a cold system.

Two more things: as @rofl said, presize the StringBuilder. Also pull the System.out.println() out of your method - it's an artificial drag on the performance.

I don't have access to your test file, but try:

private static final String[] PAD_ZEROS= 
    new String[] {
        "00000000", 
        "0000000", 
        "000000", 
        "00000", 
        "0000", 
        "000", 
        "00", 
        "0", 
        ""
    };

   public static String fast(final String ip) {
        final StringBuilder result = new StringBuilder();
        final String octals[] = ip.split("\\.");
        for (final String octal: octals) {    
            final String binaryString = Integer.toBinaryString(Integer.parseInt(octal));
            result.append(PAD_ZEROS[binaryString.length()]);
            result.append(binaryString);
        }
        return result.toString();
    }

In general, I'd suggest avoiding hungarian notation (bStringBuilder), unnecessary abbreviations(binString), and magic numbers (if you keep the 8, it should be in a private static final class-level variable).

Note that your performance testing is flawed. The problem is the VM performs internal optimizations as it runs which can improve the performance of the system. You should test either fast or slow, but not both, in one execution. Don't test the same string over and over again, but pull in your routing file. Then, unless your whole application is to run this one method on the file and then stop, you should run for a while, throw out that data, then start measuring. Maybe do 2 executions of the file, but only time the second one. That imitates the method being called from a live system that's been running a while, instead of a cold system.

I don't have access to your test file, but try:

private static final String[] PAD_ZEROS= 
    new String[] {
        "00000000", 
        "0000000", 
        "000000", 
        "00000", 
        "0000", 
        "000", 
        "00", 
        "0", 
        ""
    };

   public static String fast(final String ip) {
        final StringBuilder result = new StringBuilder();
        final String octals[] = ip.split("\\.");
        for (final String octal: octals) {    
            final String binaryString = Integer.toBinaryString(Integer.parseInt(octal));
            result.append(PAD_ZEROS[binaryString.length()]);
            result.append(binaryString);
        }
        return result.toString();
    }

In general, I'd suggest avoiding hungarian notation (bStringBuilder), unnecessary abbreviations(binString), and magic numbers (if you keep the 8, it should be in a private static final class-level variable).

Note that your performance testing is flawed. The problem is the VM performs internal optimizations as it runs which can improve the performance of the system. You should test either fast or slow, but not both, in one execution. Don't test the same string over and over again, but pull in your routing file. Then, unless your whole application is to run this one method on the file and then stop, you should run for a while, throw out that data, then start measuring. Maybe do 2 executions of the file, but only time the second one. That imitates the method being called from a live system that's been running a while, instead of a cold system.

Two more things: as @rofl said, presize the StringBuilder. Also pull the System.out.println() out of your method - it's an artificial drag on the performance.

edited body
Source Link
Eric Stein
  • 6.7k
  • 14
  • 19

I don't have access to your test file, but try:

private static final String[] prefixes =PAD_ZEROS= 
    new String[] {
        "00000000", 
        "0000000", 
        "000000", 
        "00000", 
        "0000", 
        "000", 
        "00", 
        "0", 
        ""
    };

   public static String fast(final String ip) {
        final StringBuilder result = new StringBuilder();
        final String octals[] = ip.split("\\.");
        for (final String octal: octals) {    
            final String binaryString = Integer.toBinaryString(Integer.valueOfparseInt(octal));
            result.append(padZeros[binaryStringPAD_ZEROS[binaryString.length()]);
            result.append(binaryString);
        }
        return result.toString();
    }

In general, I'd suggest avoiding hungarian notation (bStringBuilder), unnecessary abbreviations(binString), and magic numbers (if you keep the 8, it should be in a private static final class-level variable).

Note that your performance testing is flawed. The problem is the VM performs internal optimizations as it runs which can improve the performance of the system. You should test either fast or slow, but not both, in one execution. Don't test the same string over and over again, but pull in your routing file. Then, unless your whole application is to run this one method on the file and then stop, you should run for a while, throw out that data, then start measuring. Maybe do 2 executions of the file, but only time the second one. That imitates the method being called from a live system that's been running a while, instead of a cold system.

I don't have access to your test file, but try:

private static final String[] prefixes = 
    new String[] {
        "00000000", 
        "0000000", 
        "000000", 
        "00000", 
        "0000", 
        "000", 
        "00", 
        "0", 
        ""
    };

   public static String fast(final String ip) {
        final StringBuilder result = new StringBuilder();
        final String octals[] = ip.split("\\.");
        for (final String octal: octals) {    
            final String binaryString = Integer.toBinaryString(Integer.valueOf(octal));
            result.append(padZeros[binaryString.length()]);
            result.append(binaryString);
        }
        return result.toString();
    }

In general, I'd suggest avoiding hungarian notation (bStringBuilder), unnecessary abbreviations(binString), and magic numbers (if you keep the 8, it should be in a private static final class-level variable).

Note that your performance testing is flawed. The problem is the VM performs internal optimizations as it runs which can improve the performance of the system. You should test either fast or slow, but not both, in one execution. Don't test the same string over and over again, but pull in your routing file. Then, unless your whole application is to run this one method on the file and then stop, you should run for a while, throw out that data, then start measuring. Maybe do 2 executions of the file, but only time the second one. That imitates the method being called from a live system that's been running a while, instead of a cold system.

I don't have access to your test file, but try:

private static final String[] PAD_ZEROS= 
    new String[] {
        "00000000", 
        "0000000", 
        "000000", 
        "00000", 
        "0000", 
        "000", 
        "00", 
        "0", 
        ""
    };

   public static String fast(final String ip) {
        final StringBuilder result = new StringBuilder();
        final String octals[] = ip.split("\\.");
        for (final String octal: octals) {    
            final String binaryString = Integer.toBinaryString(Integer.parseInt(octal));
            result.append(PAD_ZEROS[binaryString.length()]);
            result.append(binaryString);
        }
        return result.toString();
    }

In general, I'd suggest avoiding hungarian notation (bStringBuilder), unnecessary abbreviations(binString), and magic numbers (if you keep the 8, it should be in a private static final class-level variable).

Note that your performance testing is flawed. The problem is the VM performs internal optimizations as it runs which can improve the performance of the system. You should test either fast or slow, but not both, in one execution. Don't test the same string over and over again, but pull in your routing file. Then, unless your whole application is to run this one method on the file and then stop, you should run for a while, throw out that data, then start measuring. Maybe do 2 executions of the file, but only time the second one. That imitates the method being called from a live system that's been running a while, instead of a cold system.

Source Link
Eric Stein
  • 6.7k
  • 14
  • 19
Loading