NumberUtils.createNumber() performance: Short-circuit check based on pre-derived candidate Double

[ax1nch]

This is a minor performance improvement. A codeql query I ran over some library code had picked up on the empty catch blocks in this file. Those are already handled well here of course, but I happen to notice the allocating toStrings. Since the toString based checks were only used for this condition, I thought it might be nice to find a fully a numeric-only check here too, before following through the history of this, tickets like LANG-1018

So I then tweaked the check to work with a single toString. The performance improvement was consistent, but not significant enough to justify a change. However I feel the current simple shortcircuit check might be worth it - I mean it's almost a free check (like when false), but much faster as a replacement(when true) at ~100x (relevant jmh out below) . It also feels like an apt positive-test, given the fact that the candidate Double is already available at that point

.
Result "org.ex.NumericCheck.testDefaultCheck":
  84.675 ±(99.9%) 0.949 ns/op [Average]
  (min, avg, max) = (83.505, 84.675, 86.911), stdev = 0.888
  CI (99.9%): [83.726, 85.624] (assumes normal distribution)
.
Result "org.ex.NumericCheck.testNumericPreCheck":
  0.851 ±(99.9%) 0.024 ns/op [Average]
  (min, avg, max) = (0.817, 0.851, 0.897), stdev = 0.022
  CI (99.9%): [0.827, 0.875] (assumes normal distribution)
.
.
Benchmark                         Mode  Cnt   Score   Error  Units
NumericCheck.testDefaultCheck     avgt   15  84.675 ± 0.949  ns/op
NumericCheck.testNumericPreCheck  avgt   15   0.851 ± 0.024  ns/op

Also to note: the probability of a Float return may not be high in a random distribution of the param string but this is library code and so will be used in many cases where the distribution is not random, and there it counts

[garydgregory]

Hello @ax1nch

Where is the JMH test you quote? If you don't included it here, I can't run it to replicate your experiment... TY!

[ax1nch]

Hi Gary

Sorry, missed including that last time. So below is that benchmark code I had run between the Default check and the new short-circuit check:

import org.openjdk.jmh.infra.Blackhole;
import org.openjdk.jmh.annotations.*;
import java.util.concurrent.TimeUnit;
import java.math.BigDecimal;

@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@State(Scope.Thread)
@Fork(3)
@Warmup(iterations = 3)
@Measurement(iterations = 5)
public class NumericCheck {

    private String input = "123.456789";
    //private String input = "1.1E20";
    Float f = Float.valueOf(input);
    Double d = Double.valueOf(input);

    @Benchmark
    public void testNumericPreCheck(Blackhole bh) {
        bh.consume((double) (float) d.doubleValue() == d.doubleValue());
    }

    @Benchmark
    public void testDefaultCheck(Blackhole bh) {
        bh.consume(f.toString().equals(d.toString()));
    }

}

Best, Anish

[garydgregory]

@ax1nch

This test doesn't compare anything useful. You need to compare the old code vs. the new code, so:

!f.isInfinite() && !(f.floatValue() == 0.0F && !isZero(mant, dec)) && f.toString().equals(d.toString())

versus

!f.isInfinite() && !(f.floatValue() == 0.0F && !isZero(mant, dec))
                    && ((double) d.floatValue() == d.doubleValue() || f.toString().equals(d.toString()))

or the whole of the old method vs. the whole of the new method.

[ax1nch]

@garydgregory

Sure.. Earlier I had tried to measure in isolation for when the short-circuiting code applies. I guess below is what you are looking at. The result summary and the corresponding code comparing old code vs new code:

.
.
Result "org.ex.NumericFloatBenchmark.testDefaultCheck":
  86.264 ±(99.9%) 0.854 ns/op [Average]
  (min, avg, max) = (84.942, 86.264, 87.723), stdev = 0.799
  CI (99.9%): [85.409, 87.118] (assumes normal distribution)
.
.
Result "org.ex.NumericFloatBenchmark.testShortcircuitCheck":
  1.182 ±(99.9%) 0.053 ns/op [Average]
  (min, avg, max) = (1.106, 1.182, 1.276), stdev = 0.050
  CI (99.9%): [1.129, 1.235] (assumes normal distribution)
.
.
.
Benchmark                                    Mode  Cnt   Score   Error  Units
NumericFloatBenchmark.testDefaultCheck       avgt   15  86.264 ± 0.854  ns/op
NumericFloatBenchmark.testShortcircuitCheck  avgt   15   1.182 ± 0.053  ns/op

Pls note that I've copied over couple of private methods from NumberUtils to ensure the check remains verbatim

import org.openjdk.jmh.annotations.*;
import java.util.concurrent.TimeUnit;

@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@State(Scope.Thread)
@Fork(3)
@Warmup(iterations = 3)
@Measurement(iterations = 5)
public class NumericFloatBenchmark {

    private final String str = "0.25";
    private final String mant="0";
    private final String dec="25";

    Float f = Float.valueOf(str);
    Double d = Double.valueOf(str);

    @Benchmark
    public boolean testDefaultCheck() {
        return !f.isInfinite() && !(f.floatValue() == 0.0F && !isZero(mant, dec)) && f.toString().equals(d.toString());
    }

    @Benchmark
    public boolean testShortcircuitCheck() {
        return !f.isInfinite() && !(f.floatValue() == 0.0F && !isZero(mant, dec))
                    && ((double) d.floatValue() == d.doubleValue() || f.toString().equals(d.toString()));
    }

    private static boolean isZero(final String mant, final String dec) {
        return isAllZeros(mant) && isAllZeros(dec);
    }
    private static boolean isAllZeros(final String str) {
        if (str == null) {
            return true;
        }
        for (int i = str.length() - 1; i >= 0; i--) {
            if (str.charAt(i) != '0') {
                return false;
            }
        }
        return true;
    }

[garydgregory]

@ax1nch
Thank you for the update, merged. See NumberUtilsBenchmark.