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

WIP: Add runtime selection of GIL implementation #1322

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

KyleFromKitware
Copy link

@KyleFromKitware KyleFromKitware commented Mar 14, 2018

Fixes #1276.

Example usage:

#include <pybind11/pybind11.h>
#include <pybind11/embed.h>

#include <iostream>

int main() {
  pybind11::scoped_interpreter interp;
  pybind11::select_gil_impl<pybind11::basic_gil_impl>();

  pybind11::gil_scoped_release release;
  std::cout << "Running C++ code" << std::endl;

  return 0;
}

The select_gil_impl() call notifies pybind that this particular program requires the basic_gil_impl implementation. If another executable or library has already called select_gil_impl() with a different, incompatible implementation, an exception is thrown. If no library or executable ever calls select_gil_impl(), the basic_gil_impl is used by default. (Whether we should use basic or advanced by default is an ongoing discussion. I personally am in favor of basic being the default.)

If we want to use the advanced API with the thread disassociation feature:

#include <pybind11/pybind11.h>
#include <pybind11/embed.h>

#include <iostream>

int main() {
  pybind11::scoped_interpreter interp;
  pybind11::select_gil_impl<pybind11::advanced_gil_impl>();

  pybind11::advanced_gil_impl::release release(true);
  std::cout << "Running C++ code" << std::endl;

  return 0;
}

advanced_gil_impl doesn't currently do any checks to see if basic_gil_impl has already been used. That has been left up to the gil_scoped_release internals.

Important note: this PR does introduce two incompatible changes requiring a major version update:

  1. The advanced API is no longer used by default. This is easy to change.
  2. You can no longer call gil_scoped_release with the boolean disassociation flag. You have to use advanced_gil_impl::release instead. We can certainly do some shuffling of names such that gil_scoped_acquire and gil_scoped_release serve as typedefs of the advanced API, and then the "real" API would be something like auto_gil_scoped_acquire and auto_gil_scoped_release.

This is the first draft of a WIP. It should serve as a good first sketch of what we want this to look like. Comments welcome, I look forward to hearing your feedback.

@wjakob
Copy link
Member

wjakob commented Mar 14, 2018

See higher-level comments on #1276.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Double-nested gil_scoped_release in a pythread causes fatal interpreter error
2 participants