On Mon, 11 Mar 2024 14:42:03 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> Looks great to me. Sorry for the pings, but we may need @rgiulietti to >> verify the math correctness and @cl4es to comment on whether having these 2 >> separate code paths or trying to extract a common part is the better >> approach. > >> Looks great to me. Sorry for the pings, but we may need @rgiulietti to >> verify the math correctness and @cl4es to comment on whether having these 2 >> separate code paths or trying to extract a common part is the better >> approach. > > I'd love to see less code duplication. It'd certainly be possible to refactor > to one internal constructor `BigDecimal(CharSequence)` that the > `String`-based constructors just delegates to, and then have the > `BigDecimal(char[])` variants call with some wrapping (e.g. > `this(CharBuffers.wrap(in, offset, len), mc)`). > > Such indirection might cost a few cycles per operation for the `char[]`-based > constructors due the need to wrap the `char[]`, but I wonder how performance > sensitive the `BigDecimal(char[], int, int)` paths really are once > `BigDecimal(String)` takes another path. I second @cl4es concerns about code duplication. While the µ-benchmarks show improvements, I wonder how much this impacts workloads out there. In other words, we need to contrast a performance improvement that might impact, in some unknown measure, an unknown number of real-world workloads, with visible and known code duplication, code that needs maintenance over the next many many years. IMO, @cl4es advice about code refactoring is worth giving a try, even if that costs some ns in the µ-benchmarks. ------------- PR Comment: https://git.openjdk.org/jdk/pull/18177#issuecomment-1988777526
