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

Random api migration #263

Merged
merged 1 commit into from
Feb 1, 2019

Conversation

MathiasReker
Copy link
Contributor

No description provided.

PierreRambaud
PierreRambaud previously approved these changes Dec 26, 2018
Copy link
Contributor

@PierreRambaud PierreRambaud left a comment

Choose a reason for hiding this comment

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

👍

@Quetzacoalt91
Copy link
Member

What is the added value of this method? In a performance point of view the change is not so visible.

<?php
    $START = microtime(true);
    for ($i = 1; $i < 1000000; ++$i) {
        $j = rand(100,2000);
    }
    $END = microtime(true) - $START;
    print "Short rand() took $END seconds\n";

    $START = microtime(true);
    for ($i = 1; $i < 1000000; ++$i) {
        $j = mt_rand(100,2000);
    }
    $END = microtime(true) - $START;
    print "Short mt_rand() took $END seconds\n";
Short rand() took 0.096274852752686 seconds
Short mt_rand() took 0.09580397605896 seconds

@MathiasReker
Copy link
Contributor Author

MathiasReker commented Feb 1, 2019

mt_rand uses a better randomization algorithm (Mersenne Twist), which requires more random numbers to be known before the seed can be determined and is faster. This does not mean that mt_rand is, by definition, faster than rand is, this only means that the way the numbers are generated is faster, and appears to have no real impact on the function's performance, as you here have demonstrated. :-)

@MathiasReker
Copy link
Contributor Author

mt_rand was introduced to fix the problems that existed

@Quetzacoalt91 Quetzacoalt91 merged commit ecd421f into PrestaShop:dev Feb 1, 2019
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

Successfully merging this pull request may close these issues.

3 participants