unlink() on Windows is complicated by the fact that Windows doesn't
support removing an open file. Our unlink wrapper currently goes
through a dance where it generates a random filename, renames the
file, then attempts to unlink.
This is a lot of extra work for what is likely the special case of
attempting to unlink a file that is open or can't be simply unlinked.
In this commit, we refactor the code to use fewer system calls in
the common cases of 1) unlink just works 2) file doesn't exist.
For the file doesn't exist case (before)
- stat() to determine if path is directory
- generate random filename
- rename()
after:
- stat() to get path info
For the file not open case (before)
- stat() to determine if path is directory
- generate random filename
- rename()
- unlink()
after:
- stat()
- unlink()
For the file open case (before)
- stat() to determine if path is directory
- generate random filename
- rename()
- unlink()
after:
- stat()
- unlink()
- generate random filename
- rename()
- unlink()
There is also a scenario where the unlink fails due to the file being
marked as read-only. In this case we also introduce an extra unlink()
call. However, I reckon the common case is the file isn't marked as
read-only and skipping the random generation and rename is worth it.
I /think/ this change makes bulk file writing on Windows faster. This
is because vfs.call calls unlink() when a file is opened for
writing. When writing a few hundred files files as part of a clone or
working directory update, the extra system calls can matter.
win32.unlink() did show up in performance profiles, which is what
caused me to look at this code. But I/O performance is hard to measure
and I'm not sure if the ~30s off of ~620s for a stream clone unbundle
on the Firefox repo is indicative of real world performance. I do know
the new code uses fewer system calls and shouldn't be slower.
why continue on errors != ENOENT ?