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

multithread / multiprocess safe dataset reads #84

Merged
merged 9 commits into from Jul 11, 2019

Conversation

rlizzo
Copy link
Member

@rlizzo rlizzo commented Jul 7, 2019

Motivation and Context

Why is this change required? What problem does it solve?:

This is the initial work on making hangar reads thread/process safe. The batch_get function is the preffered method for user interaction.

If it fixes an open issue, please link to the issue here:

Addresses some of the requirements to enable: #13

Description

Describe your changes in detail:

DatasetDataReader class no longer contains references to lmdb environments. This change was made as it cannot be serialized and sent across processes.

Types of changes

What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Is this PR ready for review, or a work in progress?

  • Ready for review
  • Work in progress

How Has This Been Tested?

Put an x in the boxes that apply:

  • Current tests cover modifications made
  • New tests have been added to the test suite
  • Modifications were made to existing tests to support these changes
  • Tests may be needed, but they are not included when the PR was proposed
  • I don't know. Help!

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have signed (or will sign when prompted) the tensorwork CLA.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@rlizzo rlizzo added enhancement New feature or request WIP Don't merge; Work in Progress labels Jul 7, 2019
@rlizzo rlizzo added this to the V0.2.0 Release milestone Jul 7, 2019
@rlizzo rlizzo self-assigned this Jul 7, 2019
@rlizzo rlizzo requested a review from hhsecond July 7, 2019 11:32
@rlizzo rlizzo force-pushed the enable-multiprocess-reads branch from 9461637 to d5426b4 Compare July 8, 2019 17:00
src/hangar/dataset.py Outdated Show resolved Hide resolved
src/hangar/dataset.py Outdated Show resolved Hide resolved
@rlizzo rlizzo removed the WIP Don't merge; Work in Progress label Jul 11, 2019
@rlizzo rlizzo changed the title WIP: multithread / multiprocess safe dataset reads multithread / multiprocess safe dataset reads Jul 11, 2019
@rlizzo
Copy link
Member Author

rlizzo commented Jul 11, 2019

Merging now!

@rlizzo
Copy link
Member Author

rlizzo commented Jul 11, 2019

Thanks for the review @hhsecond!

@rlizzo rlizzo merged commit ca08ff4 into tensorwerk:master Jul 11, 2019
@rlizzo rlizzo deleted the enable-multiprocess-reads branch August 3, 2019 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants