Pep 554 : High level implementation of sub-interpreters module #1

nanjekyejoannah · 2019-09-10T16:34:19Z

Sub-Interpreters in Standard Library.

Initial implementation of the High level sub-interpreters module.

nanjekyejoannah · 2019-09-10T16:51:58Z

ericsnowcurrently

This is a good start. Missing pieces:

Interpreter class
channels
unit tests (I saw only functional tests)

For the tests I left a number of suggestions, but only do what makes sense. I just wanted to present one approach (which I find effective).

Doc/library/interpreters.rst

ericsnowcurrently · 2019-09-11T09:19:50Z

Doc/library/interpreters.rst

+
+This module constructs higher-level interpreters interfaces on top of the lower
+level :mod:`_interpreters` module.
+


It would probably make sense to add a glossary entry for "subinterpreter" and to link to it above.

Likewise there should be a brief mention of the "main" interpreter and maybe a link for it (either a new glossary entry or the section in the C-API docs).

ericsnowcurrently · 2019-09-11T09:22:06Z

Doc/library/interpreters.rst

@@ -0,0 +1,44 @@
+:mod:`interpreters` --- High-level Sub-interpreters Module


Let's be sure to be consistent about naming. FWIW, at this point I'm a fan of "subinterpreter" rather than "sub-interpreter", but it's not crucial to me. :) The most important point is that we be consistent.

Doc/library/interpreters.rst

ericsnowcurrently · 2019-09-11T10:33:32Z

Lib/test/test_interpreters.py

+        main = interpreters.get_main()
+        cur = interpreters.get_current()
+        self.assertEqual(cur, main)
+


Make sure to add a function test where get_current() is called in a subinterpreter.

Lib/test/test_interpreters.py

ericsnowcurrently · 2019-09-11T10:38:32Z

Lib/test/test_interpreters.py

+    def test_create(self):
+        id = interpreters.create()
+        self.assertIn(id, interpreters.list_all())
+


Make sure to add a function test that creates more than one. You could do that in the existing test.

Co-Authored-By: Eric Snow <[email protected]>

LewisGaul · 2019-10-27T21:40:54Z

Lib/interpreters.py

+        No longer associate the current interpreterwith the channel
+        (on the sending end).
+        """
+        return _interpreters.(self.id)


Should this say return _interpreters.channel_release(self.id)? Currently a syntax error.

Good catch. Am working on it.

LewisGaul · 2019-10-27T22:23:29Z

Lib/interpreters.py

+"""Subinterpreters High Level Module."""
+
+import _interpreters
+import logger


This should be import logging?

soltysh

This is my first pass, but I'll re-review this shortly again.

soltysh · 2019-12-06T11:10:32Z

Doc/library/interpreters.rst

+    .. method:: destroy()
+
+       Destroy the identified interpreter. Attempting to destroy the current
+       interpreter results in a RuntimeError. So does an unrecognized ID.


I think current is not a good wording here, what do you mean by current. From PEP-554 the idea was to not be able destroy a running interpreter.

The last sentence is also not clear, what ID, how is it unrecognised?

soltysh · 2019-12-06T11:31:32Z

Lib/interpreters.py

+        obj = _interpreters.channel_recv(self.id)
+        if obj == None:
+            wait(timeout)
+            obj = obj = _interpreters.channel_recv(self.id)


Suggested change

obj = obj = _interpreters.channel_recv(self.id)

obj = _interpreters.channel_recv(self.id)

channel_recv doesn't return None when channel is empty, but rather it raises ChannelEmptyError. So I'd imagine this method try to read the channel, catch the error, and re-try. There are several possible approaches for this:

it can be either poll (but channels don't support that, as of now),

we could divide the input timeout by some "magical" number and re-try every X amount of time.

soltysh · 2019-12-06T11:50:05Z

Lib/interpreters.py

+
+        Like recv(), but return the default instead of waiting.
+        """
+        return _interpreters.channel_recv(self.id)


Suggested change

return _interpreters.channel_recv(self.id)

try:

return _interpreters.channel_recv(self.id)

except _interpreters.ChannelEmptyError:

return default

soltysh · 2019-12-06T11:53:31Z

Modules/_xxsubinterpretersmodule.c

@@ -293,7 +293,7 @@ channel_exceptions_init(PyObject *ns)
    // XXX Move the exceptions into per-module memory?

    // A channel-related operation failed.
