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

boa: support running Python code in non-blocking way #602

Merged
merged 19 commits into from
Oct 10, 2020

Conversation

yorkie
Copy link
Member

@yorkie yorkie commented Sep 30, 2020

This PR uses the Node.js worker_threads to start a new thread to run Python functions, and add SharedPythonObject to share(not copy) the Python object in sub-threads.

The following is an experimental API usage(example):

const { Worker, isMainThread, workerData, parentPort } = require('worker_threads');
const boa = require('@pipcook/boa');
const pybasic = boa.import('tests.base.basic');
const { SharedPythonObject, symbols } = boa;

class Foobar extends pybasic.Foobar {
  hellomsg(x) {
    return `hello <${x}> on ${this.test}(${this.count})`;
  }
}

if (isMainThread) {
  const foo = new Foobar();
  const worker = new Worker(__filename, {
    workerData: {
      foo: new SharedPythonObject(foo),
      // the `foo` is unavailable util the sub-thread exits.
      // it throws an error if trying to use `foo`.
    },
  });
  let alive = setInterval(() => {
    // this is always alive though it's blocking by `foo.sleep()` in sub-thread.
    console.log(`main: still training, ownership(${foo[symbols.GetOwnershipSymbol]()})`);
  }, 1000);

  worker.on('message', (state) => {
    if (state === 'done') {
      clearInterval(alive);
      // the process exits
    }
  });
} else {
  const { foo } = workerData; // the foo is a borrowed object from main thread by `SharedPythonObject`.
  foo.sleep(); // call Python's method `sleep()`, it would block the sub-thread 5s.
  setTimeout(() => {
    // this will 1) do ref the sub-thread 2000ms, 2) send a "done" message and 3) exit the sub-thread.
    parentPort.postMessage('done');
  }, 2000);
}

Note: an object could only be shared with 1 sub-thread at the same time.

TODO

  • improvements
    • avoid to create SharedPythonObject twice and multiple times on a same object.
    • avoid crash(segfault) if the ownershipId is invalid.
  • unit tests
  • documentation

It uses the Node.js worker_threads to start new thread to run Python functions.
Note that, we share the same Python interrupter.
@yorkie yorkie added the boa Python related issues label Sep 30, 2020
@yorkie yorkie marked this pull request as ready for review September 30, 2020 16:35
@yorkie
Copy link
Member Author

yorkie commented Sep 30, 2020

@FeelyChau Just mark this PR be ready for review :)

@yorkie yorkie requested a review from WenheLI September 30, 2020 16:38
@yorkie
Copy link
Member Author

yorkie commented Sep 30, 2020

Occurred the following error if running npm test:

* thread #1, stop reason = signal SIGSTOP
  * frame #0: 0x000000010554b27e libpython3.7m.dylib`PyErr_FormatV + 30
    frame #1: 0x000000010554b239 libpython3.7m.dylib`PyErr_Format + 137
    frame #2: 0x0000000105478590 libpython3.7m.dylib`_PyObject_GenericGetAttrWithDict + 848
    frame #3: 0x0000000105477b26 libpython3.7m.dylib`PyObject_GetAttrString + 134
    frame #4: 0x0000000105477c4c libpython3.7m.dylib`PyObject_HasAttrString + 12
    frame #5: 0x000000010526c589 boa.node`boa::PythonObject::HasAttr(Napi::CallbackInfo const&) [inlined] pybind11::hasattr(obj=<unavailable>, name=<unavailable>) at pytypes.h:397:12 [opt]
    frame #6: 0x000000010526c584 boa.node`boa::PythonObject::HasAttr(this=0x00000001053091e0, info=0x00007ffeefbfc4a0) at object.cc:315 [opt]
    frame #7: 0x0000000105271a9b boa.node`Napi::ObjectWrap<boa::PythonObject>::InstanceMethodCallbackWrapper(this=<unavailable>)::'lambda'()::operator()() const at napi-inl.h:3838:12 [opt]
    frame #8: 0x00000001052719ca boa.node`Napi::ObjectWrap<boa::PythonObject>::InstanceMethodCallbackWrapper(napi_env__*, napi_callback_info__*) [inlined] napi_value__* Napi::details::WrapCallback<Napi::ObjectWrap<boa::PythonObject>::InstanceMethodCallbackWrapper(napi_env__*, napi_callback_info__*)::'lambda'()>(Napi::ObjectWrap<boa::PythonObject>::InstanceMethodCallbackWrapper(napi_env__*, napi_callback_info__*)::'lambda'()) at napi-inl.h:73:12 [opt]
    frame #9: 0x00000001052719c5 boa.node`Napi::ObjectWrap<boa::PythonObject>::InstanceMethodCallbackWrapper(env=0x000000010500b280, info=0x00007ffeefbfc580) at napi-inl.h:3831 [opt]
    frame #10: 0x000000010006346a node`v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) + 122
    frame #11: 0x00000001002580c8 node`v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) + 616
    frame #12: 0x000000010025765c node`v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) + 524
    frame #13: 0x0000000100256d82 node`v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) + 258
    frame #14: 0x0000000100a50f59 node`Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit + 57

@yorkie
Copy link
Member Author

yorkie commented Sep 30, 2020

Occurred the following error if running npm test:

* thread #1, stop reason = signal SIGSTOP
  * frame #0: 0x000000010554b27e libpython3.7m.dylib`PyErr_FormatV + 30
    frame #1: 0x000000010554b239 libpython3.7m.dylib`PyErr_Format + 137
    frame #2: 0x0000000105478590 libpython3.7m.dylib`_PyObject_GenericGetAttrWithDict + 848
    frame #3: 0x0000000105477b26 libpython3.7m.dylib`PyObject_GetAttrString + 134
    frame #4: 0x0000000105477c4c libpython3.7m.dylib`PyObject_HasAttrString + 12
    frame #5: 0x000000010526c589 boa.node`boa::PythonObject::HasAttr(Napi::CallbackInfo const&) [inlined] pybind11::hasattr(obj=<unavailable>, name=<unavailable>) at pytypes.h:397:12 [opt]
    frame #6: 0x000000010526c584 boa.node`boa::PythonObject::HasAttr(this=0x00000001053091e0, info=0x00007ffeefbfc4a0) at object.cc:315 [opt]
    frame #7: 0x0000000105271a9b boa.node`Napi::ObjectWrap<boa::PythonObject>::InstanceMethodCallbackWrapper(this=<unavailable>)::'lambda'()::operator()() const at napi-inl.h:3838:12 [opt]
    frame #8: 0x00000001052719ca boa.node`Napi::ObjectWrap<boa::PythonObject>::InstanceMethodCallbackWrapper(napi_env__*, napi_callback_info__*) [inlined] napi_value__* Napi::details::WrapCallback<Napi::ObjectWrap<boa::PythonObject>::InstanceMethodCallbackWrapper(napi_env__*, napi_callback_info__*)::'lambda'()>(Napi::ObjectWrap<boa::PythonObject>::InstanceMethodCallbackWrapper(napi_env__*, napi_callback_info__*)::'lambda'()) at napi-inl.h:73:12 [opt]
    frame #9: 0x00000001052719c5 boa.node`Napi::ObjectWrap<boa::PythonObject>::InstanceMethodCallbackWrapper(env=0x000000010500b280, info=0x00007ffeefbfc580) at napi-inl.h:3831 [opt]
    frame #10: 0x000000010006346a node`v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) + 122
    frame #11: 0x00000001002580c8 node`v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) + 616
    frame #12: 0x000000010025765c node`v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) + 524
    frame #13: 0x0000000100256d82 node`v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) + 258
    frame #14: 0x0000000100a50f59 node`Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit + 57

This is addressed, the sub-thread would call to the finalize_interpreter() which causes the following calls to Python would crash, just added an extra state initialized to flag the finalization op must be in the main thread and Py_IsInitialized() is true.

@yorkie
Copy link
Member Author

yorkie commented Oct 1, 2020

@FeelyChau See https://github.com/alibaba/pipcook/pull/602/checks?check_run_id=1192579536, it seems failed by daemon tests which are not related to this PR.

@FeelyChau
Copy link
Collaborator

@FeelyChau See https://github.com/alibaba/pipcook/pull/602/checks?check_run_id=1192579536, it seems failed by daemon tests which are not related to this PR.

OK, I'll check it out.

@yorkie yorkie mentioned this pull request Oct 9, 2020
WenheLI
WenheLI previously approved these changes Oct 10, 2020
Copy link
Collaborator

@WenheLI WenheLI left a comment

Choose a reason for hiding this comment

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

LGTM

packages/boa/lib/proxy.js Outdated Show resolved Hide resolved
if (_ownership == nullptr) {
return true;
} else {
bool owned = _ownership->getOwned();
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems not thread-safe here.

private:
mutable std::mutex _mtx;
PyObject *_pyobject;
bool _owned;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can use std::atomic_flag instead to improve performance.

Co-authored-by: feely <feely@outlook.com>
@FeelyChau
Copy link
Collaborator

LGTM.

@FeelyChau FeelyChau merged commit 1b50e4e into master Oct 10, 2020
@FeelyChau FeelyChau deleted the boa/add-worker-threads-support branch October 10, 2020 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
boa Python related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants