Skip to content

bpo-1635741: port multiprocessing to multiphase init - #21378

Merged
vstinner merged 6 commits into
python:masterfrom
koubaa:bpo-1635741-multiprocessing
Aug 11, 2020
Merged

bpo-1635741: port multiprocessing to multiphase init#21378
vstinner merged 6 commits into
python:masterfrom
koubaa:bpo-1635741-multiprocessing

Conversation

@koubaa

@koubaa koubaa commented Jul 7, 2020

Copy link
Copy Markdown
Contributor
Comment thread Modules/_multiprocessing/multiprocessing.c Outdated
Comment thread Modules/_multiprocessing/multiprocessing.c Outdated
Comment thread Modules/_multiprocessing/multiprocessing.c Outdated
Comment thread Modules/_multiprocessing/multiprocessing.c
Comment thread Modules/_multiprocessing/multiprocessing.c Outdated
Comment thread Modules/_multiprocessing/multiprocessing.c Outdated
Comment thread Modules/_multiprocessing/multiprocessing.c Outdated
Comment thread Modules/_multiprocessing/multiprocessing.c Outdated
Comment thread Modules/_multiprocessing/multiprocessing.c
Comment thread Modules/_multiprocessing/multiprocessing.c Outdated
Comment thread Modules/_multiprocessing/multiprocessing.c Outdated
Comment thread Modules/_multiprocessing/multiprocessing.c Outdated
Comment thread Modules/_multiprocessing/multiprocessing.c
Comment thread Modules/_multiprocessing/multiprocessing.c Outdated
Comment thread Modules/_multiprocessing/multiprocessing.c
@vstinner

vstinner commented Jul 9, 2020

Copy link
Copy Markdown
Member

Hum, right now, I still cannot build the extension module. Example of errors:

/home/vstinner/python/master/Modules/_multiprocessing/multiprocessing.c:192:5: error: expected ‘while’ before ‘if’
  192 |     if (PyDict_SetItemString(flags, #name, value) < 0) {    \
      |     ^~
/home/vstinner/python/master/Modules/_multiprocessing/multiprocessing.c:200:5: note: in expansion of macro ‘ADD_FLAG’
  200 |     ADD_FLAG(HAVE_SEM_OPEN);
      |     ^~~~~~~~
In file included from ./Include/pytime.h:6,
                 from ./Include/Python.h:85,
                 from /home/vstinner/python/master/Modules/_multiprocessing/multiprocessing.h:6,
                 from /home/vstinner/python/master/Modules/_multiprocessing/multiprocessing.c:10:
/home/vstinner/python/master/Modules/_multiprocessing/multiprocessing.c:196:15: error: ‘value’ undeclared (first use in this function)
  196 |     Py_DECREF(value)                                        \
      |               ^~~~~
./Include/object.h:112:41: note: in definition of macro ‘_PyObject_CAST’
  112 | #define _PyObject_CAST(op) ((PyObject*)(op))
      |                                         ^~

} in return -1; } of ADD_FLAG is too much and should be removed.

Also, there is a missing ";" at the end of the macro, after Py_DECREF(value).

Well, see my previous comment.

I'm not sure if you updated the PR or not in the meanwhile.

Try to build Python locally.

@koubaa

koubaa commented Jul 9, 2020

Copy link
Copy Markdown
Contributor Author

@vstinner strange. It builds fine for me and the CI (on windows). I'll try to see why ubuntu/mac are not building.

@koubaa

koubaa commented Jul 9, 2020

Copy link
Copy Markdown
Contributor Author

@vstinner now it makes sense. On windows all the uses of ADD_FLAG are disabled by the preprocessor. I manually defined HAVE_SEM_OPEN to 1 to figure out the macro issue.

@koubaa

koubaa commented Aug 9, 2020

Copy link
Copy Markdown
Contributor Author

@vstinner can this be merged?

@shihai1991 shihai1991 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.

Oh, I have a comment but I forget press the submit button.

Comment thread Modules/_multiprocessing/multiprocessing.c Outdated
Comment thread Modules/_multiprocessing/multiprocessing.c
@vstinner

Copy link
Copy Markdown
Member

@shihai1991 @corona10: Would you mind to review this PR? (use the Approve button if it LGTY).

@shihai1991 shihai1991 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, LGTM.

@vstinner

Copy link
Copy Markdown
Member

Thanks @koubaa, merged. Thanks for reviews @shihai1991 and @corona10.

shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Aug 20, 2020
)

Port the _multiprocessing extension module to multiphase initialization (PEP 489).
@koubaa
koubaa deleted the bpo-1635741-multiprocessing branch August 28, 2020 00:21
xzy3 pushed a commit to xzy3/cpython that referenced this pull request Oct 18, 2020
)

Port the _multiprocessing extension module to multiphase initialization (PEP 489).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

6 participants