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-116984: Install mimalloc headers by oraluben · Pull Request #116985 · python/cpython

Conversation

@oraluben

@oraluben oraluben commented Mar 19, 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 PR installs mimalloc headers and allow extensions that access pycore_*.h continue working.

Such extensions need to explicitly add internal/mimalloc to its header search path.

  • Issue: Mimalloc header is not installed #116984

Sorry, something went wrong.

Uh oh!

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

All reactions
@oraluben
Install mimalloc headers
8aae7e1
@bedevere-app bedevere-app Bot added the awaiting review label Mar 19, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Mar 19, 2024
Mimalloc header is not installed #116984
Closed
@oraluben
add echo
deef17c
@oraluben
oraluben force-pushed the add-mimalloc-header branch from c0f3b47 to deef17c Compare March 19, 2024 14:34
@colesbury
colesbury requested a review from DinoV March 19, 2024 18:35
colesbury
colesbury reviewed Mar 19, 2024
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

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.

I think we should only install the headers if Python is built with mimalloc (i.e., not configured with --without-mimalloc).

Sorry, something went wrong.

Uh oh!

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

All reactions
@oraluben
oraluben requested review from corona10 and erlend-aasland as code owners March 20, 2024 00:51
@oraluben
oraluben force-pushed the add-mimalloc-header branch 4 times, most recently from 3d63aa4 to a969e35 Compare March 20, 2024 02:22
@oraluben
Install mimalloc header only when enabled
78c228b
@oraluben
oraluben force-pushed the add-mimalloc-header branch from a969e35 to 78c228b Compare March 20, 2024 02:31
@oraluben
Rename WITH_MIMALLOC to avoid different var with same name
207e87b
erlend-aasland
erlend-aasland reviewed Mar 20, 2024
View reviewed changes
Comment thread configure.ac Resolved

Uh oh!

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

@oraluben @erlend-aasland
Update configure.ac
cf7d0cf
Co-authored-by: Erlend E. Aasland <[email protected]>
@colesbury colesbury added the skip news label Mar 21, 2024
colesbury
colesbury approved these changes Mar 21, 2024
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

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.

This looks good to me, but I'll wait for other people to leave their feedback.

As written in the devguide, for users, the internal API is sometimes "the best workaround for a thorny problem", so we want it to compile successfully in an Python installation.

At the same time, we don't always prefix names with Py or _Py in the internal API, so including CPython internal headers can lead to language level name conflicts. In this case, if an extensions includes our internal headers and bundles mimalloc, they could run into problems. While not ideal, I don't think that's a reason not to make the internal headers compile at all in installations.

Sorry, something went wrong.

Uh oh!

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

All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Mar 21, 2024
@oraluben
Merge branch 'main' into add-mimalloc-header
29a8a97
@oraluben

oraluben commented Apr 23, 2024

Copy link
Copy Markdown
Contributor Author

Hi @vstinner , would you like to help landing this?

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland
erlend-aasland merged commit e17cd1f into python:main Apr 23, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Apr 23, 2024
@erlend-aasland

erlend-aasland commented Apr 23, 2024

Copy link
Copy Markdown
Contributor

Thanks @oraluben and @colesbury!

oraluben reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

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

@oraluben
oraluben deleted the add-mimalloc-header branch April 23, 2024 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aasland erlend-aasland erlend-aasland left review comments
@colesbury colesbury colesbury approved these changes
@DinoV DinoV Awaiting requested review from DinoV
@corona10 corona10 Awaiting requested review from corona10 corona10 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.

3 participants

@oraluben @erlend-aasland @colesbury
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.