Skip to content

Remove boost from core popsift library#81

Merged
simogasp merged 16 commits intoalicevision:developfrom
andrew-hardin:feature/remove-boost-from-core
Apr 20, 2020
Merged

Remove boost from core popsift library#81
simogasp merged 16 commits intoalicevision:developfrom
andrew-hardin:feature/remove-boost-from-core

Conversation

@andrew-hardin
Copy link
Copy Markdown
Contributor

This PR strips the boost dependency from the core popsift library. This don't alter the existing code's behavior, and the changes only make use of C++11 features. Here's the list of replaced components:

  • boost::thread was replaced by std::thread.
  • boost::sync_queue was replaced by a new class called popsift::SyncQueue. I'll admit that this doesn't have feature parity with boost, but it does include all the required methods: push(), pull(), and empty().

The value of this PR (at least for me) is that it shrinks the required prerequisites for the popsift library - it only depends on the CUDA toolkit. This makes the library more atomic, and it adds support for RHEL/CentOS 7 whose official packages are stuck at Boost 1.53.

Thank you for considering these changes.

@simogasp simogasp requested a review from griwodz April 10, 2020 22:09
@simogasp simogasp added this to the v1.0.0 milestone Apr 10, 2020
Copy link
Copy Markdown
Member

@simogasp simogasp left a comment

Choose a reason for hiding this comment

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

Hi thank you very much for your contribution, it is really cool if we can just use std library.
I have just few minor remarks.

Thanks!

Comment thread src/popsift/common/sync_queue.h Outdated
Comment thread CMakeLists.txt Outdated
Comment thread src/application/CMakeLists.txt Outdated
Comment thread src/CMakeLists.txt Outdated
Comment thread src/cmake/Config.cmake.in Outdated
@simogasp
Copy link
Copy Markdown
Member

simogasp commented Apr 20, 2020

@griwodz is it ok for you the replacement of boost::sync_queue with the new smaller in-house class?

Copy link
Copy Markdown
Member

@griwodz griwodz left a comment

Choose a reason for hiding this comment

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

These changes for work for me, and they are an improvement. They remove the mix of std::future and boost::thread that was in the code before.

@griwodz griwodz requested a review from simogasp April 20, 2020 16:52
@simogasp simogasp merged commit 149de05 into alicevision:develop Apr 20, 2020
@simogasp
Copy link
Copy Markdown
Member

Thanks @andrew-hardin for make it simpler!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants