← 返回首页
bpo-1635741: convert unicode ucd type to heap type by koubaa · Pull Request #22490 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

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

Provide feedback

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

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

bpo-1635741: convert unicode ucd type to heap type#22490

Closed
koubaa wants to merge 2 commits into
python:masterfrom
koubaa:bpo-1635741-unicode-heap-type
Closed

bpo-1635741: convert unicode ucd type to heap type#22490
koubaa wants to merge 2 commits into
python:masterfrom
koubaa:bpo-1635741-unicode-heap-type

Conversation

koubaa commented Oct 2, 2020
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

koubaa commented Oct 2, 2020

Copy link
Copy Markdown
Contributor Author

@vstinner please review

Comment thread Modules/unicodedata.c Outdated Show resolved Hide resolved
Comment thread Modules/unicodedata.c Outdated Show resolved Hide resolved
Comment thread Modules/unicodedata.c Outdated Show resolved Hide resolved
Comment thread Modules/unicodedata.c Outdated Show resolved Hide resolved
Comment thread Modules/unicodedata.c Outdated Show resolved Hide resolved

Copy link
Copy Markdown
Member

While this change is correct, I'm not excited by leaking a new heap type at Python exit.

Would you mind to attempt to add a module state to the module, and pass to all functions which use the UCD_Type? (without converting the UCD_Type to a heap type) The module state can be an empty structure, or add a "int dummy" just to make it non-empty.

koubaa commented Oct 12, 2020

Copy link
Copy Markdown
Contributor Author

While this change is correct, I'm not excited by leaking a new heap type at Python exit.

Would you mind to attempt to add a module state to the module, and pass to all functions which use the UCD_Type? (without converting the UCD_Type to a heap type) The module state can be an empty structure, or add a "int dummy" just to make it non-empty.

@vstinner This is going to a large set of changes, almost as large as the original PR, because I have to introduce an additional layer for all methods which are used by both the module and the type. I don't see how this will help with the leak

koubaa force-pushed the bpo-1635741-unicode-heap-type branch from 2862eea to d612585 Compare October 19, 2020 01:53

Copy link
Copy Markdown
Member

This PR is outdated, unicodedata got many changes in the meanwhile. I proposed one approach to convert unicodedata to multi-phase init in https://bugs.python.org/issue42157 I close this PR. Once PR #22990 will be merged, I will propose a PR to finally convert the module to multi-phase init. Sorry for the misunderstanding, but this extension module is way more complex than other extensions, and I didn't spot all corner cases at the first review. See the bpo for the list of all issues and my proposal.

vstinner closed this Oct 26, 2020

koubaa commented Nov 2, 2020

Copy link
Copy Markdown
Contributor Author

@vstinner no problem, I agree it is complex.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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

Footer

© 2026 GitHub, Inc.