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

bpo-37812: Make the small-int alloc optimization unconditional. #15203

Closed
wants to merge 2 commits into from

Conversation

gnprice
Copy link
Contributor

@gnprice gnprice commented Aug 10, 2019

This optimization was added in 1993 (commit 842d2cc), and we've
been running with it ever since. I think at this point it's fair
to say that it's no longer experimental. That lets us simplify
the code by removing a number of #ifdefs.

In particular this will help us simplify the usage of CHECK_SMALL_INT.

https://bugs.python.org/issue37812

This optimization was added in 1993 (commit 842d2cc), and we've
been running with it ever since.  I think at this point it's fair
to say that it's no longer experimental.  That lets us simplify
the code by removing a number of `#ifdef`s.

In particular this will help us simplify the usage of CHECK_SMALL_INT.
Copy link
Contributor

@aeros aeros left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

I had a minor suggestion for the news entry:

@@ -0,0 +1,6 @@
The "small int" optimization is now unconditionally enabled. This
optimization was introduced in Python 1.0.1, and means that :class:`int`
values from -5 to 256 are pre-initialized rather than constructed on demand.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is worthwhile to mention that this range is inclusive. Otherwise, users might assume that the maximum is 255 instead of 256, or that the minimum is -4 instead of -5. I've seen this range most commonly notated with [-5, 257), but I think adding "(inclusive)" after it is probably more appropriate and easy to understand for a news entry.

Suggested change
values from -5 to 256 are pre-initialized rather than constructed on demand.
values from -5 to 256 (inclusive) are pre-initialized rather than constructed on demand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, thanks. I don't want to make a big deal of the exact numbers because it's not something it makes sense for people to depend on -- but it's a natural question for a reader to be curious about. I made it "-5 through 256", which I think is unambiguous while staying unobtrusive.

@rhettinger rhettinger self-assigned this Aug 11, 2019
@rhettinger rhettinger closed this Aug 11, 2019
@gnprice gnprice deleted the pr-long-smallopt-always branch August 11, 2019 22:31
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.

5 participants