Skip to content

Hoist the float conversion out of the inner loop. #10430

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

Conversation

rhettinger
Copy link
Contributor

Currently, the n and total variables get converted to floats each time they are multiplied by random(). This minor tweak does the conversion just once and gets a small speedup (approx 3%).

Lib/random.py Outdated
return [population[_int(random() * n)] for i in range(k)]
cum_weights = list(_itertools.accumulate(weights))
elif weights is not None:
raise TypeError('Cannot specify both weights and cumulative weights')
if len(cum_weights) != n:
raise ValueError('The number of weights does not match the population')
bisect = _bisect.bisect
total = cum_weights[-1]
total = cum_weights[-1] + 0.0
Copy link
Member

Choose a reason for hiding this comment

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

Maybe repeat the comment? The code above can be changed independently, and the comment can be lost.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay.

@serhiy-storchaka serhiy-storchaka removed their assignment Nov 9, 2018
@serhiy-storchaka
Copy link
Member

No need to assign PRs to me. Just add me to the list of reviewers.

@miss-islington
Copy link
Contributor

@rhettinger: Status check is done, and it's a success ✅ .

1 similar comment
@miss-islington
Copy link
Contributor

@rhettinger: Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit 0a18e05 into python:master Nov 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants