Skip to content

gh-152433: Windows: allow build mimalloc for UWP - #152477

Open
thexai wants to merge 3 commits into
python:mainfrom
thexai:uwp-prim.c
Open

gh-152433: Windows: allow build mimalloc for UWP#152477
thexai wants to merge 3 commits into
python:mainfrom
thexai:uwp-prim.c

Conversation

@thexai

@thexaithexai commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

All changes are trivial:

Remove some run-time dynamic linking and replace some LoadLibrary with LoadPackagedLibrary for UWP.

@bedevere-app

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@thexaithexai changed the title gh-152433: Windows: allow build prim.c for UWPgh-152433: Windows: allow build prim.c for UWPJul 4, 2026
Comment threadMisc/NEWS.d/next/Windows/2026-07-04-20-07-28.gh-issue-152433.Kk3gZm.rst Outdated
@chris-eibl

Copy link
Copy Markdown
Member

LGTM considering @zooba's #152433 (comment).

cc @colesbury who tracks mimalloc differences from upstream in #113141.

@zooba

Copy link
Copy Markdown
Member

This change looks fine, if we're maintaining the sources separately. If we're trying to track upstream closely, then we'll need them to make the change first. That's @colesbury's call, it seems.

@zware

Copy link
Copy Markdown
Member

Our copy of mimalloc is fairly out of date at this point, and does carry several patches against the equivalent upstream version. Regardless, I think we should be sure that this is done upstream before we merge it ourselves, or it's likely to be lost if and when we update our copy -- ideally, when we do such an update we can do it without carrying any patches forward.

@colesbury

Copy link
Copy Markdown
Contributor

Yeah, if it's not CPython specific, please also open a PR in upstream mimalloc.

@thexai

Copy link
Copy Markdown
ContributorAuthor

Patch submitted to upstream (microsoft/mimalloc#1340).

Note is a bit different because GetNumaHighestNodeNumber thing is already addressed in current version and only replaced LoadLibrary for better code portability (to very old Windows versions).

I think PR can still be merged here because it doesn't do any harm and in the future it will be overwritten by whatever is upstream.

@thexaithexai closed this Jul 24, 2026
@thexaithexai reopened this Jul 24, 2026
Comment threadObjects/mimalloc/prim/windows/prim.c Outdated
@thexai

Copy link
Copy Markdown
ContributorAuthor

The fix has been merged in the mimalloc dev branch:
microsoft/mimalloc@c9c86c7

@chris-eibl

Copy link
Copy Markdown
Member

May I trouble you to make this PR look closer to the diff that has been merged upstream?

For instance, there GetProcessMemoryInfo is still only loaded on demand - now via mi_win_loadlibrary(TEXT("psapi.dll")), but this PR currently removes the loading on demand.

@thexai

Copy link
Copy Markdown
ContributorAuthor

PR updated with almost same commit that upstream.

@chris-eiblchris-eibl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @thexai!

@thexaithexai changed the title gh-152433: Windows: allow build prim.c for UWPgh-152433: Windows: allow build mimalloc for UWPAug 1, 2026
@thexai

thexai commented Aug 1, 2026

Copy link
Copy Markdown
ContributorAuthor

I tested the updated PR on Xbox and not even compiles due a missing header 😒

PR updated with the fix.

(the header was already present in the first version of the PR)

@thexai

Copy link
Copy Markdown
ContributorAuthor

Finally found that LoadLibrary can be used on UWP for system libs from Windows SDK 10.0.22000.0.

And most changes are not necessary 😊

Sign up for freeto 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

@thexai@chris-eibl@zooba@zware@colesbury