[PATCH] D38715: Support: On Windows, speculatively call DeleteFileW from sys::fs::remove().

[Peter Collingbourne via llvm-commits]

Thanks, that does appear to work. I've updated the diff.

Peter

On Mon, Oct 9, 2017 at 9:53 PM, Zachary Turner <zturner at google.com> wrote:

> This makes the case of removing a file faster, but removing a directory
> slower, because you’ve essentially got 2 stats for a directory.
>
> It may be possible to knock all of this out in one call.
>
> Can you try calling CreateFile with
>
> FILE_FLAG_OPEN_REPARSE_POINT // Don’t follow symlinks
> | FILE_FLAG_BACKUP_SEMANTICS // Allow opening a directory
> | FILE_FLAG_DELETE_ON_CLOSE // Delete it
>
> Not sure if the last two options can be used together, but seems worth a
> try
>
> On Mon, Oct 9, 2017 at 9:35 PM Peter Collingbourne via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> pcc created this revision.
>> Herald added a subscriber: hiraditya.
>>
>> This saves a call to stat().
>>
>>
>> https://reviews.llvm.org/D38715
>>
>> Files:
>>   llvm/lib/Support/Windows/Path.inc
>>
>>
>> Index: llvm/lib/Support/Windows/Path.inc
>> ===================================================================
>> --- llvm/lib/Support/Windows/Path.inc
>> +++ llvm/lib/Support/Windows/Path.inc
>> @@ -259,26 +259,21 @@
>>  std::error_code remove(const Twine &path, bool IgnoreNonExisting) {
>>    SmallVector<wchar_t, 128> path_utf16;
>>
>> -  file_status ST;
>> -  if (std::error_code EC = status(path, ST)) {
>> -    if (EC != errc::no_such_file_or_directory || !IgnoreNonExisting)
>> -      return EC;
>> -    return std::error_code();
>> -  }
>> -
>>    if (std::error_code ec = widenPath(path, path_utf16))
>>      return ec;
>>
>> -  if (ST.type() == file_type::directory_file) {
>> -    if (!::RemoveDirectoryW(c_str(path_utf16))) {
>> -      std::error_code EC = mapWindowsError(::GetLastError());
>> -      if (EC != errc::no_such_file_or_directory || !IgnoreNonExisting)
>> -        return EC;
>> -    }
>> -    return std::error_code();
>> -  }
>> +  // Speculate that the path is actually a file in order to save a
>> stat() call.
>>    if (!::DeleteFileW(c_str(path_utf16))) {
>>      std::error_code EC = mapWindowsError(::GetLastError());
>> +    if (EC == errc::permission_denied) {
>> +      if (!::RemoveDirectoryW(c_str(path_utf16))) {
>> +        std::error_code EC = mapWindowsError(::GetLastError());
>> +        if (EC != errc::no_such_file_or_directory || !IgnoreNonExisting)
>> +          return EC;
>> +      }
>> +      return std::error_code();
>> +    }
>> +
>>      if (EC != errc::no_such_file_or_directory || !IgnoreNonExisting)
>>        return EC;
>>    }
>>
>>
>>


-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171010/51425da8/attachment.html>