Skip to content
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

ext/bcmath: Supports scientific notation #18068

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

SakiTakamachi
Copy link
Member

@SakiTakamachi SakiTakamachi commented Mar 14, 2025

related #17876

This probably requires an RFC, or at least a discussion on the mailing list, so I'll send an email once I've polished the code a bit more.

This change is to support code like this:

var_dump(
    new BcMath\Number('1.23e1'),
    new BcMath\Number('4.567E4'),
    new BcMath\Number('8.9e-2'),
    new BcMath\Number('1.2345E-5'),
);

output:

object(BcMath\Number)#1 (2) {
  ["value"]=>
  string(4) "12.3"
  ["scale"]=>
  int(1)
}
object(BcMath\Number)#2 (2) {
  ["value"]=>
  string(5) "45670"
  ["scale"]=>
  int(0)
}
object(BcMath\Number)#3 (2) {
  ["value"]=>
  string(5) "0.089"
  ["scale"]=>
  int(3)
}
object(BcMath\Number)#4 (2) {
  ["value"]=>
  string(11) "0.000012345"
  ["scale"]=>
  int(9)
}

I've used the Number class as an easy example to follow, but functions will accept these values ​​as well.

Benchmark

This change will make it a bit slower, but that may be tolerable.
(This is just a prototype, so there is a possibility that it could be improved further.)

The performance seems to be about the same.

code:

for ($i = 0; $i < 4000000; $i++) {
    $num = new BcMath\Number('1.2345678901234567890');
}

result (first commit):

Benchmark 1: /php-dev/sapi/cli/php /mount/bc/num/1.php
  Time (mean ± σ):     352.8 ms ±   1.7 ms    [User: 342.4 ms, System: 5.1 ms]
  Range (min … max):   349.6 ms … 354.9 ms    10 runs
 
Benchmark 2: /master/sapi/cli/php /mount/bc/num/1.php
  Time (mean ± σ):     345.0 ms ±   4.2 ms    [User: 334.1 ms, System: 5.3 ms]
  Range (min … max):   341.8 ms … 352.7 ms    10 runs
 
Summary
  '/master/sapi/cli/php /mount/bc/num/1.php' ran
    1.02 ± 0.01 times faster than '/php-dev/sapi/cli/php /mount/bc/num/1.php'

result (after commit 4)

Benchmark 1: /php-dev/sapi/cli/php /mount/bc/num/1.php
  Time (mean ± σ):     343.0 ms ±   2.6 ms    [User: 331.3 ms, System: 6.2 ms]
  Range (min … max):   339.8 ms … 347.7 ms    10 runs
 
Benchmark 2: /master/sapi/cli/php /mount/bc/num/1.php
  Time (mean ± σ):     344.5 ms ±   5.6 ms    [User: 333.9 ms, System: 5.3 ms]
  Range (min … max):   338.9 ms … 357.0 ms    10 runs
 
Summary
  '/php-dev/sapi/cli/php /mount/bc/num/1.php' ran
    1.00 ± 0.02 times faster than '/master/sapi/cli/php /mount/bc/num/1.php'

{
const char *fractional_end = exponent_ptr;

/* In scientific notation, the mantissa always has one integer digit. */
Copy link
Member

Choose a reason for hiding this comment

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

This is false, e.g. 12e-3 should evaluate to 0.012.

Copy link
Member Author

Choose a reason for hiding this comment

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

@nielsdos

I remember that this is what the ISO says, but I may be remembering it wrong...

I'll fix it

goto fail;
}

/* Must be 1 <= mantissa < 10 */
Copy link
Member Author

Choose a reason for hiding this comment

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

ditto

@SakiTakamachi SakiTakamachi force-pushed the bcmath/support_scientific_notation branch 3 times, most recently from 02f10fd to 0f6e267 Compare March 16, 2025 08:27
@SakiTakamachi SakiTakamachi force-pushed the bcmath/support_scientific_notation branch 2 times, most recently from f93b82a to 679e586 Compare March 16, 2025 08:36
@SakiTakamachi SakiTakamachi force-pushed the bcmath/support_scientific_notation branch from 679e586 to e3fa32c Compare March 16, 2025 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants