furiosa-ai / device-api

APIs that offers NPU devices' information and allow to control the devices
Apache License 2.0
5 stars 8 forks source link

Remove unnecessary async keyword in hwmon handler #54

Closed libc-furiosa closed 1 year ago

libc-furiosa commented 1 year ago

Functions in hwmon module have async keyword but doesn't really work meaningfully. (just sequential async-await) For convenience, it would be better to change to sync. (Some other APIs already applied) ( #41 )

hyunsik commented 1 year ago

I agree with that the IO operations are trivial and run sequentially. async/await may not be able to impact on performance or throughput.

To make a better decision, let's also think about the role of async/await as yield points. The reason why we use async/await is basically concurrency. In more details, async/await makes multiple tasks to run concurrently even in a single thread through cooperative multi-tasking which are based on yield control.

async function is a way to generate yield points implicitly and automatically. The main difference of tokio::fs from std::fs is that tokio::fs internally use a blocking thread pool and schedule IO operations to the blocking thread pool. So, tokio::fs doesn't block tokio executor and yield when any IO operation is called.

When users develop some applications with async/await version like monitoring system for dozen NPU cards, users don't have to worry about the blocking calls within async tasks. If users use the std::fs version, they may need to be concerned with the IO blocking calls within their applications using async executor.

libc-furiosa commented 1 year ago

Thanks for the detailed explanation. I agree with your argument.

It seems that the API needs to be improved in the near future. I think we have two options at this point. Whether to temporarily approve this PR and allow only blocking calls explicitly, or discard this PR and keep the existing implementation.

Please tell me your opinions.

hyunsik commented 1 year ago

If we are not sure, let's keep the current code.

libc-furiosa commented 1 year ago

Thank you. Close this PR.