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

Enable miniepoch for MultiProcessingReadingService #1170

Closed
wants to merge 1 commit into from

Conversation

ejguan
Copy link
Contributor

@ejguan ejguan commented May 23, 2023

Summary:
A retry of #1147

  • Add pause, resume, limit API to MultiProcessingReadingService to support miniepoch API.
  • When limit is requested, IterQueueWrapper will make sure only limit number of requests are sent to worker process and dispatching proccess.
  • Consolidate a few API/messages
    • reset and reset_epoch is merged as a single message
    • Process reset function is shared for both worker process and dispatching process
  • For pause/resume/limit/reset_epoch, add a counter to sync between loops to make sure the DataPipe graph is in-place modified once.
  • Remove unused thread eventloop

Differential Revision: D45666912

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels May 23, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45666912

ejguan added a commit to ejguan/data that referenced this pull request May 23, 2023
…ch#1170)

Summary:
Pull Request resolved: pytorch#1170

A retry of pytorch#1147
- Add pause, resume, limit API to MultiProcessingReadingService to support miniepoch API.
- When limit is requested, `IterQueueWrapper` will make sure only `limit` number of requests are sent to worker process and dispatching proccess.
- Consolidate a few API/messages
  - reset and reset_epoch is merged as a single message
  - Process reset function is shared for both worker process and dispatching process
- For pause/resume/limit/reset_epoch, add a counter to sync between loops to make sure the DataPipe graph is in-place modified once.
- Remove unused thread eventloop

Differential Revision: D45666912

fbshipit-source-id: 48176f10354494754f47a96a1ca166ed45e7e247
@ejguan ejguan force-pushed the export-D45666912 branch from 7305e57 to ea93434 Compare May 23, 2023 21:48
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45666912

ejguan added a commit to ejguan/data that referenced this pull request May 23, 2023
…ch#1170)

Summary:
Pull Request resolved: pytorch#1170

A retry of pytorch#1147
- Add pause, resume, limit API to MultiProcessingReadingService to support miniepoch API.
- When limit is requested, `IterQueueWrapper` will make sure only `limit` number of requests are sent to worker process and dispatching proccess.
- Consolidate a few API/messages
  - reset and reset_epoch is merged as a single message
  - Process reset function is shared for both worker process and dispatching process
- For pause/resume/limit/reset_epoch, add a counter to sync between loops to make sure the DataPipe graph is in-place modified once.
- Remove unused thread eventloop

Differential Revision: D45666912

fbshipit-source-id: a2e064371a510463cc13253419aa7d95233bf41e
@ejguan ejguan force-pushed the export-D45666912 branch from ea93434 to 73f7fd5 Compare May 23, 2023 22:19
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45666912

@ejguan ejguan force-pushed the export-D45666912 branch from 73f7fd5 to df62eab Compare May 23, 2023 22:26
@facebook-github-bot
Copy link
Contributor

@ejguan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

ejguan added a commit to ejguan/data that referenced this pull request May 23, 2023
Summary:
A retry of pytorch#1147
- Add pause, resume, limit API to MultiProcessingReadingService to support miniepoch API.
- When limit is requested, `IterQueueWrapper` will make sure only `limit` number of requests are sent to worker process and dispatching proccess.
- Consolidate a few API/messages
  - reset and reset_epoch is merged as a single message
  - Process reset function is shared for both worker process and dispatching process
- For pause/resume/limit/reset_epoch, add a counter to sync between loops to make sure the DataPipe graph is in-place modified once.
- Remove unused thread eventloop

Pull Request resolved: pytorch#1170

Differential Revision: D45666912

Pulled By: ejguan

fbshipit-source-id: 1e88dde3af0f406c062ca523e7b96a3c79a74b6d
@ejguan ejguan force-pushed the export-D45666912 branch from df62eab to c8a52da Compare May 23, 2023 22:52
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45666912

@ejguan ejguan requested a review from NivekT May 24, 2023 15:14
Copy link
Contributor

@NivekT NivekT left a comment

Choose a reason for hiding this comment

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

A few questions/notes. Overall, it does look good but our codebase is getting very complicated.

ejguan added a commit to ejguan/data that referenced this pull request May 26, 2023
Summary:
A retry of pytorch#1147
- Add pause, resume, limit API to MultiProcessingReadingService to support miniepoch API.
- When limit is requested, `IterQueueWrapper` will make sure only `limit` number of requests are sent to worker process and dispatching proccess.
- Consolidate a few API/messages
  - reset and reset_epoch is merged as a single message
  - Process reset function is shared for both worker process and dispatching process
- For pause/resume/limit/reset_epoch, add a counter to sync between loops to make sure the DataPipe graph is in-place modified once.
- Remove unused thread eventloop

Pull Request resolved: pytorch#1170

Differential Revision: D45666912

Pulled By: ejguan

fbshipit-source-id: 1e88dde3af0f406c062ca523e7b96a3c79a74b6d
@ejguan ejguan force-pushed the export-D45666912 branch from c8a52da to 4a22cd2 Compare May 26, 2023 16:05
@facebook-github-bot
Copy link
Contributor

@ejguan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

ejguan added a commit to ejguan/data that referenced this pull request May 26, 2023
Summary:
A retry of pytorch#1147
- Add pause, resume, limit API to MultiProcessingReadingService to support miniepoch API.
- When limit is requested, `IterQueueWrapper` will make sure only `limit` number of requests are sent to worker process and dispatching proccess.
- Consolidate a few API/messages
  - reset and reset_epoch is merged as a single message
  - Process reset function is shared for both worker process and dispatching process
- For pause/resume/limit/reset_epoch, add a counter to sync between loops to make sure the DataPipe graph is in-place modified once.
- Remove unused thread eventloop

Pull Request resolved: pytorch#1170

Differential Revision: D45666912

Pulled By: ejguan

fbshipit-source-id: ddff582f35a25528a2b8a1751ea0d0359f866ed0
@ejguan ejguan force-pushed the export-D45666912 branch from 4a22cd2 to a7e0940 Compare May 26, 2023 16:39
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45666912

ejguan added a commit to ejguan/data that referenced this pull request May 26, 2023
Summary:
A retry of pytorch#1147
- Add pause, resume, limit API to MultiProcessingReadingService to support miniepoch API.
- When limit is requested, `IterQueueWrapper` will make sure only `limit` number of requests are sent to worker process and dispatching proccess.
- Consolidate a few API/messages
  - reset and reset_epoch is merged as a single message
  - Process reset function is shared for both worker process and dispatching process
- For pause/resume/limit/reset_epoch, add a counter to sync between loops to make sure the DataPipe graph is in-place modified once.
- Remove unused thread eventloop

Pull Request resolved: pytorch#1170

Differential Revision: D45666912

Pulled By: ejguan

fbshipit-source-id: f533892a1ffcdc103988a724bea8ce20572465b6
@ejguan ejguan force-pushed the export-D45666912 branch from a7e0940 to 951db08 Compare May 26, 2023 16:45
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45666912

ejguan added a commit to ejguan/data that referenced this pull request May 26, 2023
Summary:
A retry of pytorch#1147
- Add pause, resume, limit API to MultiProcessingReadingService to support miniepoch API.
- When limit is requested, `IterQueueWrapper` will make sure only `limit` number of requests are sent to worker process and dispatching proccess.
- Consolidate a few API/messages
  - reset and reset_epoch is merged as a single message
  - Process reset function is shared for both worker process and dispatching process
- For pause/resume/limit/reset_epoch, add a counter to sync between loops to make sure the DataPipe graph is in-place modified once.
- Remove unused thread eventloop

Pull Request resolved: pytorch#1170

Differential Revision: D45666912

Pulled By: ejguan

fbshipit-source-id: 40ec4eef0fdf1de05d3a545e08c11f155446ae2d
@ejguan ejguan force-pushed the export-D45666912 branch from 951db08 to 8f6cb7d Compare May 26, 2023 16:51
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45666912

ejguan added a commit to ejguan/data that referenced this pull request May 26, 2023
Summary:
A retry of pytorch#1147
- Add pause, resume, limit API to MultiProcessingReadingService to support miniepoch API.
- When limit is requested, `IterQueueWrapper` will make sure only `limit` number of requests are sent to worker process and dispatching proccess.
- Consolidate a few API/messages
  - reset and reset_epoch is merged as a single message
  - Process reset function is shared for both worker process and dispatching process
- For pause/resume/limit/reset_epoch, add a counter to sync between loops to make sure the DataPipe graph is in-place modified once.
- Remove unused thread eventloop

Pull Request resolved: pytorch#1170

Differential Revision: D45666912

Pulled By: ejguan

fbshipit-source-id: ffafcbb0b78add9c863d2faaec7248332ef36f68
@ejguan ejguan force-pushed the export-D45666912 branch from 8f6cb7d to 6b5a3b9 Compare May 26, 2023 16:59
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45666912

Summary:
A retry of pytorch#1147
- Add pause, resume, limit API to MultiProcessingReadingService to support miniepoch API.
- When limit is requested, `IterQueueWrapper` will make sure only `limit` number of requests are sent to worker process and dispatching proccess.
- Consolidate a few API/messages
  - reset and reset_epoch is merged as a single message
  - Process reset function is shared for both worker process and dispatching process
- For pause/resume/limit/reset_epoch, add a counter to sync between loops to make sure the DataPipe graph is in-place modified once.
- Remove unused thread eventloop

Pull Request resolved: pytorch#1170

Differential Revision: D45666912

Pulled By: ejguan

fbshipit-source-id: 7f4d208e3ff0a6bea4688862f7b8d5456af8e7f6
@ejguan ejguan force-pushed the export-D45666912 branch from 6b5a3b9 to 62f06c7 Compare May 26, 2023 17:10
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45666912

@ejguan ejguan requested a review from NivekT May 26, 2023 17:23
Copy link
Contributor

@NivekT NivekT left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@facebook-github-bot
Copy link
Contributor

@ejguan merged this pull request in bc0650a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants