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

Methods flagged by the avoid_slow_async_io lint should have better documentation #36269

Open
jamesderlin opened this issue Mar 19, 2019 · 8 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io type-documentation A request to add or improve documentation

Comments

@jamesderlin
Copy link
Contributor

jamesderlin commented Mar 19, 2019

I recently discovered the avoid_slow_async_io lint.

The documentation to File.exists, File.stat, et al. don't mention that they're much slower than their synchronous counterparts. If their synchronous counterparts should be preferred, we should say so.

(Why do File.exists, File.stat, et al. exist at all if their synchronous counterparts are faster? (Why do we even have that lever?) And it's too late to do now, but IMO if we wanted to discourage using the asynchronous versions, then the asynchronous versions should have been given longer names, not the synchronous versions.)

If there are some situations where the asynchronous versions should be used, then that probably should be mentioned in the documentation for the avoid_slow_async_io lint.

  • dartanalyzer version 2.1.0
  • Dart VM version: 2.1.0 (Tue Nov 13 18:22:02 2018 +0100) on "linux_x64"
@kevmoo kevmoo added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io type-documentation A request to add or improve documentation labels Apr 3, 2019
@kevmoo
Copy link
Member

kevmoo commented Apr 3, 2019

There is a cost for the extra concurrency, certainly, but we can also leverage the concurrency if there are multiple IO operations happening at the same time.

We could clarify this in the docs!

@suragch
Copy link
Contributor

suragch commented Apr 13, 2020

I'm curious about the reason for the slow times.

@jamesderlin
Copy link
Contributor Author

jamesderlin commented Apr 13, 2020

I think fundamentally asynchronous I/O is going to introduce some latency since the I/O needs to be scheduled. If you're not doing frequent I/O, this could be worse. If you're doing a lot of I/O where operations can be batched, then asynchronous I/O likely will have better overall performance.

IMO this lint seems a bit weird since I'd expect that whether asynchronous I/O is better or worse than synchronous I/O is rather dependent on what the application is doing. Is the expectation that the operations listed by the avoid_slow_async_io documentation are performed infrequently?

@jamesderlin
Copy link
Contributor Author

Also see dart-lang/linter#2103 which questions the avoid_slow_async_io lint.

@lukepighetti
Copy link

lukepighetti commented Aug 12, 2021

I would expect these async calls to be preferable in a Flutter project because in theory it shouldn't block UI painting compared to a synchronous call. If this is true, I find the lint to be misleading without providing additional context that async calls provide concurrency

related: dart-lang/linter#2103

@davidmorgan
Copy link
Contributor

The methods flagged by avoid_slow_async_io are unusual, depending on the filesystem they can be much faster synchronously. Given that for flutter the filesystem is unlikely to be a spinning disk ;) it probably does make more sense to use the sync methods. You would have to benchmark to be sure.

Even in a server handling concurrent requests, it's usually better to use the sync versions of these methods and just block everything. They're that fast. Again, you would have to benchmark to be sure.

The only case where you really don't want the sync version is if it's a network-based or other weird filesystem. Then, the sync methods might block for a long time. Again, benchmark to be sure ;)

@lukepighetti
Copy link

Did a little more digging and it looks like if you follow the linter blindly the result is 99% of the way there, but I still see some dropped frames in some testing. The performance difference is pretty minor between that and pure async calls.

But it looks like the jank concern is generally a non-issue. I will personally be making a point of using async calls though as a default, as I'd rather have guaranteed concurrency than an imperceptible performance improvement blending sync/async io calls.

https://gist.github.com/lukepighetti/921f347d94c889bc7febf59971892f10

@lrhn
Copy link
Member

lrhn commented Sep 23, 2021

If an I/O method is mostly very fast when sync, then the latency of being async is definitely felt more.

I think one of the reasons you would see much slower async latency is that, even if the result is actually available quickly, the delivery of that result is put at the end of the event queue (the top-level event queue, not the microtask queue), so all prior events need to run, with all their included microtasks, before the result can actually be seen. That's an inherent latency in async code, we have no quality-of-service priorities in our event queue.

That is, some of the latency of asynchrony is independent of that actual operation. You can only really compare sync I/O to async I/O if you compare the entire program execution, or if you make sure the I/O is the only thing your isolate is doing (which makes it moot to be async, since you get none of the benefits).
An operation is not slow just because it's late if the program actually makes other progress in the meantime.

(Could be fun to fake-implement async functions as doing the sync operation first, then returning a Future.delayed(Duration.zero, () => value) to make you wait for delivery of the value, and then see how much it differs from the current async operations).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io type-documentation A request to add or improve documentation
Projects
None yet
Development

No branches or pull requests

6 participants