Skip to content

Fix persistent_workers + pin_memory#48543

Closed
ssnl wants to merge 1 commit intopytorch:masterfrom
ssnl:dl_persistent_pin_memory
Closed

Fix persistent_workers + pin_memory#48543
ssnl wants to merge 1 commit intopytorch:masterfrom
ssnl:dl_persistent_pin_memory

Conversation

@ssnl
Copy link
Copy Markdown
Collaborator

@ssnl ssnl commented Nov 28, 2020

Fixes #48370 #47445

cc @emcastillo who authored the original functionality.

@ssnl ssnl requested a review from apaszke as a code owner November 28, 2020 22:07
@ssnl ssnl removed the request for review from apaszke November 28, 2020 22:07
@ssnl ssnl requested a review from VitalyFedyunin November 28, 2020 22:07
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 29, 2020

Codecov Report

Merging #48543 (c1081cd) into master (4ed7f36) will increase coverage by 0.00%.
The diff coverage is 75.00%.

@@           Coverage Diff           @@
##           master   #48543   +/-   ##
=======================================
  Coverage   80.94%   80.95%           
=======================================
  Files        1854     1854           
  Lines      199918   199919    +1     
=======================================
+ Hits       161831   161840    +9     
+ Misses      38087    38079    -8     

@ejguan ejguan added module: dataloader Related to torch.utils.data.DataLoader and Sampler triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Nov 30, 2020
if isinstance(r, _ResumeIteration):
# Acknowledge the main process
data_queue.put(r)
data_queue.put((r, None))
Copy link
Copy Markdown
Contributor

@ejguan ejguan Nov 30, 2020

Choose a reason for hiding this comment

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

Should r be the data? Maybe use worker_id as the index.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Well, it doesn't really matter. But I prefer _ResumeIteration being the index since integer indices are associated with actual data.

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@ssnl
Copy link
Copy Markdown
Collaborator Author

ssnl commented Dec 29, 2020

@VitalyFedyunin ping

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@ejguan merged this pull request in 54ce171.

hwangdeyu pushed a commit to hwangdeyu/pytorch that referenced this pull request Jan 14, 2021
Summary:
Fixes pytorch#48370 pytorch#47445

cc emcastillo who authored the original functionality.

Pull Request resolved: pytorch#48543

Reviewed By: bdhirsh

Differential Revision: D25277474

Pulled By: ejguan

fbshipit-source-id: 1967002124fb0fff57caca8982bc7df359a059a2
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
Fixes pytorch#48370 pytorch#47445

cc emcastillo who authored the original functionality.

Pull Request resolved: pytorch#48543

Reviewed By: bdhirsh

Differential Revision: D25277474

Pulled By: ejguan

fbshipit-source-id: 1967002124fb0fff57caca8982bc7df359a059a2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla signed Merged module: dataloader Related to torch.utils.data.DataLoader and Sampler open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error when dataloader has pinned memory and persistent workers

5 participants