-    ChannelError = PyErr_NewException("_xxsubinterpreters.ChannelError",
+    ChannelError = PyErr_NewException("_interpreters.ChannelError",


I think with this we should also consider renaming the file, for consistency.

``` Direct leak of 8 byte(s) in 1 object(s) allocated from: #0 0x7f008bf19667 in __interceptor_malloc (/lib64/libasan.so.6+0xb0667) #1 0x7f007a0bee4a in subprocess_fork_exec /home/heimes/dev/python/cpython/Modules/_posixsubprocess.c:774 python#2 0xe0305b in cfunction_call Objects/methodobject.c:546 ``` Signed-off-by: Christian Heimes <[email protected]>

1: Initial CI support. r=ltratt a=vext01 This is preliminary CI support for CPython. A test is failing: ``` test test_signal failed -- Traceback (most recent call last): File "/ci/Lib/test/test_signal.py", line 184, in test_main self.fail(tb) AssertionError: Traceback (most recent call last): File "/ci/Lib/test/test_signal.py", line 167, in test_main self.run_test() File "/ci/Lib/test/test_signal.py", line 94, in run_test child = ignoring_eintr(subprocess.Popen, ['kill', '-HUP', str(pid)]) File "/ci/Lib/test/test_signal.py", line 32, in ignoring_eintr return __func(*args, **kwargs) File "/ci/Lib/subprocess.py", line 394, in __init__ errread, errwrite) File "/ci/Lib/subprocess.py", line 1047, in _execute_child raise child_exception OSError: [Errno 2] No such file or directory ``` Does anyone know anything about it? Could have cloned at a bad time? Co-authored-by: Edd Barrett <[email protected]>

) Fix test_gdb.test_pycfunction() for Python built with clang -Og. Tolerate inlined functions in the gdb traceback. When _testcapimodule.c is built by clang -Og, _null_to_none() is inlined in meth_varargs() and so gdb returns _null_to_none() as the frame #1. If it's not inlined, meth_varargs() is the frame #1.

…python#91466) Fix an uninitialized bool in exception print context. `struct exception_print_context.need_close` was uninitialized. Found by oss-fuzz in a test case running under the undefined behavior sanitizer. https://oss-fuzz.com/testcase-detail/6217746058182656 ``` Python/pythonrun.c:1241:28: runtime error: load of value 253, which is not a valid value for type 'bool' #0 0xbf2203 in print_chained cpython3/Python/pythonrun.c:1241:28 #1 0xbea4bb in print_exception_cause_and_context cpython3/Python/pythonrun.c:1320:19 python#2 0xbea4bb in print_exception_recursive cpython3/Python/pythonrun.c:1470:13 python#3 0xbe9e39 in _PyErr_Display cpython3/Python/pythonrun.c:1517:9 ``` Pretty obvious what the ommission was upon code inspection.

nanjekyejoannah added 3 commits September 10, 2019 11:00

Add tests for the interpreters module

f5b464a

create stdlib interpreters module and _interpreters internal module

331be62

Global initial Sub-Interpreters functions

3f2f2f4

nanjekyejoannah changed the title ~~Pep 554~~ Pep 554 : High level implementation of sub-interpreters module Sep 10, 2019

update __all__

4618343

ericsnowcurrently suggested changes Sep 11, 2019

View reviewed changes

nanjekyejoannah and others added 9 commits September 12, 2019 17:01

Implementation of all methods and relevant documentation

ec63183

Update Lib/test/test_interpreters.py

80f6808

Co-Authored-By: Eric Snow <[email protected]>

Update Lib/test/test_interpreters.py

724e618

Co-Authored-By: Eric Snow <[email protected]>

Update Lib/test/test_interpreters.py

abd0011

Co-Authored-By: Eric Snow <[email protected]>

Update Lib/test/test_interpreters.py

d502415

Co-Authored-By: Eric Snow <[email protected]>

Update Doc/library/interpreters.rst

a751e15

Co-Authored-By: Eric Snow <[email protected]>

Update Doc/library/interpreters.rst

56083c2

Co-Authored-By: Eric Snow <[email protected]>

Update Doc/library/interpreters.rst

14e97fc

Co-Authored-By: Eric Snow <[email protected]>

Add tests

9ab54eb

nanjekyejoannah mentioned this pull request Oct 27, 2019

Low level functionality for listing interpreters associated with a channel ericsnowcurrently/multi-core-python#52

Closed

LewisGaul reviewed Oct 27, 2019

View reviewed changes

Lib/interpreters.py Outdated

"""Subinterpreters High Level Module."""

import _interpreters

import logger

Copy link

LewisGaul Oct 27, 2019

Choose a reason for hiding this comment

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

This should be import logging?

soltysh reviewed Dec 6, 2019

View reviewed changes

Fix tests

b7dce73

nanjekyejoannah force-pushed the pep_554 branch from ecbb741 to b7dce73 Compare March 3, 2020 21:38

nanjekyejoannah added 3 commits March 4, 2020 22:39

Add send buffer functionality

e9b56b8

Add tests for Global functions and Interpreter

2bf89d4

Test coverage and documentation

158e54f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pep 554 : High level implementation of sub-interpreters module #1

Pep 554 : High level implementation of sub-interpreters module #1

nanjekyejoannah commented Sep 10, 2019

nanjekyejoannah commented Sep 10, 2019

ericsnowcurrently left a comment

ericsnowcurrently Sep 11, 2019

ericsnowcurrently Sep 11, 2019

ericsnowcurrently Sep 11, 2019

ericsnowcurrently Sep 11, 2019

LewisGaul Oct 27, 2019

nanjekyejoannah Oct 27, 2019

LewisGaul Oct 27, 2019

soltysh left a comment

soltysh Dec 6, 2019

soltysh Dec 6, 2019

soltysh Dec 6, 2019

soltysh Dec 6, 2019

soltysh Dec 6, 2019

soltysh Dec 6, 2019


		This module constructs higher-level interpreters interfaces on top of the lower
		level :mod:`_interpreters` module.

		@@ -0,0 +1,44 @@
		:mod:`interpreters` --- High-level Sub-interpreters Module

	obj = obj = _interpreters.channel_recv(self.id)
	obj = _interpreters.channel_recv(self.id)

Pep 554 : High level implementation of sub-interpreters module #1

Are you sure you want to change the base?

Pep 554 : High level implementation of sub-interpreters module #1

Conversation

nanjekyejoannah commented Sep 10, 2019

nanjekyejoannah commented Sep 10, 2019

ericsnowcurrently left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soltysh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment