RFR: 8315585: Optimization for new BigDecimal(String)
Chen Liang
liach at openjdk.org
Mon Mar 11 11:15:14 UTC 2024
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
More information about the core-libs-dev
mailing list