Skip to content

More BCMath performance improvements #14106

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.

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

Merged
merged 7 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 18 additions & 26 deletions ext/bcmath/bcmath.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ PHP_FUNCTION(bcadd)
zend_string *left, *right;
zend_long scale_param;
bool scale_param_is_null = 1;
bc_num first = NULL, second = NULL, result;
bc_num first = NULL, second = NULL, result = NULL;
int scale;

ZEND_PARSE_PARAMETERS_START(2, 3)
Expand All @@ -167,8 +167,6 @@ PHP_FUNCTION(bcadd)
scale = (int) scale_param;
}

bc_init_num(&result);

if (php_str2num(&first, ZSTR_VAL(left)) == FAILURE) {
zend_argument_value_error(1, "is not well-formed");
goto cleanup;
Expand All @@ -179,9 +177,9 @@ PHP_FUNCTION(bcadd)
goto cleanup;
}

bc_add (first, second, &result, scale);
result = bc_add (first, second, scale);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
result = bc_add (first, second, scale);
bc_num result = bc_add (first, second, scale);

Maybe can declare and initialize it at the same time and remove the result from the cleanup.

But I'm not sure if it affects performance.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. I expect this will only make the failure path potentially faster, as it can avoid a call then to the freeing. For the success path I don't think it matters, but I guess feel free to try.


RETVAL_STR(bc_num2str_ex(result, scale));
RETVAL_NEW_STR(bc_num2str_ex(result, scale));

cleanup: {
bc_free_num(&first);
Expand All @@ -197,7 +195,7 @@ PHP_FUNCTION(bcsub)
zend_string *left, *right;
zend_long scale_param;
bool scale_param_is_null = 1;
bc_num first = NULL, second = NULL, result;
bc_num first = NULL, second = NULL, result = NULL;
int scale;

ZEND_PARSE_PARAMETERS_START(2, 3)
Expand All @@ -216,8 +214,6 @@ PHP_FUNCTION(bcsub)
scale = (int) scale_param;
}

bc_init_num(&result);

if (php_str2num(&first, ZSTR_VAL(left)) == FAILURE) {
zend_argument_value_error(1, "is not well-formed");
goto cleanup;
Expand All @@ -228,9 +224,9 @@ PHP_FUNCTION(bcsub)
goto cleanup;
}

bc_sub (first, second, &result, scale);
result = bc_sub (first, second, scale);

RETVAL_STR(bc_num2str_ex(result, scale));
RETVAL_NEW_STR(bc_num2str_ex(result, scale));

cleanup: {
bc_free_num(&first);
Expand All @@ -246,7 +242,7 @@ PHP_FUNCTION(bcmul)
zend_string *left, *right;
zend_long scale_param;
bool scale_param_is_null = 1;
bc_num first = NULL, second = NULL, result;
bc_num first = NULL, second = NULL, result = NULL;
int scale;

ZEND_PARSE_PARAMETERS_START(2, 3)
Expand All @@ -265,8 +261,6 @@ PHP_FUNCTION(bcmul)
scale = (int) scale_param;
}

bc_init_num(&result);

if (php_str2num(&first, ZSTR_VAL(left)) == FAILURE) {
zend_argument_value_error(1, "is not well-formed");
goto cleanup;
Expand All @@ -277,9 +271,9 @@ PHP_FUNCTION(bcmul)
goto cleanup;
}

bc_multiply (first, second, &result, scale);
result = bc_multiply (first, second, scale);

RETVAL_STR(bc_num2str_ex(result, scale));
RETVAL_NEW_STR(bc_num2str_ex(result, scale));

cleanup: {
bc_free_num(&first);
Expand Down Expand Up @@ -331,7 +325,7 @@ PHP_FUNCTION(bcdiv)
goto cleanup;
}

RETVAL_STR(bc_num2str_ex(result, scale));
RETVAL_NEW_STR(bc_num2str_ex(result, scale));

cleanup: {
bc_free_num(&first);
Expand Down Expand Up @@ -383,7 +377,7 @@ PHP_FUNCTION(bcmod)
goto cleanup;
}

RETVAL_STR(bc_num2str_ex(result, scale));
RETVAL_NEW_STR(bc_num2str_ex(result, scale));

cleanup: {
bc_free_num(&first);
Expand Down Expand Up @@ -454,7 +448,7 @@ PHP_FUNCTION(bcpowmod)
zend_throw_exception_ex(zend_ce_division_by_zero_error, 0, "Modulo by zero");
goto cleanup;
case OK:
RETVAL_STR(bc_num2str_ex(result, scale));
RETVAL_NEW_STR(bc_num2str_ex(result, scale));
break;
EMPTY_SWITCH_DEFAULT_CASE();
}
Expand Down Expand Up @@ -518,7 +512,7 @@ PHP_FUNCTION(bcpow)

bc_raise(first, exponent, &result, scale);

RETVAL_STR(bc_num2str_ex(result, scale));
RETVAL_NEW_STR(bc_num2str_ex(result, scale));

cleanup: {
bc_free_num(&first);
Expand Down Expand Up @@ -558,7 +552,7 @@ PHP_FUNCTION(bcsqrt)
}

if (bc_sqrt (&result, scale) != 0) {
RETVAL_STR(bc_num2str_ex(result, scale));
RETVAL_NEW_STR(bc_num2str_ex(result, scale));
} else {
zend_argument_value_error(1, "must be greater than or equal to 0");
}
Expand Down Expand Up @@ -617,21 +611,19 @@ PHP_FUNCTION(bccomp)
static void bcfloor_or_bcceil(INTERNAL_FUNCTION_PARAMETERS, bool is_floor)
{
zend_string *numstr;
bc_num num = NULL, result;
bc_num num = NULL, result = NULL;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_STR(numstr)
ZEND_PARSE_PARAMETERS_END();

bc_init_num(&result);

if (php_str2num(&num, ZSTR_VAL(numstr)) == FAILURE) {
zend_argument_value_error(1, "is not well-formed");
goto cleanup;
}

bc_floor_or_ceil(num, is_floor, &result);
RETVAL_STR(bc_num2str_ex(result, 0));
result = bc_floor_or_ceil(num, is_floor);
RETVAL_NEW_STR(bc_num2str_ex(result, 0));

cleanup: {
bc_free_num(&num);
Expand Down Expand Up @@ -692,7 +684,7 @@ PHP_FUNCTION(bcround)
}

bc_round(num, precision, mode, &result);
RETVAL_STR(bc_num2str_ex(result, result->n_scale));
RETVAL_NEW_STR(bc_num2str_ex(result, result->n_scale));

cleanup: {
bc_free_num(&num);
Expand Down
6 changes: 2 additions & 4 deletions ext/bcmath/libbcmath/src/add.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
N1 is added to N2 and the result placed into RESULT. SCALE_MIN
is the minimum scale for the result. */

void bc_add(bc_num n1, bc_num n2, bc_num *result, size_t scale_min)
bc_num bc_add(bc_num n1, bc_num n2, size_t scale_min)
{
bc_num sum = NULL;

Expand Down Expand Up @@ -67,7 +67,5 @@ void bc_add(bc_num n1, bc_num n2, bc_num *result, size_t scale_min)
}
}

/* Clean up and return. */
bc_free_num (result);
*result = sum;
return sum;
}
33 changes: 28 additions & 5 deletions ext/bcmath/libbcmath/src/bcmath.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,12 @@ bc_num _bc_new_num_ex(size_t length, size_t scale, bool persistent);

void _bc_free_num_ex(bc_num *num, bool persistent);

bc_num bc_copy_num(bc_num num);
/* Make a copy of a number! Just increments the reference count! */
static inline bc_num bc_copy_num(bc_num num)
{
num->n_refs++;
return num;
}

void bc_init_num(bc_num *num);

Expand All @@ -110,19 +115,37 @@ bool bc_is_near_zero(bc_num num, size_t scale);

bool bc_is_neg(bc_num num);

void bc_add(bc_num n1, bc_num n2, bc_num *result, size_t scale_min);
bc_num bc_add(bc_num n1, bc_num n2, size_t scale_min);

void bc_sub(bc_num n1, bc_num n2, bc_num *result, size_t scale_min);
#define bc_add_ex(n1, n2, result, scale_min) do { \
bc_num add_ex = bc_add(n1, n2, scale_min); \
bc_free_num (result); \
*(result) = add_ex; \
} while (0)

void bc_multiply(bc_num n1, bc_num n2, bc_num *prod, size_t scale);
bc_num bc_sub(bc_num n1, bc_num n2, size_t scale_min);

#define bc_sub_ex(n1, n2, result, scale_min) do { \
bc_num sub_ex = bc_sub(n1, n2, scale_min); \
bc_free_num (result); \
*(result) = sub_ex; \
} while (0)

bc_num bc_multiply(bc_num n1, bc_num n2, size_t scale);

#define bc_multiply_ex(n1, n2, result, scale_min) do { \
bc_num mul_ex = bc_multiply(n1, n2, scale_min); \
bc_free_num (result); \
*(result) = mul_ex; \
} while (0)

bool bc_divide(bc_num n1, bc_num n2, bc_num *quot, int scale);

bool bc_modulo(bc_num num1, bc_num num2, bc_num *resul, size_t scale);

bool bc_divmod(bc_num num1, bc_num num2, bc_num *quo, bc_num *rem, size_t scale);

void bc_floor_or_ceil(bc_num num, bool is_floor, bc_num *result);
bc_num bc_floor_or_ceil(bc_num num, bool is_floor);

void bc_round(bc_num num, zend_long places, zend_long mode, bc_num *result);

Expand Down
16 changes: 16 additions & 0 deletions ext/bcmath/libbcmath/src/convert.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

#include "bcmath.h"
#include "convert.h"
#ifdef __SSE2__
# include <emmintrin.h>
#endif

/* This will be 0x01010101 for 32-bit and 0x0101010101010101 */
#define SWAR_ONES (~((size_t) 0) / 0xFF)
Expand All @@ -31,6 +34,19 @@ static char *bc_copy_and_shift_numbers(char *restrict dest, const char *source,
shift = -shift;
}

#ifdef __SSE2__
/* SIMD SSE2 bulk shift + copy */
__m128i shift_vector = _mm_set1_epi8(shift);
while (source + sizeof(__m128i) <= source_end) {
__m128i bytes = _mm_loadu_si128((const __m128i *) source);
bytes = _mm_add_epi8(bytes, shift_vector);
_mm_storeu_si128((__m128i *) dest, bytes);

source += sizeof(__m128i);
dest += sizeof(__m128i);
}
#endif

/* Handle sizeof(size_t) (i.e. 4/8) bytes at once.
* We know that adding/subtracting an individual byte cannot overflow,
* so it is possible to add/subtract an entire word of bytes at once
Expand Down
4 changes: 2 additions & 2 deletions ext/bcmath/libbcmath/src/divmod.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ bool bc_divmod(bc_num num1, bc_num num2, bc_num *quot, bc_num *rem, size_t scale
if (quot) {
quotient = bc_copy_num(temp);
}
bc_multiply(temp, num2, &temp, rscale);
bc_sub(num1, temp, rem, rscale);
bc_multiply_ex(temp, num2, &temp, rscale);
bc_sub_ex(num1, temp, rem, rscale);
bc_free_num (&temp);

if (quot) {
Expand Down
25 changes: 11 additions & 14 deletions ext/bcmath/libbcmath/src/floor_or_ceil.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,19 @@
#include "private.h"
#include <stddef.h>

void bc_floor_or_ceil(bc_num num, bool is_floor, bc_num *result)
bc_num bc_floor_or_ceil(bc_num num, bool is_floor)
{
/* clear result */
bc_free_num(result);

/* Initialize result */
*result = bc_new_num(num->n_len, 0);
(*result)->n_sign = num->n_sign;
bc_num result = bc_new_num(num->n_len, 0);
result->n_sign = num->n_sign;

/* copy integer part */
memcpy((*result)->n_value, num->n_value, num->n_len);
memcpy(result->n_value, num->n_value, num->n_len);

/* If the number is positive and we are flooring, then nothing else needs to be done.
* Similarly, if the number is negative and we are ceiling, then nothing else needs to be done. */
if (num->n_scale == 0 || (*result)->n_sign == (is_floor ? PLUS : MINUS)) {
return;
if (num->n_scale == 0 || result->n_sign == (is_floor ? PLUS : MINUS)) {
return result;
}

/* check fractional part. */
Expand All @@ -46,12 +43,12 @@ void bc_floor_or_ceil(bc_num num, bool is_floor, bc_num *result)

/* If all digits past the decimal point are 0 */
if (count == 0) {
return;
return result;
}

/* Increment the absolute value of the result by 1 and add sign information */
bc_num tmp = _bc_do_add(*result, BCG(_one_), 0);
tmp->n_sign = (*result)->n_sign;
bc_free_num(result);
*result = tmp;
bc_num tmp = _bc_do_add(result, BCG(_one_), 0);
tmp->n_sign = result->n_sign;
bc_free_num(&result);
return tmp;
}
8 changes: 0 additions & 8 deletions ext/bcmath/libbcmath/src/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,6 @@ void bc_init_numbers(void)
}


/* Make a copy of a number! Just increments the reference count! */
bc_num bc_copy_num(bc_num num)
{
num->n_refs++;
return num;
}


/* Initialize a number NUM by making it a copy of zero. */
void bc_init_num(bc_num *num)
{
Expand Down
11 changes: 5 additions & 6 deletions ext/bcmath/libbcmath/src/output.c
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void bc_out_num(bc_num num, int o_base, void (*out_char)(char), bool leading_zer
int index, fdigit;
bool pre_space;
stk_rec *digits, *temp;
bc_num int_part, frac_part, base, cur_dig, t_num, max_o_digit;
bc_num int_part, base, cur_dig, t_num, max_o_digit;

/* The negative sign if needed. */
if (num->n_sign == MINUS) (*out_char)('-');
Expand Down Expand Up @@ -120,10 +120,9 @@ void bc_out_num(bc_num num, int o_base, void (*out_char)(char), bool leading_zer
digits = NULL;
bc_init_num(&int_part);
bc_divide(num, BCG(_one_), &int_part, 0);
bc_init_num(&frac_part);
bc_init_num(&cur_dig);
bc_init_num(&base);
bc_sub(num, int_part, &frac_part, 0);
bc_num frac_part = bc_sub(num, int_part, 0);
/* Make the INT_PART and FRAC_PART positive. */
int_part->n_sign = PLUS;
frac_part->n_sign = PLUS;
Expand Down Expand Up @@ -163,17 +162,17 @@ void bc_out_num(bc_num num, int o_base, void (*out_char)(char), bool leading_zer
pre_space = false;
t_num = bc_copy_num(BCG(_one_));
while (t_num->n_len <= num->n_scale) {
bc_multiply(frac_part, base, &frac_part, num->n_scale);
bc_multiply_ex(frac_part, base, &frac_part, num->n_scale);
fdigit = bc_num2long(frac_part);
bc_int2num(&int_part, fdigit);
bc_sub(frac_part, int_part, &frac_part, 0);
bc_sub_ex(frac_part, int_part, &frac_part, 0);
if (o_base <= 16) {
(*out_char)(ref_str[fdigit]);
} else {
bc_out_long(fdigit, max_o_digit->n_len, pre_space, out_char);
pre_space = true;
}
bc_multiply(t_num, base, &t_num, 0);
bc_multiply_ex(t_num, base, &t_num, 0);
}
bc_free_num (&t_num);
}
Expand Down
Loading
Loading