Fix dataloader hang when it is not completely iterated #9655

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.

Sign up for GitHub

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

Jump to bottom

Closed

ssnl wants to merge 5 commits into pytorch:master from ssnl:dl

Collaborator

ssnl commented Jul 20, 2018

second trial of #7140

cc @csarofeen Let's see if this works. It passes everything locally.

ssnl requested review from apaszke, colesbury, ezyang, gchanan, soumith and zdevito as code owners

July 20, 2018 20:19

ssnl force-pushed the dl branch from a6a7efd to 0b76b97 Compare

July 20, 2018 20:20

facebook-github-bot reviewed

View reviewed changes

Contributor

facebook-github-bot left a comment

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

ssnl force-pushed the dl branch from 0b76b97 to be39e60 Compare

July 20, 2018 20:31

Collaborator Author

ssnl commented Jul 20, 2018

@pytorchbot retest this please

ssnl force-pushed the dl branch from be39e60 to 4ad0b89 Compare

July 21, 2018 02:52

facebook-github-bot reviewed

View reviewed changes

Contributor

facebook-github-bot left a comment

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

ssnl mentioned this pull request

Dataloader shutdown hangs #9539

Closed

ssnl added 4 commits

July 21, 2018 11:23


          Change self.worker_result_queue to Queue


          test ci

3aa48a6


          fix hang; improve timeout by passing done_event to worker

bda1b03


          add test for partial loader and main process error

30b8b2e

ssnl force-pushed the dl branch from 4ad0b89 to 30b8b2e Compare

July 21, 2018 15:24

facebook-github-bot reviewed

View reviewed changes

Contributor

facebook-github-bot left a comment

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

Contributor

csarofeen commented Jul 21, 2018

This is something I was thinking of too. I don't know why but it still produces (very occasionally) a hang. I totally agree it should work, but will try to figure out why it still hangs on occasion.

Collaborator Author

ssnl commented Jul 21, 2018

@pytorchbot retest this please

Contributor

csarofeen commented Jul 21, 2018

Been running a script over and over today to check, only happened twice out of many hundred so who knows. I think it should be fine.

Collaborator Author

ssnl commented Jul 22, 2018 via email

Did it hang at one of the joins? If so, which one was it?

On Sat, Jul 21, 2018 at 19:42 Christian Sarofeen ***@***.***> wrote: Been running a script over and over today to check, only happened twice out of many hundred so who knows. I think it should be fine. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9655 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFaWZQDO37R8N15StnMXOW7c3F3pcb-Fks5uI7xVgaJpZM4VZN8l> .

Collaborator Author

ssnl commented Jul 22, 2018 via email

Thanks for checking! :) On Sat, Jul 21, 2018 at 23:26 Tongzhou Wang <[email protected]> wrote:

Did it hang at one of the joins? If so, which one was it? On Sat, Jul 21, 2018 at 19:42 Christian Sarofeen ***@***.***> wrote: > Been running a script over and over today to check, only happened twice > out of many hundred so who knows. I think it should be fine. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#9655 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AFaWZQDO37R8N15StnMXOW7c3F3pcb-Fks5uI7xVgaJpZM4VZN8l> > . >

apaszke approved these changes

View reviewed changes

Contributor

apaszke left a comment

Generally looks look, but I'd rather get rid of the done_event unless it's necessary

torch/utils/data/dataloader.py

    
                  torch.manual_seed(seed)

                  # Do not wait for putting thread to join when this worker exits. Otherwise,

                  # this worker may always be waiting to put and doesn't check index_queue

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

torch/utils/data/dataloader.py

    
                      if r is None:

                      # use done_event so that we can get faster exiting signal even if there

                      # are still indices in index_queue

                      if r is None or done_event.is_set():

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

torch/utils/data/dataloader.py

    
                          self.index_queues = [multiprocessing.Queue() for _ in range(self.num_workers)]

                          self.worker_queue_idx = 0

                          self.worker_result_queue = multiprocessing.SimpleQueue()

                          self.worker_result_queue = multiprocessing.Queue()

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

torch/utils/data/dataloader.py Outdated

    
                          if self.pin_memory or self.timeout > 0:

                          if self.pin_memory:

                              self.data_queue = queue.Queue()

                              if self.pin_memory:

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

torch/utils/data/dataloader.py Outdated

    
                          # removes pids no matter what

                      if not self.shutdown:

                          self.shutdown = True

                          self.done_event.set()

This comment was marked as off-topic.

Sign in to view

test/test_dataloader.py

    
                      time.sleep(self.sleep_sec)

                      if not self.sleeped:

                          time.sleep(self.sleep_sec)

                          self.sleeped = True

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view


          address comments

4a11713

facebook-github-bot reviewed

View reviewed changes

Contributor

facebook-github-bot left a comment

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

apaszke approved these changes

View reviewed changes

torch/utils/data/dataloader.py

    
                      if r is None:

                      # use done_event so that we can get faster exiting signal even if there

                      # are still indices in index_queue

                      if r is None or done_event.is_set():

This comment was marked as off-topic.

Sign in to view

torch/utils/data/dataloader.py

    
                          self.index_queues = [multiprocessing.Queue() for _ in range(self.num_workers)]

                          self.worker_queue_idx = 0

                          self.worker_result_queue = multiprocessing.SimpleQueue()

                          self.worker_result_queue = multiprocessing.Queue()

