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

gh-100240: Use a consistent implementation for freelists #121934

Merged
merged 6 commits into from
Jul 22, 2024

Conversation

colesbury
Copy link
Contributor

@colesbury colesbury commented Jul 17, 2024

This combines and updates our freelist handling to use a consistent implementation. Objects in the freelist are linked together using the first word of memory block.

When Python is configured to build without freelists, these operations are essentially no-ops.

This combines and updates our freelist handling to use a consistent
implementation. Objects in the freelist are linked together using the
first word of memory block.

If configured with freelists disabled, these operations are essentially
no-ops.
@colesbury
Copy link
Contributor Author

I'm putting this up for discussion and feedback.

@markshannon, this doesn't do all the things listed in the issue -- I haven't
changed the capacity of the freelists and they're still separated by type --
but it removes a lot of duplicated code.

In my limited testing, I haven't seen a significant performance difference.

@colesbury colesbury requested a review from a team as a code owner July 17, 2024 19:09
@markshannon
Copy link
Member

Perfomance is in the noise but doesn't look like it's any worse.

Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

Very nice. +290 −700 had me sold 🙂

A lot of compiler warnings, but should be fixable with one cast in the _Py_FREELIST_SIZE macro.

Consider it approved once the macro is fixed.

Include/internal/pycore_freelist.h Outdated Show resolved Hide resolved
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

Awesome!

Consider it approved once the macro is fixed.

Same opinion :)

@colesbury
Copy link
Contributor Author

!buildbot AMD64 Ubuntu NoGIL Refleaks

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @colesbury for commit aa8e4d5 🤖

The command will test the builders whose names match following regular expression: AMD64 Ubuntu NoGIL Refleaks

The builders matched are:

  • AMD64 Ubuntu NoGIL Refleaks PR

@colesbury colesbury merged commit 5716cc3 into python:main Jul 22, 2024
35 checks passed
@colesbury colesbury deleted the gh-100240-freelist branch July 22, 2024 16:08
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.

4 participants