RFR: 8077587: BigInteger Roots [v19]

Chen Liang liach at openjdk.org
Sat Apr 26 15:35:48 UTC 2025


On Thu, 24 Apr 2025 20:18:42 GMT, fabioromano1 <duke at openjdk.org> wrote:

>> @fabioromano1 I just had a cursory glance at this PR.
>> 
>> AFAIU, there are two main contributions here:
>> 
>> - Performance enhancements in `pow()`, which is of general interest and does not require submitting a [CSR](https://wiki.openjdk.org/display/csr/Main).
>> - Introduction of a new public API point for the _n_-th root, which would require a CSR.
>> 
>> It's important to understand that if we add public API points, there must be some evidence and consensus about their general usefulness and demand for them. Every addition is a commitment for this community in terms of code maintenance, reviews, testing, documentation, so they should be evaluated with this perspective in mind.
>> 
>> In this case, I feel that the proposed _n_-th root might not be among the top priority API points to add to `BigInteger`. Perhaps the overall plan is to use this method to implement a _n_-th root in `BigDecimal` in some followup PR, but this is not stated here.
>> 
>> Anyway, a [preliminary discussion](https://openjdk.org/guide/#socialize-your-change) about the proposal should take place on the mailing list, _before_ submitting the PR and invest too much work on the code.
>> 
>> To make progress here, I suggest to split this PR in two, if technically possible:
>> 
>> - One for the enhancements in `pow`, with JMH results before/after.
>> - Another PR for the proposed _n_-th root.
>> 
>> Thanks
>
>> * Performance enhancements in `pow()`, which is of general interest and does not require submitting a [CSR](https://wiki.openjdk.org/display/csr/Main).
> 
> @rgiulietti Yes, but here, the primary enhancement in `pow()` is not concerned most on execution time, but rather in memory optimization, because the PR implementation does the "shift of the exponent" squaring the result rather than the base, so the base is not squared like in the current implementation, and this permits to save about half of the memory.
> 
>> To make progress here, I suggest to split this PR in two, if technically possible:
>> 
>> * One for the enhancements in `pow`, with JMH results before/after.
>> * Another PR for the proposed _n_-th root.
> 
> I'm not very sure if it is technically possible, because both `pow()` and `nthRoot()` requires the computation of a power of a long, so that code has to be shared by both methods, and so a supposed PR for `nthRoot()` would require that method.

@fabioromano1 In this case, we will add the common code to pow() enhancement - since it does not require a CSR, it is most likely to be integrated before nthRoot() addition, and nthRoot() can be a dependent PR on the pow() enhancement (i.e. you create a new branch against `pr/#` branch in openjdk and open pr, such a pr is a dependent)

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

PR Comment: https://git.openjdk.org/jdk/pull/24690#issuecomment-2832321224


More information about the core-libs-dev mailing list