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-106931: Intern Statically Allocated Strings Globally by ericsnowcurrently · Pull Request #107272 · python/cpython

Conversation

@ericsnowcurrently

@ericsnowcurrently ericsnowcurrently commented Jul 25, 2023 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

We tried this before with a dict and for all interned strings. That ran into problems due to interpreter isolation. However, exclusively using a per-interpreter cache caused some inconsistency that can eliminate the benefit of interning. Here we circle back to using a global cache, but only for statically allocated strings. We also use a more-basic _Py_hashtable_t for that global cache instead of a dict.

Ideally we would only have the global cache, but the optional isolation of each interpreter's allocator means that a non-static string object must not outlive its interpreter. Thus we would have to store a copy of each such interned string in the global cache, tied to the main interpreter.

  • Issue: Global String Objects are Interned Only in the First Interpreter #106931

Sorry, something went wrong.

Uh oh!

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

All reactions
ericsnowcurrently added 7 commits July 25, 2023 10:07
@ericsnowcurrently
Add PyASCIIObject.state.statically_allocated.
20d904e
@ericsnowcurrently
Handle statically allocated strings explicitly when interning.
76f1988
@ericsnowcurrently
Add tests.
2530e4f
@ericsnowcurrently
Add _Py_hashtable_len().
7fbe710
@ericsnowcurrently
Add _PyRuntime.cached_objects.interned_strings.
37508a8
@ericsnowcurrently
Intern static str objects to the global hashtable.
1903d30
@ericsnowcurrently
Add a NEWS entry.
d93fbaf
@ericsnowcurrently ericsnowcurrently added the needs backport to 3.12 only security fixes label Jul 25, 2023
@ericsnowcurrently
ericsnowcurrently requested review from Yhg1s and kumaraditya303 July 25, 2023 21:30
@bedevere-bot bedevere-bot mentioned this pull request Jul 25, 2023
Global String Objects are Interned Only in the First Interpreter #106931
Closed
@bedevere-bot bedevere-bot added the awaiting core review label Jul 25, 2023
kumaraditya303
kumaraditya303 reviewed Jul 27, 2023
View reviewed changes
Comment thread Objects/unicodeobject.c Outdated
{
return PyObject_Length(get_interned_dict(_PyInterpreterState_GET()));
PyObject *dict = get_interned_dict(_PyInterpreterState_GET());
return _Py_hashtable_len(INTERNED_STRINGS) + PyObject_Length(dict);

@kumaraditya303 kumaraditya303 Jul 27, 2023

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.

Suggested change
return _Py_hashtable_len(INTERNED_STRINGS) + PyObject_Length(dict);
return _Py_hashtable_len(INTERNED_STRINGS) + PyDict_GET_SIZE(dict);

Sorry, something went wrong.

Uh oh!

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

ericsnowcurrently reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@ericsnowcurrently ericsnowcurrently Jul 27, 2023

Copy link
Copy Markdown
Member Author

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.

done

Sorry, something went wrong.

Uh oh!

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

All reactions
@Yhg1s

Yhg1s commented Jul 27, 2023

Copy link
Copy Markdown
Member

FWIW, I wouldn't be too sad if we didn't do this in 3.12 (considering the current state is that strings just aren't interned in isolated subinterpreters), but if you feel confident enough to backport this before next monday, I'm okay with that.

All reactions

Sorry, something went wrong.

Uh oh!

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

Yhg1s
Yhg1s approved these changes Jul 27, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jul 27, 2023
@ericsnowcurrently

ericsnowcurrently commented Jul 27, 2023

Copy link
Copy Markdown
Member Author

FWIW, I wouldn't be too sad if we didn't do this in 3.12 (considering the current state is that strings just aren't interned in isolated subinterpreters),

Honestly, I was a little on the fence myself. My rationale is IMHO reasonable but clearly not the strongest: leaving a performance penalty, however small, because of subinterpreters is frustrating. If the solution were any more complex I'd probably lean the other way.

if you feel confident enough to backport this before next monday, I'm okay with that.

Regardless of what I said above, I'll stew on it a bit more. Thanks for the clarity.

All reactions

Sorry, something went wrong.

Uh oh!

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

ericsnowcurrently added 2 commits July 27, 2023 12:02
@ericsnowcurrently
Use PyDict_GET_SIZE().
534f5fd
@ericsnowcurrently
Use the right allocator.
8905116
@ericsnowcurrently
ericsnowcurrently merged commit b72947a into python:main Jul 27, 2023
@miss-islington

miss-islington commented Jul 27, 2023

Copy link
Copy Markdown
Contributor

Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot bedevere-bot removed the awaiting merge label Jul 27, 2023
@ericsnowcurrently
ericsnowcurrently deleted the fix-interned-strings branch July 27, 2023 19:57
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 27, 2023
@ericsnowcurrently @miss-islington
pythongh-106931: Intern Statically Allocated Strings Globally (python…
b5cc1b6
…gh-107272)

We tried this before with a dict and for all interned strings.  That ran into problems due to interpreter isolation.  However, exclusively using a per-interpreter cache caused some inconsistency that can eliminate the benefit of interning.  Here we circle back to using a global cache, but only for statically allocated strings.  We also use a more-basic _Py_hashtable_t for that global cache instead of a dict.

Ideally we would only have the global cache, but the optional isolation of each interpreter's allocator means that a non-static string object must not outlive its interpreter.  Thus we would have to store a copy of each such interned string in the global cache, tied to the main interpreter.
(cherry picked from commit b72947a)

Co-authored-by: Eric Snow <[email protected]>
@bedevere-bot

bedevere-bot commented Jul 27, 2023

Copy link
Copy Markdown

GH-107358 is a backport of this pull request to the 3.12 branch.

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Jul 27, 2023
@ericsnowcurrently ericsnowcurrently mentioned this pull request Jul 27, 2023
[3.12] gh-106931: Intern Statically Allocated Strings Globally (gh-107272) #107358
Closed
@bedevere-bot

bedevere-bot commented Jul 27, 2023

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot wasm32-emscripten node (dynamic linking) 3.x has failed when building commit b72947a.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/1056/builds/2656) and take a look at the build logs.
  4. Check if the failure is related to this commit (b72947a) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/1056/builds/2656

Failed tests:

  • test_sys

Summary of the results of the build (if available):

== Tests result: FAILURE ==

325 tests OK.

10 slowest tests:

  • test_hashlib: 2 min 13 sec
  • test_math: 1 min 45 sec
  • test_unparse: 48.2 sec
  • test_tokenize: 34.2 sec
  • test_capi: 33.6 sec
  • test_tarfile: 29.1 sec
  • test_unicodedata: 23.9 sec
  • test_fstring: 23.9 sec
  • test_pickle: 16.1 sec
  • test_decimal: 13.9 sec

1 test failed:
test_sys

121 tests skipped:
test.test_asyncio.test_base_events
test.test_asyncio.test_buffered_proto
test.test_asyncio.test_context
test.test_asyncio.test_eager_task_factory
test.test_asyncio.test_events test.test_asyncio.test_futures
test.test_asyncio.test_futures2 test.test_asyncio.test_locks
test.test_asyncio.test_pep492
test.test_asyncio.test_proactor_events
test.test_asyncio.test_protocols test.test_asyncio.test_queues
test.test_asyncio.test_runners
test.test_asyncio.test_selector_events
test.test_asyncio.test_sendfile test.test_asyncio.test_server
test.test_asyncio.test_sock_lowlevel test.test_asyncio.test_ssl
test.test_asyncio.test_sslproto test.test_asyncio.test_streams
test.test_asyncio.test_subprocess
test.test_asyncio.test_taskgroups test.test_asyncio.test_tasks
test.test_asyncio.test_threads test.test_asyncio.test_timeouts
test.test_asyncio.test_transports
test.test_asyncio.test_unix_events test.test_asyncio.test_waitfor
test.test_asyncio.test_windows_events
test.test_asyncio.test_windows_utils test__xxinterpchannels
test__xxsubinterpreters test_asyncgen test_clinic test_cmd_line
test_concurrent_futures test_contextlib_async test_ctypes
test_curses test_dbm_gnu test_dbm_ndbm test_devpoll test_doctest
test_docxmlrpc test_dtrace test_embed test_epoll test_faulthandler
test_fcntl test_file_eintr test_fork1 test_ftplib test_gdb
test_generated_cases test_grp test_httplib test_httpservers
test_idle test_imaplib test_interpreters test_ioctl test_kqueue
test_launcher test_lzma test_mmap test_multiprocessing_fork
test_multiprocessing_forkserver test_multiprocessing_main_handling
test_multiprocessing_spawn test_openpty test_pdb
test_perf_profiler test_perfmaps test_poll test_poplib test_pty
test_pwd test_queue test_readline test_regrtest test_repl
test_resource test_select test_selectors test_smtplib test_smtpnet
test_socket test_socketserver test_ssl test_stable_abi_ctypes
test_startfile test_subprocess test_sys_settrace test_syslog
test_tcl test_thread test_threadedtempfile test_threading
test_threading_local test_tkinter test_tools test_ttk
test_ttk_textonly test_turtle test_urllib2 test_urllib2_localnet
test_urllib2net test_urllibnet test_venv test_wait3 test_wait4
test_webbrowser test_winconsoleio test_winreg test_winsound
test_wmi test_wsgiref test_xmlrpc test_zipfile64
test_zipimport_support test_zoneinfo
0:26:18 load avg: 9.76
0:26:18 load avg: 9.76 Re-running failed tests is not supported with --python host runner option.

Total duration: 26 min 18 sec

Click to see traceback logs
Traceback (most recent call last):
  File "/opt/buildbot/bcannon-wasm/3.x.bcannon-wasm.emscripten-node-dl/build/Lib/test/libregrtest/runtest.py", line 373, in _runtest_inner
    refleak = _runtest_inner2(ns, test_name)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/buildbot/bcannon-wasm/3.x.bcannon-wasm.emscripten-node-dl/build/Lib/test/libregrtest/runtest.py", line 313, in _runtest_inner2
    the_module = importlib.import_module(abstest)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/buildbot/bcannon-wasm/3.x.bcannon-wasm.emscripten-node-dl/build/Lib/importlib/__init__.py", line 88, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1293, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1266, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1237, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 841, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 1002, in exec_module
  File "<frozen importlib._bootstrap>", line 400, in _call_with_frames_removed
  File "/opt/buildbot/bcannon-wasm/3.x.bcannon-wasm.emscripten-node-dl/build/Lib/test/test_sys.py", line 17, in <module>
    from test.support import interpreters
  File "/opt/buildbot/bcannon-wasm/3.x.bcannon-wasm.emscripten-node-dl/build/Lib/test/support/interpreters.py", line 4, in <module>
    import _xxsubinterpreters as _interpreters
ModuleNotFoundError: No module named '_xxsubinterpreters'
All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot

bedevere-bot commented Jul 27, 2023

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot wasm32-emscripten node (pthreads) 3.x has failed when building commit b72947a.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/1050/builds/2643) and take a look at the build logs.
  4. Check if the failure is related to this commit (b72947a) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/1050/builds/2643

Failed tests:

  • test_sys

Summary of the results of the build (if available):

== Tests result: FAILURE ==

329 tests OK.

10 slowest tests:

  • test_hashlib: 2 min 10 sec
  • test_math: 1 min 34 sec
  • test_tarfile: 1 min
  • test_unparse: 1 min
  • test_tokenize: 47.8 sec
  • test_io: 43.1 sec
  • test_capi: 39.1 sec
  • test_unicodedata: 26.6 sec
  • test_pathlib: 25.6 sec
  • test_zipfile: 25.0 sec

1 test failed:
test_sys

117 tests skipped:
test.test_asyncio.test_base_events
test.test_asyncio.test_buffered_proto
test.test_asyncio.test_context
test.test_asyncio.test_eager_task_factory
test.test_asyncio.test_events test.test_asyncio.test_futures
test.test_asyncio.test_futures2 test.test_asyncio.test_locks
test.test_asyncio.test_pep492
test.test_asyncio.test_proactor_events
test.test_asyncio.test_protocols test.test_asyncio.test_queues
test.test_asyncio.test_runners
test.test_asyncio.test_selector_events
test.test_asyncio.test_sendfile test.test_asyncio.test_server
test.test_asyncio.test_sock_lowlevel test.test_asyncio.test_ssl
test.test_asyncio.test_sslproto test.test_asyncio.test_streams
test.test_asyncio.test_subprocess
test.test_asyncio.test_taskgroups test.test_asyncio.test_tasks
test.test_asyncio.test_threads test.test_asyncio.test_timeouts
test.test_asyncio.test_transports
test.test_asyncio.test_unix_events test.test_asyncio.test_waitfor
test.test_asyncio.test_windows_events
test.test_asyncio.test_windows_utils test__xxinterpchannels
test__xxsubinterpreters test_asyncgen test_clinic test_cmd_line
test_concurrent_futures test_contextlib_async test_ctypes
test_curses test_dbm_gnu test_dbm_ndbm test_devpoll test_doctest
test_docxmlrpc test_dtrace test_embed test_epoll test_faulthandler
test_fcntl test_file_eintr test_fork1 test_ftplib test_gdb
test_generated_cases test_grp test_httplib test_httpservers
test_idle test_imaplib test_interpreters test_ioctl test_kqueue
test_launcher test_lzma test_mmap test_multiprocessing_fork
test_multiprocessing_forkserver test_multiprocessing_main_handling
test_multiprocessing_spawn test_openpty test_pdb
test_perf_profiler test_perfmaps test_poll test_poplib test_pty
test_pwd test_readline test_regrtest test_repl test_resource
test_select test_selectors test_smtplib test_smtpnet test_socket
test_socketserver test_ssl test_stable_abi_ctypes test_startfile
test_subprocess test_sys_settrace test_syslog test_tcl
test_tkinter test_tools test_ttk test_ttk_textonly test_turtle
test_urllib2 test_urllib2_localnet test_urllib2net test_urllibnet
test_venv test_wait3 test_wait4 test_webbrowser test_winconsoleio
test_winreg test_winsound test_wmi test_wsgiref test_xmlrpc
test_xxlimited test_zipfile64 test_zipimport_support test_zoneinfo
0:26:17 load avg: 6.26
0:26:17 load avg: 6.26 Re-running failed tests is not supported with --python host runner option.

Total duration: 26 min 17 sec

Click to see traceback logs
Traceback (most recent call last):
  File "/opt/buildbot/bcannon-wasm/3.x.bcannon-wasm.emscripten-node-pthreads/build/Lib/test/libregrtest/runtest.py", line 373, in _runtest_inner
    refleak = _runtest_inner2(ns, test_name)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/buildbot/bcannon-wasm/3.x.bcannon-wasm.emscripten-node-pthreads/build/Lib/test/libregrtest/runtest.py", line 313, in _runtest_inner2
    the_module = importlib.import_module(abstest)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/buildbot/bcannon-wasm/3.x.bcannon-wasm.emscripten-node-pthreads/build/Lib/importlib/__init__.py", line 88, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1293, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1266, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1237, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 841, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 1002, in exec_module
  File "<frozen importlib._bootstrap>", line 400, in _call_with_frames_removed
  File "/opt/buildbot/bcannon-wasm/3.x.bcannon-wasm.emscripten-node-pthreads/build/Lib/test/test_sys.py", line 17, in <module>
    from test.support import interpreters
  File "/opt/buildbot/bcannon-wasm/3.x.bcannon-wasm.emscripten-node-pthreads/build/Lib/test/support/interpreters.py", line 4, in <module>
    import _xxsubinterpreters as _interpreters
ModuleNotFoundError: No module named '_xxsubinterpreters'
All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot

bedevere-bot commented Jul 27, 2023

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot wasm32-wasi 3.x has failed when building commit b72947a.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/1046/builds/2616) and take a look at the build logs.
  4. Check if the failure is related to this commit (b72947a) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/1046/builds/2616

Failed tests:

  • test_sys

Summary of the results of the build (if available):

== Tests result: FAILURE ==

317 tests OK.

10 slowest tests:

  • test_hashlib: 1 min 13 sec
  • test_math: 1 min 11 sec
  • test_unparse: 31.1 sec
  • test_tokenize: 28.2 sec
  • test_capi: 22.1 sec
  • test_fstring: 21.4 sec
  • test_unicodedata: 16.2 sec
  • test_pickle: 13.7 sec
  • test_tarfile: 12.9 sec
  • test_decimal: 12.5 sec

1 test failed:
test_sys

129 tests skipped:
test.test_asyncio.test_base_events
test.test_asyncio.test_buffered_proto
test.test_asyncio.test_context
test.test_asyncio.test_eager_task_factory
test.test_asyncio.test_events test.test_asyncio.test_futures
test.test_asyncio.test_futures2 test.test_asyncio.test_locks
test.test_asyncio.test_pep492
test.test_asyncio.test_proactor_events
test.test_asyncio.test_protocols test.test_asyncio.test_queues
test.test_asyncio.test_runners
test.test_asyncio.test_selector_events
test.test_asyncio.test_sendfile test.test_asyncio.test_server
test.test_asyncio.test_sock_lowlevel test.test_asyncio.test_ssl
test.test_asyncio.test_sslproto test.test_asyncio.test_streams
test.test_asyncio.test_subprocess
test.test_asyncio.test_taskgroups test.test_asyncio.test_tasks
test.test_asyncio.test_threads test.test_asyncio.test_timeouts
test.test_asyncio.test_transports
test.test_asyncio.test_unix_events test.test_asyncio.test_waitfor
test.test_asyncio.test_windows_events
test.test_asyncio.test_windows_utils test__xxinterpchannels
test__xxsubinterpreters test_asyncgen test_bz2 test_clinic
test_cmd_line test_concurrent_futures test_contextlib_async
test_ctypes test_curses test_dbm_gnu test_dbm_ndbm test_devpoll
test_doctest test_docxmlrpc test_dtrace test_embed test_epoll
test_faulthandler test_fcntl test_file_eintr test_fork1
test_ftplib test_gdb test_generated_cases test_grp test_gzip
test_httplib test_httpservers test_idle test_imaplib
test_interpreters test_ioctl test_kqueue test_launcher test_lzma
test_mailbox test_mmap test_multiprocessing_fork
test_multiprocessing_forkserver test_multiprocessing_main_handling
test_multiprocessing_spawn test_openpty test_pdb
test_perf_profiler test_perfmaps test_poll test_poplib test_pty
test_pwd test_queue test_readline test_regrtest test_repl
test_resource test_select test_selectors test_smtplib test_smtpnet
test_socket test_socketserver test_sqlite3 test_ssl
test_stable_abi_ctypes test_startfile test_subprocess
test_sys_settrace test_syslog test_tcl test_thread
test_threadedtempfile test_threading test_threading_local
test_tkinter test_tools test_ttk test_ttk_textonly test_turtle
test_urllib test_urllib2 test_urllib2_localnet test_urllib2net
test_urllib_response test_urllibnet test_venv test_wait3
test_wait4 test_webbrowser test_winconsoleio test_winreg
test_winsound test_wmi test_wsgiref test_xmlrpc test_xxlimited
test_zipfile64 test_zipimport_support test_zlib test_zoneinfo
0:05:22 load avg: 3.15
0:05:22 load avg: 3.15 Re-running failed tests is not supported with --python host runner option.

