GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-100240: Use a consistent implementation for freelists by colesbury · Pull Request #121934 · python/cpython

Conversation

@colesbury

@colesbury colesbury commented Jul 17, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

  • Issue: Use a principled, and consistent, implementation of freelists. #100240

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vitaldmit reacted with thumbs up emoji corona10 and johnslavik reacted with eyes emoji
All reactions
  • 👍 1 reaction
  • 👀 2 reactions
@colesbury
pythongh-100240: Use a consistent implementation for freelists
f1e7867 
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
colesbury requested review from Fidget-Spinner, corona10, ericsnowcurrently and markshannon July 17, 2024 19:08
@colesbury
colesbury requested review from 1st1, erlend-aasland and methane as code owners July 17, 2024 19:08
@bedevere-app bedevere-app Bot mentioned this pull request Jul 17, 2024
Use a principled, and consistent, implementation of freelists. #100240
Closed
@bedevere-app bedevere-app Bot added the awaiting core review label Jul 17, 2024
@colesbury

colesbury commented Jul 17, 2024

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@colesbury
Add files to MSVC project
a4dfb45
@colesbury
colesbury requested a review from a team as a code owner July 17, 2024 19:09
@colesbury colesbury added the skip news label Jul 17, 2024
colesbury added 2 commits July 17, 2024 19:13
@colesbury
Revert a few stray changes
995c306
@colesbury
Remove unused include
85453d0
@colesbury colesbury mentioned this pull request Jul 17, 2024
gh-121621: Move asyncio freelist to thread state #121915
Closed
@markshannon

markshannon commented Jul 18, 2024

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
markshannon
markshannon approved these changes Jul 18, 2024
View reviewed changes

@markshannon markshannon left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/internal/pycore_freelist.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Jul 18, 2024
corona10
corona10 approved these changes Jul 18, 2024
View reviewed changes

@corona10 corona10 left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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 :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
colesbury and others added 2 commits July 18, 2024 12:22
@colesbury @markshannon
Update Include/internal/pycore_freelist.h
a90b05f 
Co-authored-by: Mark Shannon <[email protected]>
@colesbury
Merge branch 'main' into pythongh-100240-freelist
aa8e4d5
@colesbury

colesbury commented Jul 19, 2024

Copy link
Copy Markdown
Contributor Author

!buildbot AMD64 Ubuntu NoGIL Refleaks
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented Jul 19, 2024

Copy link
Copy Markdown

🤖 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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@colesbury
colesbury merged commit 5716cc3 into python:main Jul 22, 2024
@colesbury
colesbury deleted the gh-100240-freelist branch July 22, 2024 16:08
@bedevere-app bedevere-app Bot removed the awaiting merge label Jul 22, 2024
@eendebakpt eendebakpt mentioned this pull request Nov 11, 2024
Use freelist for range object, iterator objects and other often used objects #126703
Closed
@eendebakpt eendebakpt mentioned this pull request Nov 15, 2024
gh-126868: Add freelist for compact int objects #126865
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@corona10 corona10 corona10 approved these changes
@markshannon markshannon markshannon approved these changes
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@Fidget-Spinner Fidget-Spinner Awaiting requested review from Fidget-Spinner
@methane methane Awaiting requested review from methane methane is a code owner
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@colesbury @markshannon @bedevere-bot @corona10
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.