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

Optimisations for slow dataset transfers on Windows #620

Closed
marcmorcos opened this issue May 4, 2021 · 6 comments · Fixed by #627
Closed

Optimisations for slow dataset transfers on Windows #620

marcmorcos opened this issue May 4, 2021 · 6 comments · Fixed by #627

Comments

@marcmorcos
Copy link

marcmorcos commented May 4, 2021

Describe the bug
Large RT Dose (80MB) takes about 1 minute to get stored. Not sure where bottle neck is exactly but this line is printed right away:
D: pydicom.read_dataset() TransferSyntax="Little Endian Implicit"

Then we wait.... a minute later this line and the file transfer is complete shortly after (almost instantaneously):
I: Received Store Request

So things work... Just very slowly. Have tested CTs, RPs and RSs (images, plans and structures) and those are all lightning fast... of course they are also much smaller.

Steps To Reproduce
Using tutorial example from here.

Have tried pynetdicom versions 1.4.1 and 1.5.7 with same behavior.

To rule out network issues, the DICOM exporter (clinical software) and pynetdicom SCP are both sitting on the same PC (in otherwords, AE IP is localhost).

My environment

$ python -c "import platform; print(platform.platform())"
Windows-10-10.0.19041-SP0

$ python -c "import sys; print('Python ', sys.version)"
Python  3.9.1 (default, Dec 11 2020, 09:29:25) [MSC v.1916 64 bit (AMD64)]

$ python -c "import pydicom; print('pydicom ', pydicom.__version__)"
pydicom  2.1.2

$ python -c "import pynetdicom; print('pynetdicom ', pynetdicom.__version__)"
pynetdicom  1.5.7
@scaramallion
Copy link
Member

scaramallion commented May 4, 2021

Have you tried increasing AE.maximum_pdu_size? Or setting it to 0 (unlimited)?

Have you tried comparing it against something like DCMTK's storescp?

There was also this similar issue on Windows that might be related.

I just ran test 5 in the benchmarking script, which should be similar to the tutorial code, on the 2.1 MB RTImageStorage dataset and it took 209 s to transfer 2.1 GB, which means your dataset should take about 8 s. For comparison, test 6 which uses an unlimited PDU size took about 45 s for all 2.1 GB. That's on Ubuntu, though.

@marcmorcos
Copy link
Author

DCMTK (with max PDU as default or max) is instant (<<0.5 sec, timed by hand...) for the same (80MB) file.
Per your suggestion, setting maximum_pdu_size=0 reduces time from 80 to 10 seconds. A major improvement, thank you.

I wonder if we can further reduce the time and approach DCMTKs performance.

@scaramallion
Copy link
Member

I'd take a look at the suggestion towards the bottom of the linked issue about changing Window's timer resolution as well.

@marcmorcos
Copy link
Author

marcmorcos commented May 5, 2021

Ok wow. setting the time resolution to 1ms made a huge difference!
Now transfers in 1.6 seconds (PDU unlimited also needed).

To be safe, I set the timer resolution to 1 ms @ EVT_ACCEPTED, and unset it @ EVT_RELEASED.
I figure this way we avoid affecting the thread scheduling globally which can degrade system performance and power management. Do you think this is the way to do it? I'm not sure how I could unset it if I just used EVT_REQUESTED.

Essentially what I'm doing:

def accepted(event, ntdll):
       ntdll.timeBeginPeriod(1)
...

def released(event, ntdll):
       ntdll.timeEndPeriod(1)
...
def main():
      ntdll = ctypes.WinDLL('WINMM.DLL')

      handlers = [(evt.EVT_ACCEPTED, accepted, [ntdll]),
                          (evt.EVT_RELEASED, release, [ntdll])
                         ]
...

What do you think? Thanks again.

@scaramallion
Copy link
Member

scaramallion commented May 5, 2021

Yup that seems pretty good. I've been thinking I should probably set that automatically when on Windows so users don't have to worry about it.

Maybe also include EVT_ABORTED

@scaramallion scaramallion changed the title Slow to transfer large RT Dose. Pynetdicom on same PC as dicom exporter. Optimisations for slow dataset transfers on Windows May 5, 2021
@marcmorcos
Copy link
Author

Thank you for your help and contributions!

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

Successfully merging a pull request may close this issue.

2 participants