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

Performance #45

Open
TheLevti opened this issue Feb 9, 2023 · 7 comments
Open

Performance #45

TheLevti opened this issue Feb 9, 2023 · 7 comments
Assignees

Comments

@TheLevti
Copy link

TheLevti commented Feb 9, 2023

Please provide the following details.

  • Operating System: Linux
  • PHP Version: 8.1.14

Performance implications

I am using BC::roundDown() on my responses to always round down specific fields to a specific decimal place. Now let's say we have in a for me realistic scenario of 16000 BC::roundDown() operations on my response. It's fine if they are all raw and not already rounded. But what if they are already? In my case the values coming cache and other sources are already rounded to 2 decimals. So why should I run BC::roundDown() again, but at the same time I can not be sure that the source always gives me formatted values?

I did 3 different solutions with 10 million values to be round:

  1. Explode a string and check if number of chars after the dot is <= required decimals
  2. Use preg_match
  3. Just run BC::roundDown as we currently do:

Simple validation with explode takes 2.6 seconds to run.
Using preg_match takes 0.8 seconds to run
Just do what we do now and just run BC::roundDown takes 76 seconds to run

So projected on my current project, I can drop another 10 ms at least from the response generation. And in total reduce CPU time if we can add this simple validation to the library where it makes sense.

Code used for the test

$case = 2;
$testString = '1234.123';
$start = microtime(true);
for ($i = 0; $i < 16000; ++$i) {
    switch ($case) {
        case 0:
            $elements = explode('.', $testString);
            $elementsCount = count($elements);
            if ($elementsCount === 1) {
                continue 2;
            }

            if (strlen(end($elements)) <= 2) {
                // Match
            }

            break;

        case 1:
            if (preg_match("/\d+(\.\d{1,2})?$/", $testString)) {
                // Match
            }

            break;

        case 2:
            BC::roundDown($testString, 2);

            break;
    }
}
dump(microtime(true) - $start);
@TheLevti
Copy link
Author

TheLevti commented Feb 9, 2023

Besides that further optimizations are:

Do a simple string cutoff if you roundDown a positive number or roundUp a negative number.

@krowinski krowinski self-assigned this Feb 24, 2023
@krowinski
Copy link
Owner

Indeed this are cool optimization, I need to test it

@peter279k
Copy link
Contributor

The above comment is phishing and it should be removed.

@peter279k
Copy link
Contributor

peter279k commented Feb 26, 2024

@TheLevti, after looking at the BC::roundDown method, its approach is more complicated than other cases you mention.

The approach are about the BC::roundDown method is as follows:

  • Calling the static::convertScientificNotationToString method.
  • Calling the static::parseNumber method.

After checking above of these methods, the static::parseNumber takes about 0.27 seconds.
The case 0 takes about 0.00523084 seconds. and the case 1 takes about 0.002723818 seconds.

Repository owner deleted a comment from vishalgupta1987 Feb 26, 2024
Repository owner deleted a comment from vishalgupta1987 Feb 26, 2024
@TheLevti
Copy link
Author

@TheLevti, after looking at the BC::roundDown method, its approach is more complicated than other cases you mention.

The approach are about the BC::roundDown method is as follows:

  • Calling the static::convertScientificNotationToString method.
  • Calling the static::parseNumber method.

After checking above of these methods, the static::parseNumber takes about 0.27 seconds. The case 0 takes about 0.00523084 seconds. and the case 1 takes about 0.002723818 seconds.

I don't suggest to completely replace it, just for specific cases we should take shortcuts.

@TheLevti
Copy link
Author

TheLevti commented Feb 26, 2024

I am not quite sure what you conclude in the last comment :)

When you do several thousand BC operations per second, it is heavily noticeable how much CPU is required. Little optimizations showed huge differences.

Unfortunately I did not have time to properly contribute to this issue code wise. But if you agree, for sure I can find some time to contribute.

@peter279k
Copy link
Contributor

peter279k commented Feb 27, 2024

@TheLevti, thanks for your reply. I concluded that we might use other approaches to refactor the BC::roundDown method and let rounding number efficiency and optimization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants