win: various micro-optimization for opendir/readdir

Summary:
After enabling strace profiling for the dispatcher, it became very clear that
once files are populated on disk, most of the notification in edenfs come from
listing directories. While looking at the code to find any improvements, I
picked up a couple of low hanging fruits and cleanup that should help (not
measured).

Reviewed By: chadaustin

Differential Revision: D23121434

fbshipit-source-id: 8a8b369f3be6ffb6097e6e12ab0f07af0a6d56b7
This commit is contained in:
Xavier Deguillard 2020-08-14 17:33:49 -07:00 committed by Facebook GitHub Bot
parent 76ea3054de
commit 17776fbbc1
6 changed files with 44 additions and 58 deletions

View File

@ -1885,13 +1885,14 @@ folly::Future<std::vector<FileMetadata>> TreeInode::readdir() {
->getObjectStore()
->getBlobSize(
hash.value(), ObjectFetchContext::getNullContext())
.thenValue([winName = std::move(winName)](uint64_t size) {
return FileMetadata(winName, false, size);
}));
.thenValue(
[winName = std::move(winName)](uint64_t size) mutable {
return FileMetadata(std::move(winName), false, size);
}));
continue;
}
}
futures.emplace_back(FileMetadata(winName, isDir, 0));
futures.emplace_back(FileMetadata(std::move(winName), isDir, 0));
}
// We can release the content lock here.

View File