This comment was marked as off-topic.

Sign in to view

facebook-github-bot closed this in

9ee5133

ssnl added a commit that referenced this pull request


          Fix dataloader hang when it is not completely iterated (#9655)

2edf053

Summary:
second trial of #7140

cc csarofeen Let's see if this works. It passes everything locally.
Pull Request resolved: #9655

Differential Revision: D8940177

Pulled By: SsnL

fbshipit-source-id: 8d6340fc9f7355c71e1e26b262da166402faa158

ssnl deleted the dl branch

July 23, 2018 03:55

ssnl restored the dl branch

July 25, 2018 02:48

ssnl added a commit to ssnl/pytorch that referenced this pull request


          Revert "Fix dataloader hang when it is not completely iterated (pytor…

a37cf20

…ch#9655)"

This reverts commit 9ee5133.

ssnl added a commit to ssnl/pytorch that referenced this pull request


          Revert "Fix dataloader hang when it is not completely iterated (pytor…

d93828f

…ch#9655)"

This reverts commit 9ee5133.

ssnl added a commit that referenced this pull request


          Revert "Fix dataloader hang when it is not completely iterated (#9655)"

f34528a

This reverts commit 9ee5133.

facebook-github-bot pushed a commit that referenced this pull request


          Revert "Fix dataloader hang when it is not completely iterated (#9655)…

a7f183f

…" (#9804)

Summary:
This reverts commit 9ee5133.
Pull Request resolved: #9804

Reviewed By: ezyang

Differential Revision: D8987780

Pulled By: SsnL

fbshipit-source-id: 75ad70b0b8d672d0b35235fa248b187be64b68e5

jramseyer pushed a commit to jramseyer/pytorch that referenced this pull request


          Fix dataloader hang when it is not completely iterated (pytorch#9655)

045a39e

Summary:
second trial of pytorch#7140

cc csarofeen Let's see if this works. It passes everything locally.
Pull Request resolved: pytorch#9655

Differential Revision: D8940177

Pulled By: SsnL

fbshipit-source-id: 8d6340fc9f7355c71e1e26b262da166402faa158

jramseyer pushed a commit to jramseyer/pytorch that referenced this pull request


          Revert "Fix dataloader hang when it is not completely iterated (pytor…

…ch#9655)" (pytorch#9804)

Summary:
This reverts commit 9ee5133.
Pull Request resolved: pytorch#9804

Reviewed By: ezyang

Differential Revision: D8987780

Pulled By: SsnL

fbshipit-source-id: 75ad70b0b8d672d0b35235fa248b187be64b68e5

ssnl mentioned this pull request

Resubmit: Fix dataloader hang when it is not completely iterated #10366

Closed

facebook-github-bot pushed a commit that referenced this pull request


          Resubmit: Fix dataloader hang when it is not completely iterated (#10366

04f3816

)

Summary:
#9655
Pull Request resolved: #10366

Differential Revision: D9237393

Pulled By: SsnL

fbshipit-source-id: fabfad7f371ba33300098f6b885c0e3f26c3e14a

PenghuiCheng pushed a commit to PenghuiCheng/pytorch that referenced this pull request


          Resubmit: Fix dataloader hang when it is not completely iterated (pyt…

f791bec

…orch#10366)

Summary:
pytorch#9655
Pull Request resolved: pytorch#10366

Differential Revision: D9237393

Pulled By: SsnL

fbshipit-source-id: fabfad7f371ba33300098f6b885c0e3f26c3e14a

goodlux pushed a commit to goodlux/pytorch that referenced this pull request


          Fix dataloader hang when it is not completely iterated (pytorch#9655)

ef13ba8

Summary:
second trial of pytorch#7140

cc csarofeen Let's see if this works. It passes everything locally.
Pull Request resolved: pytorch#9655

Differential Revision: D8940177

Pulled By: SsnL

fbshipit-source-id: 8d6340fc9f7355c71e1e26b262da166402faa158

goodlux pushed a commit to goodlux/pytorch that referenced this pull request


          Revert "Fix dataloader hang when it is not completely iterated (pytor…

fb94ea4

…ch#9655)" (pytorch#9804)

Summary:
This reverts commit 9ee5133.
Pull Request resolved: pytorch#9804

Reviewed By: ezyang

Differential Revision: D8987780

Pulled By: SsnL

fbshipit-source-id: 75ad70b0b8d672d0b35235fa248b187be64b68e5

goodlux pushed a commit to goodlux/pytorch that referenced this pull request


          Resubmit: Fix dataloader hang when it is not completely iterated (pyt…

e85d0f7

…orch#10366)

Summary:
pytorch#9655
Pull Request resolved: pytorch#10366

Differential Revision: D9237393

Pulled By: SsnL

fbshipit-source-id: fabfad7f371ba33300098f6b885c0e3f26c3e14a

yingjianling mentioned this pull request

Distributed Training shut down on second epoch #11327

Closed

wsnedy mentioned this pull request

the training process hangs open-mmlab/mmdetection#99

Closed

ezyang added open source merged labels

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

Reviewers

facebook-github-bot facebook-github-bot left review comments

colesbury Awaiting requested review from colesbury

ezyang Awaiting requested review from ezyang

gchanan Awaiting requested review from gchanan

soumith Awaiting requested review from soumith

zdevito Awaiting requested review from zdevito

+1 more reviewer

apaszke apaszke approved these changes

Labels