Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Closed
gnprice wants to merge 2 commits into
python:masterpython/cpython:masterfrom
gnprice:pr-long-smallopt-alwaysgnprice/cpython:pr-long-smallopt-alwaysCopy head branch name to clipboard
Closed

bpo-37812: Make the small-int alloc optimization unconditional.#15203
gnprice wants to merge 2 commits into
python:masterpython/cpython:masterfrom
gnprice:pr-long-smallopt-alwaysgnprice/cpython:pr-long-smallopt-alwaysCopy head branch name to clipboard

Conversation

@gnprice

@gnprice gnprice commented Aug 10, 2019

Copy link
Copy Markdown
Contributor

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.

@aeros aeros left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.