dicom-rs icon indicating copy to clipboard operation
dicom-rs copied to clipboard

Starting the discussion on async support

Open jennydaman opened this issue 1 year ago • 3 comments

I see on the roadmap that there is the intention for async support.

https://github.com/Enet4/dicom-rs/wiki/Roadmap#async-api

I am trying to write (what I think is) a SCP, mostly based on the code of storescp/src/main.rs. For each received DICOM file, I need to make an HTTP request, so the program could be much faster if dicom-rs supported async. Also, I would like to be able to use some of my favorite crates, opentelemetry (which prefers tokio) and fs_err (which, for async, requires tokio).

I have identified FileDicomObj::write_to_file and the TcpStream accepting methods of dicom_ul::association::ServerAssociationOptions as befitting of async.

@Enet4 are you currently working on async? If not, how would you like to approach it?

A key question is: do we want to lock ourselves into Tokio?

jennydaman avatar Mar 15 '24 16:03 jennydaman

Actually, a bit of Rust circlejerking: I did benchmarks and as-is, the SCP service I wrote is too fast for the Python Django backend it interacts with. So there's no real motivation for me to optimize it further...

jennydaman avatar Mar 15 '24 17:03 jennydaman

Thank you for the initiative, @jennydaman. Going async for DICOM network operations is unquestionably the way forward for the project.

I have identified FileDicomObj::write_to_file and the TcpStream accepting methods of dicom_ul::association::ServerAssociationOptions as befitting of async.

You are right that the best place to start with async support would be at the dicom_ul association API, preferably by offering a nonblocking and holding a blocking version for a smoother transition (or in scenarios where this isn't very important). Async file reading and writing could be a subsequent step, but I feel that non-blocking network is more important.

@Enet4 are you currently working on async? If not, how would you like to approach it?

I remember doing some tinkering to find out the implications of making an async version of associations, though I did not have much done here. I do recall that there was a need to rewrite the PDU reading logic, due to the direct use of blocking Read calls everywhere. I am likely only resuming this until after 0.7.0 is released, but if you would like I can push a WIP branch of what I have so others can continue building on top of it.

A key question is: do we want to lock ourselves into Tokio?

Using Tokio is a reasonable approach. The Tokio ecosystem is powerful and mature, making it much easier to build what we want without resorting to complicated low-level constructs.

Actually, a bit of Rust circlejerking: I did benchmarks and as-is, the SCP service I wrote is too fast for the Python Django backend it interacts with. So there's no real motivation for me to optimize it further...

That is fine, it's great to know that the current implementation works for you. :) In any case there is a place for async constructs in other scenarios.

Enet4 avatar Mar 16 '24 12:03 Enet4

I started a very rough draft in #463 which only adds the async at the application level, not in the library. To be fair, I haven't done that much async in any language except maybe JS, but I read this blog post the other day and it seems like adding it at the library level is hard

naterichman avatar Mar 22 '24 20:03 naterichman