RFR: 8315585: Optimization for new BigDecimal(String)

[Chen Liang]

On Sun, 10 Mar 2024 16:11:12 GMT, Shaojin Wen <duke at openjdk.org> wrote:

> The current BigDecimal(String) constructor calls String#toCharArray, which has a memory allocation.
> 
> 
> public BigDecimal(String val) {
>     this(val.toCharArray(), 0, val.length()); // allocate char[]
> }
> 
> 
> When the length is greater than 18, create a char[]
> 
> 
> boolean isCompact = (len <= MAX_COMPACT_DIGITS); // 18
> if (!isCompact) {
>     // ...
> } else {
>     char[] coeff = new char[len]; // allocate char[]
>     // ...
> }
> 
> 
> This PR eliminates the two memory allocations mentioned above, resulting in an approximate 60% increase in performance..

Good idea! Since we are now maintaining two code paths, when we update one, we might forget about the other; is it possible to create another interal constructor that takes a `CharSequence`, so we can pass the string in directly, and we pass the `char[]` via `CharBuffer.wrap()`?

Hmm, `CharBuffer.wrap()` only allocates a wrapper without copying the passed char array argument. Say we have something like this:

public BigDecimal(String val) {
    this((CharSequence) val, 0, val.length()); // cast
}

public BigDecimal(char[] val) {
    this(CharBuffer.wrap(val), 0, val.length); // allocate a wrapper without copying val
}

private BigDecimal(CharSequence seq, int start, int end) {
    // actual code
}


Can't escape analysis eliminate the `CharBuffer` wrapper here so that each char array access via `seq.charAt` becomes much slower? I know profile pollution from 2 implementations of `CharSequence` can affect JIT, but I don't think that's a good reason to duplicate a lot of code, which is bug prone.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/18177#issuecomment-1987352817
PR Comment: https://git.openjdk.org/jdk/pull/18177#issuecomment-1987480225