Windows async file I/O

[Evan Stade]

Hi net/ owners,

I noticed net::FileStream uses async file i/o on Windows.* Is this a holdover from some earlier design before task management/threading was simplified in Chromium, or is there still a reason for it?

Since all the file operations occur on a background task runner, it seems like synchronous I/O should be ok. Perhaps I don't understand the subtleties of async and sync I/O on Windows.

Here is some potentially useful historical context.

Thanks in advance for your advice!

* Ignore this comment; it's actually always async. Also it doesn't seem like the async flag does anything on POSIX.

-- Evan Stade

[Matt Menke]

It does date back to when Chrome had a single global file thread.  The disk cache is the most significant consumer of it, and I wouldn't be surprised if its index lives on the same single thread where all disk cache operations take place, making async IO still important for disk cache performance, but am not at all positive of that.  If that's the case, I suspect reworking the legacy blockfile disk cache backend would not be an option...  It's not fun code.

--
You received this message because you are subscribed to the Google Groups "net-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to net-dev+u...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/net-dev/CAO4XGS-s2i3zziaQmrUkBUMs%2Bhr_hSB%2BFFK1wxBRH2dzQHhVfQ%40mail.gmail.com.

[Maksim Orlovich]

blockfile does not actually use net::FileStream... but has its own implementation of async file I/O (which uses overlapped I/O on windows).

The index is also memory mapped. 

Most usages actually seem to be outside of net; inside I can only spot UploadFileElementReader.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/net-dev/CAEK7mvrTvd1PKi1aMgB%2BKVLPiu8PE-WqSCj%3DL-PHdax1MUebXQ%40mail.gmail.com.

[Evan Stade]

Thanks Matt and Maksim. I now feel more confident that FileStream is largely an anachronism at this point :)

For more context: it was while I was looking at one of these out-of-net consumers of FileStream (IndexedDB blob reading) that I became curious about FileStream. It seems that our code can become a good deal simpler if we just stop using FileStream, but I was worried that FileStream was somehow useful in a way that wasn't immediately apparent to me. It sounds like the async i/o (on Windows) is only useful in the context of having a shared file thread that we don't want to block (a somewhat archaic concern, in most cases, probably), and in the case of a dedicated ThreadPool TaskRunner it's not buying us anything. In fact, FileStream has the annoying property of needing to be run on a TYPE_IO thread.

Looking at the other consumers of FileStream, in or out of net::, was not in my immediate scope, but probably something to add to the storage team backlog. Skimming through them, most use a dedicated taskrunner, although a few use a taskrunner shared by a few objects, which would mean async IO might have some effect for them. But those probably could be updated to dedicated TaskRunners, which would also be an improvement for non-win if there is indeed contention for that thread.

> Most usages actually seem to be outside of net; inside I can only spot UploadFileElementReader.

This plus the very minor/easily breakable dependencies on the rest of net:: makes me think that it might be in the wrong directory (if it even needs to exist long term any more).

-- Evan Stade

[Matt Menke]

Doing some quick archaeology, it looks like it was originally added for the disk_cache:  https://codereview.chromium.org/8843.  It's not the first helper intended for net/ use that has attracted heavy external use.  Anyhow, I'm in agreement that things can be migrated to blocking I/O on their own thread, as long as we aren't too concerned about extra threads.

That having been said, FileStream does all the thread hopping for the caller, so it doesn't need to manage all of that itself.  Unless base/ already provides the same capability, it may be useful to keep FileStream, or something like it, though without async file I/O.

[Evan Stade]

On Thu, Oct 3, 2024 at 12:56 PM Matt Menke <mme...@chromium.org> wrote:

Doing some quick archaeology, it looks like it was originally added for the disk_cache:  https://codereview.chromium.org/8843.  It's not the first helper intended for net/ use that has attracted heavy external use.  Anyhow, I'm in agreement that things can be migrated to blocking I/O on their own thread, as long as we aren't too concerned about extra threads.

That having been said, FileStream does all the thread hopping for the caller, so it doesn't need to manage all of that itself.  Unless base/ already provides the same capability, it may be useful to keep FileStream, or something like it, though without async file I/O.

Agreed. The thread management is helpful in some cases. That said, thread helpers are a lot more ergonomic now than they used to be, e.g. `PostTaskAndReply()`, so we may want to evaluate the utility on a case by case basis.