Skip to content

ext/bcmath: Performance improvement bcsqrt() #18771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

ext/bcmath: Performance improvement bcsqrt() #18771

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
57b91d6
local_num is not needed so it has been removed.
SakiTakamachi Jun 5, 2025
0b34dfd
Optimized and changed the initialization process to run a little later.
SakiTakamachi Jun 5, 2025
bb71577
create fast path and move old logic to standard path
SakiTakamachi Jun 7, 2025
975fcc4
Optimized standard path calculation
SakiTakamachi Jun 8, 2025
f6c50d8
Efficiently obtain the initial value for the standard path.
SakiTakamachi Jun 9, 2025
dd64d94
Omit low precision digits in calculations
SakiTakamachi Jun 10, 2025
d7d1d80
Added test cases
SakiTakamachi Jun 11, 2025
aef9c17
removed nearzero.c
SakiTakamachi Jun 11, 2025
7555b2f
Removed `bc_add_ex`, `bc_int2num`, and `bc_raise_bc_exponent` as they…
SakiTakamachi Jun 12, 2025
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
local_num is not needed so it has been removed.
  • Loading branch information
@SakiTakamachi
SakiTakamachi committed Jun 11, 2025
commit 57b91d685d6ca61259b90209a284d239c2f3cbe4
13 changes: 6 additions & 7 deletions ext/bcmath/libbcmath/src/sqrt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,19 @@

bool bc_sqrt(bc_num *num, size_t scale)
{
const bc_num local_num = *num;
/* Initial checks. */
if (bc_is_neg(local_num)) {
if (bc_is_neg(*num)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I wonder is whether this was the right move.
One the one hand, keeping *num in a local variable may save a pointer load, on the other hand it may get spilled in between calls anyway.
/* Cannot take the square root of a negative number */
return false;
}
/* Square root of 0 is 0 */
if (bc_is_zero(local_num)) {
if (bc_is_zero(*num)) {
bc_free_num (num);
*num = bc_copy_num(BCG(_zero_));
return true;
}

bcmath_compare_result num_cmp_one = bc_compare(local_num, BCG(_one_), local_num->n_scale);
bcmath_compare_result num_cmp_one = bc_compare(*num, BCG(_one_), (*num)->n_scale);
/* Square root of 1 is 1 */
if (num_cmp_one == BCMATH_EQUAL) {
bc_free_num (num);
Expand All @@ -62,7 +61,7 @@ bool bc_sqrt(bc_num *num, size_t scale)
/* Initialize the variables. */
size_t cscale;
bc_num guess, guess1, point5, diff;
size_t rscale = MAX(scale, local_num->n_scale);
size_t rscale = MAX(scale, (*num)->n_scale);

bc_init_num(&guess1);
bc_init_num(&diff);
Expand All @@ -74,13 +73,13 @@ bool bc_sqrt(bc_num *num, size_t scale)
if (num_cmp_one == BCMATH_RIGHT_GREATER) {
/* The number is between 0 and 1. Guess should start at 1. */
guess = bc_copy_num(BCG(_one_));
cscale = local_num->n_scale;
cscale = (*num)->n_scale;
} else {
/* The number is greater than 1. Guess should start at 10^(exp/2). */
bc_init_num(&guess);
bc_int2num(&guess, 10);

bc_int2num(&guess1, local_num->n_len);
bc_int2num(&guess1, (*num)->n_len);
bc_multiply_ex(guess1, point5, &guess1, 0);
guess1->n_scale = 0;
bc_raise_bc_exponent(guess, guess1, &guess, 0);
Expand Down