On Sun, 10 Mar 2024 16:11:12 GMT, Shaojin Wen <d...@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