@ -88,8 +88,8 @@ HRESULT EdenDispatcher::startEnumeration(
})
.get();
auto [iterator, inserted] = enumSessions_.wlock()->emplace(
enumerationId, make_unique<Enumerator>(guid, std::move(list)));
auto [iterator, inserted] =
enumSessions_.wlock()->emplace(guid, std::move(list));
DCHECK(inserted);
return S_OK;
} catch (const std::exception& ex) {
@ -99,10 +99,10 @@ HRESULT EdenDispatcher::startEnumeration(
HRESULT EdenDispatcher::endEnumeration(const GUID& enumerationId) noexcept {
try {
FB_LOGF(
mount_->getStraceLogger(), DBG7, "releasedir({})", Guid(enumerationId));
auto guid = Guid(enumerationId);
FB_LOGF(mount_->getStraceLogger(), DBG7, "releasedir({})", guid);
auto erasedCount = enumSessions_.wlock()->erase(enumerationId);
auto erasedCount = enumSessions_.wlock()->erase(guid);
DCHECK(erasedCount == 1);
return S_OK;
} catch (const std::exception& ex) {
@ -126,9 +126,6 @@ HRESULT EdenDispatcher::getEnumerationData(
? "<nullptr>"
: wideToMultibyteString<std::string>(searchExpression));
//
// Error if we don't have the session.
//
auto lockedSessions = enumSessions_.rlock();
auto sessionIterator = lockedSessions->find(guid);
if (sessionIterator == lockedSessions->end()) {
@ -139,26 +136,30 @@ HRESULT EdenDispatcher::getEnumerationData(
auto shouldRestart =
bool(callbackData.Flags & PRJ_CB_DATA_FLAG_ENUM_RESTART_SCAN);
auto& session = sessionIterator->second;
if (session->isSearchExpressionEmpty() || shouldRestart) {
// We won't ever get concurrent callbacks for a given enumeration, it is
// therefore safe to modify the session here even though we do not hold an
// exclusive lock to it.
auto& session = const_cast<Enumerator&>(sessionIterator->second);
if (session.isSearchExpressionEmpty() || shouldRestart) {
if (searchExpression != nullptr) {
session->saveExpression(searchExpression);
session.saveExpression(searchExpression);
} else {
session->saveExpression(L"*");
session.saveExpression(L"*");
}
}
if (shouldRestart) {
session->restart();
session.restart();
}
//
// Traverse the list enumeration list and fill the remaining entry. Start
// from where the last call left off.
//
for (const FileMetadata* entry; (entry = session->current());
session->advance()) {
for (const FileMetadata* entry; (entry = session.current());
session.advance()) {
auto fileInfo = PRJ_FILE_BASIC_INFO();
fileInfo.IsDirectory = entry->isDirectory;

View File

@ -10,13 +10,13 @@
#include "folly/portability/Windows.h"
#include <ProjectedFSLib.h>
#include <folly/Synchronized.h>
#include <folly/container/F14Map.h>
#include <cstdint>
#include <cstring>
#include <map>
#include <string>
#include "eden/fs/win/mount/Enumerator.h"
#include "eden/fs/win/utils/Guid.h"
#include "folly/Synchronized.h"
constexpr uint32_t kDispatcherCode = 0x1155aaff;
@ -72,12 +72,8 @@ class EdenDispatcher {
// The EdenMount that owns this EdenDispatcher.
EdenMount* const mount_;
//
// This will have a list of currently active enumeration sessions
//
// TODO: add the hash function and convert it to unordered_map (may be).
folly::Synchronized<std::map<GUID, std::unique_ptr<Enumerator>, CompareGuid>>
enumSessions_;
// Set of currently active directory enumerations.
folly::Synchronized<folly::F14FastMap<Guid, Enumerator>> enumSessions_;
const std::string dotEdenConfig_;

View File

@ -13,9 +13,7 @@
namespace facebook {
namespace eden {
Enumerator::Enumerator(
const GUID& enumerationId,
std::vector<FileMetadata> entryList)
Enumerator::Enumerator(std::vector<FileMetadata>&& entryList)
: metadataList_(std::move(entryList)) {
std::sort(
metadataList_.begin(),

View File

@ -31,29 +31,10 @@ struct FileMetadata {
//
size_t size{0};
//
// This is the hash we use to fetch Tree and Blob. When working
// with mercurial it is hg proxy hash.
//
Hash hash{};
FileMetadata(std::wstring name, bool isDir, size_t size)
FileMetadata(std::wstring&& name, bool isDir, size_t size)
: name(std::move(name)), isDirectory(isDir), size(size) {}
FileMetadata(std::wstring name, bool isDir, size_t size, const Hash& hash)
: name(std::move(name)), isDirectory(isDir), size(size), hash{hash} {}
FileMetadata() = default;
bool operator==(const FileMetadata& other) const {
return (
(name == other.name) && (isDirectory == other.isDirectory) &&
(size == other.size) && (hash == other.hash));
}
bool operator!=(const FileMetadata& other) const {
return !(*this == other);
}
FileMetadata() = delete;
};
class Enumerator {
@ -61,7 +42,12 @@ class Enumerator {
Enumerator(const Enumerator&) = delete;
Enumerator& operator=(const Enumerator&) = delete;
Enumerator(const GUID& enumerationId, std::vector<FileMetadata> entryList);
Enumerator(std::vector<FileMetadata>&& entryList);
Enumerator(Enumerator&& other)
: metadataList_(std::move(other.metadataList_)),
searchExpression_(std::move(other.searchExpression_)),
listIndex_(std::move(other.listIndex_)) {}
explicit Enumerator() = delete;

View File

@ -78,15 +78,19 @@ class Guid {
GUID guid_;
};
struct CompareGuid {
bool operator()(const GUID& left, const GUID& right) const noexcept {
return memcmp(&left, &right, sizeof(right)) < 0;
}
};
} // namespace eden
} // namespace facebook
namespace std {
template <>
struct hash<facebook::eden::Guid> {
size_t operator()(const facebook::eden::Guid& guid) const {
return folly::hash::SpookyHashV2::Hash64(
reinterpret_cast<const void*>(&guid), sizeof(guid), 0);
}
};
} // namespace std
namespace fmt {
template <>
struct formatter<facebook::eden::Guid> : formatter<string_view> {