Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow nogil for most types of methods #181

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@ This document is displayed because you either opened an issue or you want to pro
When interacting with other developers, users or anyone else from our community, please adhere to
[the OpenMS CODE OF CONDUCT](https://github.com/OpenMS/OpenMS/blob/develop/CODE_OF_CONDUCT.md)

# Setting up dev environment
Install conda/mamba (or use a venv).
Install git.
Install a C++ compiler.

```bash
git clone https://github.com/OpenMS/autowrap
cd autowrap
mamba create -n autowrap -f environment.yml
python -m pytest
```


# Reporting an Issue:

You most likely came here to:
Expand All @@ -18,10 +31,10 @@ If you found a bug, e.g. an autowrap tool crashes during code generation, it is
- how you installed autowrap (e.g., from source, pip)
- a description on how to reproduce the bug
- relevant tool output (e.g., error messages)
- data to repoduce the bug (If possible as a GitHub gist. Other platforms like Dropbox, Google Drive links also work. If you can't share the data publicly please indicate this and we will contact you in private.)
- data to reproduce the bug (If possible as a GitHub gist. Other platforms like Dropbox, Google Drive links also work. If you can't share the data publicly please indicate this and we will contact you in private.)

If you are an official OpenMS team meber:
- label your issue using github labels (e.g. as: question, defect) that indicate the type of issue and which components of autowrap (blue labels) are affected. The severity is usually assigned by OpenMS maintainers and used internally to e.g. indicate if a bug is a blocker for a new release.
If you are an official OpenMS team member:
- label your issue using GitHub labels (e.g. as: question, defect) that indicate the type of issue and which components of autowrap (blue labels) are affected. The severity is usually assigned by OpenMS maintainers and used internally to e.g. indicate if a bug is a blocker for a new release.

# Opening a Pull Request

Expand Down
9 changes: 0 additions & 9 deletions README_DEVELOP

This file was deleted.

45 changes: 35 additions & 10 deletions autowrap/CodeGenerator.py
Original file line number Diff line number Diff line change
Expand Up @@ -1113,18 +1113,22 @@ def _create_fun_decl_and_input_conversion(self, code, py_name, method, is_free_f
call_args = []
checks = []
in_types = []
decls = []
decl_calls = []
for arg_num, (t, n) in enumerate(args):
# get new ConversionProvider using the converter registry
converter = self.cr.get(t)
converter.cr = self.cr
py_type = converter.matching_python_type(t)
py_typing_type = converter.matching_python_type_full(t)
conv_code, call_as, cleanup = converter.input_conversion(t, n, arg_num)
conv_code, call_as, cleanup, (decl, decl_call) = converter.input_conversion(t, n, arg_num)
py_signature_parts.append("%s %s " % (py_type, n))
py_typing_signature_parts.append("%s: %s " % (n, py_typing_type))
input_conversion_codes.append(conv_code)
cleanups.append(cleanup)
call_args.append(call_as)
decls.append(decl)
decl_calls.append(decl_call)
in_types.append(t)
checks.append((n, converter.type_check_expression(t, n)))

Expand Down Expand Up @@ -1211,7 +1215,7 @@ def _create_fun_decl_and_input_conversion(self, code, py_name, method, is_free_f
for conv_code in input_conversion_codes:
code.add(conv_code)

return call_args, cleanups, in_types, stub
return call_args, cleanups, in_types, stub, decls, decl_calls

def _create_wrapper_for_attribute(self, attribute):
code = Code()
Expand All @@ -1228,7 +1232,7 @@ def _create_wrapper_for_attribute(self, attribute):
converter = self.cr.get(t)
py_type = converter.matching_python_type(t)
py_typing_type = converter.matching_python_type_full(t)
conv_code, call_as, cleanup = converter.input_conversion(t, name, 0)
conv_code, call_as, cleanup, (decl, decl_call) = converter.input_conversion(t, name, 0)

code.add(
"""
Expand Down Expand Up @@ -1323,7 +1327,7 @@ def _create_wrapper_for_attribute(self, attribute):
code.add(" return py_result")
return code, stubs

def create_wrapper_for_nonoverloaded_method(self, cdcl, py_name, method):
def create_wrapper_for_nonoverloaded_method(self, cdcl, py_name, method: ResolvedMethod):
L.info(" create wrapper for %s ('%s')" % (py_name, method))
meth_code = Code()

Expand All @@ -1332,10 +1336,22 @@ def create_wrapper_for_nonoverloaded_method(self, cdcl, py_name, method):
cleanups,
in_types,
stubs,
decls,
decl_calls
) = self._create_fun_decl_and_input_conversion(meth_code, py_name, method)

init = ""
c_call_args = []
for decl, decl_call in zip(decls, decl_calls):
init = init + decl + "\n"
c_call_args.append(decl_call)

# call wrapped method and convert result value back to python
cpp_name = method.cpp_decl.name

if method.with_nogil and init.strip("\n"):
call_args = c_call_args

call_args_str = ", ".join(call_args)
if method.is_static:
cy_call_str = "%s.%s(%s)" % (
Expand All @@ -1348,13 +1364,15 @@ def create_wrapper_for_nonoverloaded_method(self, cdcl, py_name, method):

res_t = method.result_type
out_converter = self.cr.get(res_t)
full_call_stmt = out_converter.call_method(res_t, cy_call_str)
full_call_stmt = out_converter.call_method(res_t, cy_call_str, True, not method.with_nogil)

if method.with_nogil:
meth_code.add(
"""
| $init
| with nogil:
"""
|
""", locals()
)
indented = Code()
else:
Expand All @@ -1370,6 +1388,10 @@ def create_wrapper_for_nonoverloaded_method(self, cdcl, py_name, method):
else:
indented.add(full_call_stmt)

if method.with_nogil:
meth_code.add(indented)
indented = meth_code

for cleanup in reversed(cleanups):
if not cleanup:
continue
Expand All @@ -1385,9 +1407,6 @@ def create_wrapper_for_nonoverloaded_method(self, cdcl, py_name, method):
indented.add(to_py_code)
indented.add(" return py_result")

if method.with_nogil:
meth_code.add(indented)

return meth_code, stubs

def create_wrapper_for_free_function(self, decl: ResolvedFunction, out_codes: CodeDict) -> None:
Expand Down Expand Up @@ -1445,6 +1464,8 @@ def _create_wrapper_for_free_function(
cleanups,
in_types,
stubs,
decls,
decl_calls
) = self._create_fun_decl_and_input_conversion(fun_code, name, decl, is_free_fun=True)

call_args_str = ", ".join(call_args)
Expand Down Expand Up @@ -1552,6 +1573,8 @@ def create_wrapper_for_nonoverloaded_constructor(self, class_decl, py_name, cons
cleanups,
in_types,
stubs,
decls,
decl_calls
) = self._create_fun_decl_and_input_conversion(cons_code, py_name, cons_decl)

stub_code.extend(stubs)
Expand Down Expand Up @@ -1663,6 +1686,8 @@ def create_special_getitem_method(self, mdcl):
cleanups,
(in_type,),
stubs,
decls,
decl_calls
) = self._create_fun_decl_and_input_conversion(meth_code, "__getitem__", mdcl)

meth_code.add(
Expand Down Expand Up @@ -1790,7 +1815,7 @@ def create_special_setitem_method(self, mdcl):
# CppObject[ idx ] = value
#
cy_call_str = "deref(self.inst.get())[%s]" % call_arg
code, call_as, cleanup = out_converter.input_conversion(res_t, value_arg, 0)
code, call_as, cleanup, (decl, decl_call) = out_converter.input_conversion(res_t, value_arg, 0)
meth_code.add(
"""
| $cy_call_str = $call_as
Expand Down
Loading
Loading