[issue46430] intern strings in deepfrozen modules

Change by Dong-hee Na : -- pull_requests: -29803 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mai

[issue46430] intern strings in deepfrozen modules

Change by Dong-hee Na : -- pull_requests: +29803 pull_request: https://github.com/python/cpython/pull/31683 ___ Python tracker ___ _

[issue46430] intern strings in deepfrozen modules

Guido van Rossum added the comment: > PEP 587 introduced PyStatus to Python startup code which let the > Py_Initialize() caller to decide how to handle errors ;-) For example, you > can open a graphical popup rather than killing the process with SIGABRT > (Py_FatalError() behavior) which mig

[issue46430] intern strings in deepfrozen modules

STINNER Victor added the comment: > The other functions you are calling *do* return errors. You should not ignore > those. If any errors are reported the caller can decide what to do (e.g. call > Py_FatalError(). PEP 587 introduced PyStatus to Python startup code which let the Py_Initialize

[issue46430] intern strings in deepfrozen modules

STINNER Victor added the comment: commit 0d9b565e62a5fc8c3e9b8c64cce764fe084ccb2b Author: Kumar Aditya <59607654+kumaraditya...@users.noreply.github.com> Date: Sat Feb 26 22:05:03 2022 +0530 Propagate errors (however unlikely) from _Py_Deepfreeze_Init() (GH-31596) -- __

[issue46430] intern strings in deepfrozen modules

Change by Guido van Rossum : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker ___

[issue46430] intern strings in deepfrozen modules

Change by Kumar Aditya : -- pull_requests: +29719 pull_request: https://github.com/python/cpython/pull/31596 ___ Python tracker ___

[issue46430] intern strings in deepfrozen modules

Guido van Rossum added the comment: > How it should be handled? Currently PyUnicode_InternInPlace ignores any > errors and does not return it. It would be backwards-incompatible to change > that, moreover as I explained in > https://github.com/python/cpython/pull/30683#discussion_r800648477

[issue46430] intern strings in deepfrozen modules

STINNER Victor added the comment: I wrote https://github.com/python/cpython/pull/31555 to make sure that Python doesn't leak at Python exit. -- ___ Python tracker ___ ___

[issue46430] intern strings in deepfrozen modules

Change by Dong-hee Na : -- pull_requests: +29678 pull_request: https://github.com/python/cpython/pull/31556 ___ Python tracker ___ _

[issue46430] intern strings in deepfrozen modules

STINNER Victor added the comment: New changeset 4dc746310bd37ad6b381f9176acd167d445f4385 by Kumar Aditya in branch 'main': bpo-46430: Fix memory leak in interned strings of deep-frozen modules (GH-31549) https://github.com/python/cpython/commit/4dc746310bd37ad6b381f9176acd167d445f4385 -

[issue46430] intern strings in deepfrozen modules

Kumar Aditya added the comment: > Okay, let's change the error handling. @Kumar, can you handle that? How it should be handled? Currently PyUnicode_InternInPlace ignores any errors and does not return it. It would be backwards-incompatible to change that, moreover as I explained in https://

[issue46430] intern strings in deepfrozen modules

Kumar Aditya added the comment: I have created a PR to fix the memory leak, See https://github.com/python/cpython/pull/31549 -- ___ Python tracker ___ ___

[issue46430] intern strings in deepfrozen modules

Change by Kumar Aditya : -- pull_requests: +29670 stage: -> patch review pull_request: https://github.com/python/cpython/pull/31549 ___ Python tracker ___

[issue46430] intern strings in deepfrozen modules

Change by Dong-hee Na : -- nosy: +corona10 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.pytho

[issue46430] intern strings in deepfrozen modules

STINNER Victor added the comment: I tried to hack _PyStaticCode_Dealloc() to free strings, but since immortal objects are half-baken today, calling Py_DECREF() does crash. Py_SETREF() should increase _Py_RefTotal if the old object is immortal and he new object is not. Maybe this change shou

[issue46430] intern strings in deepfrozen modules

STINNER Victor added the comment: PyUnicode_InternInPlace() in intern_strings() can convert an immortal string to a regular Python strong reference, whereas _PyStaticCode_Dealloc() doesn't bother with clearing co_names, co_consts and co_localsplusnames. -- __

[issue46430] intern strings in deepfrozen modules

Guido van Rossum added the comment: That's pretty mysterious. The deep-freeze code isn't on the stack for either of those, but allocation of new unicode string objects is. I'm guessing these are somehow leaked by the interning, but I don't follow yet how. --

[issue46430] intern strings in deepfrozen modules

STINNER Victor added the comment: > Sure, but "leaks" caused by deep-freezing cannot be solved by freeing up the > deep-frozen memory -- the solution must be to update the accounting somewhere. Python allocates memory (ex: with PyObject_Malloc()) which is not released at exit. Two examples f

[issue46430] intern strings in deepfrozen modules

Guido van Rossum added the comment: > Not leaking memory at exit matters when Python is embedded > in an application. Sure, but "leaks" caused by deep-freezing cannot be solved by freeing up the deep-frozen memory -- the solution must be to update the accounting somewhere. Where is the exist

[issue46430] intern strings in deepfrozen modules

STINNER Victor added the comment: > Presumably the leak is imaginary Well, you can check with your favorite memory debugger like Valgrind if you don't trust Python internal memory debugger :-) Not leaking memory at exit matters when Python is embedded in an application. I don't think that i

[issue46430] intern strings in deepfrozen modules

Guido van Rossum added the comment: Okay, let's change the error handling. @Kumar, can you handle that? @Victor, the refleak is unrelated to the error handling right? Presumably the leak is imaginary -- the deep-frozen interned strings should be accounted for somehow. @Kumar do you need help

[issue46430] intern strings in deepfrozen modules

STINNER Victor added the comment: _PyStaticCode_InternStrings() error handling is based on assert(): that's really bad. It can crash Python (exit with abort()) at the first memory allocation failure. Why not returning -1 on error? -- ___ Python tr

[issue46430] intern strings in deepfrozen modules

STINNER Victor added the comment: > This change introduced a memory leak at Python exit. It's regression related to bpo-1635741 which I fixed recently: https://mail.python.org/archives/list/python-...@python.org/thread/E4C6TDNVDPDNNP73HTGHN5W42LGAE22F/ --

[issue46430] intern strings in deepfrozen modules

STINNER Victor added the comment: > New changeset c0a5ebeb1239020f2ecc199053bb1a70d78841a1 by Kumar Aditya in > branch 'main': > bpo-46430: Intern strings in deep-frozen modules (GH-30683) This change introduced a memory leak at Python exit. Before: $ ./python -X showrefcount -c pass [0 re

[issue46430] intern strings in deepfrozen modules

Christian Heimes added the comment: Please leave the ticket open until we have an agreement how to handle the missing error checks. -- resolution: fixed -> stage: resolved -> status: closed -> open type: -> behavior ___ Python tracker

[issue46430] intern strings in deepfrozen modules

Kumar Aditya added the comment: I consider this done so closing it as improving the error handling of interning it out of scope of this issue. -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue46430] intern strings in deepfrozen modules

miss-islington added the comment: New changeset dee020a6f5bf29f95bec6294da9bcd577114f592 by Nikita Sobolev in branch 'main': Fix sphinx-lint after #31097 and b878b3a (GH-31248) https://github.com/python/cpython/commit/dee020a6f5bf29f95bec6294da9bcd577114f592 -- nosy: +miss-islington

[issue46430] intern strings in deepfrozen modules

Change by Nikita Sobolev : -- nosy: +sobolevn nosy_count: 3.0 -> 4.0 pull_requests: +29417 pull_request: https://github.com/python/cpython/pull/31248 ___ Python tracker ___ ___

[issue46430] intern strings in deepfrozen modules

Guido van Rossum added the comment: We discussed that and found that a lot of errors are ignored during interning anyway. But it's not too late to change if you want to (sending a PR would probably be quicker than arguing :-). -- ___ Python track

[issue46430] intern strings in deepfrozen modules

Christian Heimes added the comment: I noticed that the new interning code lacks proper error reporting. There are only asserts. _PyCode_New() checks the return value of intern_strings. -- nosy: +christian.heimes ___ Python tracker

[issue46430] intern strings in deepfrozen modules

Guido van Rossum added the comment: New changeset c0a5ebeb1239020f2ecc199053bb1a70d78841a1 by Kumar Aditya in branch 'main': bpo-46430: Intern strings in deep-frozen modules (GH-30683) https://github.com/python/cpython/commit/c0a5ebeb1239020f2ecc199053bb1a70d78841a1 -- ___

[issue46430] intern strings in deepfrozen modules

Kumar Aditya added the comment: This speeds up comparison of strings by just comparing their pointers so it is much faster. -- ___ Python tracker ___

[issue46430] intern strings in deepfrozen modules

New submission from Kumar Aditya : Interns strings in deep-frozen modules. See https://github.com/faster-cpython/ideas/issues/218 -- ___ Python tracker ___ ___

[issue46430] intern strings in deepfrozen modules

Change by Kumar Aditya : -- keywords: +patch pull_requests: +28882 stage: -> patch review pull_request: https://github.com/python/cpython/pull/30683 ___ Python tracker ___ ___

[issue46430] intern strings in deepfrozen modules

Change by Kumar Aditya : -- nosy: gvanrossum, kumaraditya303 priority: normal severity: normal status: open title: intern strings in deepfrozen modules versions: Python 3.11 ___ Python tracker ___