Open3D icon indicating copy to clipboard operation
Open3D copied to clipboard

Release Python GIL in Eigen API

Open ssheorey opened this issue 3 years ago • 8 comments

Type

  • [x] Bug fix (non-breaking change which fixes an issue): Fixes #5837
  • [ ] New feature (non-breaking change which adds functionality). Resolves #
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

Release Python GIL in estimate normals, optimize pose graph, fpfh computation. Allow Python multithreading for these operations.

Update: Release Python GIL in most Eigen API functions for geometry and pipelines. Updates to visualization, ML and newer Tensor API modules is future work.

Note to users:

  • Do not rely on the Python GIL to protect user data (Open3D data structures). When using Python multi-threading, use separate locks to protect data from concurrent access. This PR may break user code that used Python GIL in this way earlier.
  • Many Open3D functions already use multithreading automatically based on the number of CPUs in the system. Programs will likely not see speed increase by calling multiple such Open3D functions in parallel. This PR does offer benefits of running IO (e.g. Open3D / Python file reading) and compute (e.g. Open3D registration) in parallel. Some Open3D compute intensive functions are single threaded and can benefit from being run in parallel as well. Check the CPU usage of your workloads to check if Python multithreading would be useful.

Checklist:

  • [x] I have run python util/check_style.py --apply to apply Open3D code style to my code.
  • [ ] This PR changes Open3D behavior or adds new functionality.
    • [ ] Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is updated accordingly.
    • [ ] I have added or updated C++ and / or Python unit tests OR included test results (e.g. screenshots or numbers) here.
  • [x] I will follow up and update the code if CI fails.
  • [ ] For fork PRs, I have selected Allow edits from maintainers.

This change is Reviewable

ssheorey avatar Feb 03 '23 23:02 ssheorey

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

update-docs[bot] avatar Feb 03 '23 23:02 update-docs[bot]

Hi @ssheorey , should we also release the pyhton GIL for methods of tensor based PointCloud?

yuecideng avatar Feb 04 '23 04:02 yuecideng

Hi @ssheorey , should we also release the pyhton GIL for methods of tensor based PointCloud?

Updated PR to be more comprehensive. Added user notes about potential benefits and user warning.

ssheorey avatar Feb 08 '23 06:02 ssheorey

Pleasy please give it a watch?

I have a very involved program and calculating fpfh features on the main process would be very cumbersome.

Thanks to everyone for the wonderful work.

tiglieto avatar Jun 01 '23 22:06 tiglieto

@ssheorey is there any significant changes required? Otherwise I think we can make it ready for review?

theNded avatar Aug 11 '23 04:08 theNded

@theNded This PR removed the GIL for all eigen based functions and many unit tests failed. I need to look at each of the failing unit tests and selectively remove GIL. It may take some time for me to get around to this.

ssheorey avatar Aug 11 '23 04:08 ssheorey

@theNded This PR removed the GIL for all eigen based functions and many unit tests failed. I need to look at each of the failing unit tests and selectively remove GIL. It may take some time for me to get around to this.

I can do this if these are the only required changes.

theNded avatar Aug 11 '23 04:08 theNded

@theNded This PR removed the GIL for all eigen based functions and many unit tests failed. I need to look at each of the failing unit tests and selectively remove GIL. It may take some time for me to get around to this.

I can do this if these are the only required changes.

Yes that should be all. Thanks for picking this up!

ssheorey avatar Aug 11 '23 05:08 ssheorey