Total duration: 5 min 22 sec

Click to see traceback logs
Traceback (most recent call last):
  File "/Lib/test/libregrtest/runtest.py", line 373, in _runtest_inner
    refleak = _runtest_inner2(ns, test_name)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Lib/test/libregrtest/runtest.py", line 313, in _runtest_inner2
    the_module = importlib.import_module(abstest)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Lib/importlib/__init__.py", line 88, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1293, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1266, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1237, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 841, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 1002, in exec_module
  File "<frozen importlib._bootstrap>", line 400, in _call_with_frames_removed
  File "/Lib/test/test_sys.py", line 17, in <module>
    from test.support import interpreters
  File "/Lib/test/support/interpreters.py", line 4, in <module>
    import _xxsubinterpreters as _interpreters
ModuleNotFoundError: No module named '_xxsubinterpreters'
All reactions

Sorry, something went wrong.

Uh oh!

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

@ericsnowcurrently

ericsnowcurrently commented Jul 27, 2023

Copy link
Copy Markdown
Member Author

See gh-107362.

All reactions

Sorry, something went wrong.

Uh oh!

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

ericsnowcurrently added a commit to ericsnowcurrently/cpython that referenced this pull request Oct 11, 2023
@ericsnowcurrently
[3.12] pythongh-106931: Intern Statically Allocated Strings Globally (p…
352cfd9
…ythongh-107272)

We tried this before with a dict and for all interned strings.  That ran into problems due to interpreter isolation.  However, exclusively using a per-interpreter cache caused some inconsistency that can eliminate the benefit of interning.  Here we circle back to using a global cache, but only for statically allocated strings.  We also use a more-basic _Py_hashtable_t for that global cache instead of a dict.

Ideally we would only have the global cache, but the optional isolation of each interpreter's allocator means that a non-static string object must not outlive its interpreter.  Thus we would have to store a copy of each such interned string in the global cache, tied to the main interpreter.

(cherry-picked from commit b72947a)
ericsnowcurrently added a commit to ericsnowcurrently/cpython that referenced this pull request Oct 11, 2023
@ericsnowcurrently
[3.12] pythongh-106931: Intern Statically Allocated Strings Globally (p…
4e60828
…ythongh-107272)

We tried this before with a dict and for all interned strings.  That ran into problems due to interpreter isolation.  However, exclusively using a per-interpreter cache caused some inconsistency that can eliminate the benefit of interning.  Here we circle back to using a global cache, but only for statically allocated strings.  We also use a more-basic _Py_hashtable_t for that global cache instead of a dict.

Ideally we would only have the global cache, but the optional isolation of each interpreter's allocator means that a non-static string object must not outlive its interpreter.  Thus we would have to store a copy of each such interned string in the global cache, tied to the main interpreter.

(cherry-picked from commit b72947a)
@bedevere-app

bedevere-app Bot commented Oct 11, 2023

Copy link
Copy Markdown

GH-110713 is a backport of this pull request to the 3.12 branch.

All reactions

Sorry, something went wrong.

Uh oh!

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

ericsnowcurrently added a commit that referenced this pull request Nov 27, 2023
@ericsnowcurrently
[3.12] gh-106931: Intern Statically Allocated Strings Globally (gh-10…
4f71f16
…7272) (gh-110713)

We tried this before with a dict and for all interned strings.  That ran into problems due to interpreter isolation.  However, exclusively using a per-interpreter cache caused some inconsistency that can eliminate the benefit of interning.  Here we circle back to using a global cache, but only for statically allocated strings.  We also use a more-basic _Py_hashtable_t for that global cache instead of a dict.

Ideally we would only have the global cache, but the optional isolation of each interpreter's allocator means that a non-static string object must not outlive its interpreter.  Thus we would have to store a copy of each such interned string in the global cache, tied to the main interpreter.

(cherry-picked from commit b72947a)
@aisk aisk mentioned this pull request Jun 9, 2024
gh-115649: Fix double free for intern strings dict #120290
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 left review comments
@Yhg1s Yhg1s Yhg1s approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

5 participants

@ericsnowcurrently @Yhg1s @miss-islington @bedevere-bot @kumaraditya303
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.