Review comments

This commit is contained in:
Ryan Houdek committed 2023-04-07 17:01:53 -07:00
1 parent 96e9b5cf51
commit e98a46aa5f
11 files changed
+61 -56

No files matched your search

+1 -1
View File
@@ -168,7 +168,7 @@ namespace FEXCore::Context {
void SetAOTIRLoader(std::function<int(const fextl::string&)> CacheReader) override {
IRCaptureCache.SetAOTIRLoader(CacheReader);
}
void SetAOTIRWriter(std::function<fextl::unique_ptr<AOTIRWriterFD>(const fextl::string&)> CacheWriter) override {
void SetAOTIRWriter(std::function<fextl::unique_ptr<AOTIRWriter>(const fextl::string&)> CacheWriter) override {
IRCaptureCache.SetAOTIRWriter(CacheWriter);
}
void SetAOTIRRenamer(std::function<void(const fextl::string&)> CacheRenamer) override {
+1
View File
@@ -1279,6 +1279,7 @@ void GdbServer::StartThread() {
}
void GdbServer::OpenListenSocket() {
// getaddrinfo allocates memory that can't be removed.
FEXCore::Allocator::YesIKnowImNotSupposedToUseTheGlibcAllocator glibc;
struct addrinfo hints, *res;
+2 -1
View File
@@ -199,6 +199,7 @@ namespace FEXCore::IR {
for (;;) {
// This code is tricky to refactor so it doesn't allocate memory through glibc.
// The moved std::function object deallocates memory at the end of scope.
FEXCore::Allocator::YesIKnowImNotSupposedToUseTheGlibcAllocator glibc;
AOTIRCaptureCacheWriteoutLock.lock();
@@ -329,7 +330,7 @@ namespace FEXCore::IR {
auto RADataCopyDeleter = RADataCopy.get_deleter();
auto IRListCopy = IRList->CreateCopy();
// This code is tricky to refactor so it doesn't allocate memory through glibc.
// The lambda is converted to std::function. This is tricky to refactor so it doesn't allocate memory through glibc.
FEXCore::Allocator::YesIKnowImNotSupposedToUseTheGlibcAllocator glibc;
AOTIRCaptureCacheWriteoutQueue_Append([this, LocalRIP, LocalStartAddr, Length, hash, IRListCopy, RADataCopy=RADataCopy.release(), RADataCopyDeleter, FileId]() {
+3 -3
View File
@@ -69,7 +69,7 @@ namespace FEXCore::IR {
};
struct AOTIRCaptureCacheEntry {
fextl::unique_ptr<FEXCore::Context::AOTIRWriterFD> Stream;
fextl::unique_ptr<FEXCore::Context::AOTIRWriter> Stream;
fextl::map<uint64_t, uint64_t> Index;
void AppendAOTIRCaptureCache(uint64_t GuestRIP, uint64_t Start, uint64_t Length, uint64_t Hash, FEXCore::IR::IRListView *IRList, FEXCore::IR::RegisterAllocationData *RAData);
@@ -125,7 +125,7 @@ namespace FEXCore::IR {
AOTIRLoader = CacheReader;
}
void SetAOTIRWriter(std::function<fextl::unique_ptr<FEXCore::Context::AOTIRWriterFD>(const fextl::string&)> CacheWriter) {
void SetAOTIRWriter(std::function<fextl::unique_ptr<FEXCore::Context::AOTIRWriter>(const fextl::string&)> CacheWriter) {
AOTIRWriter = CacheWriter;
}
@@ -145,7 +145,7 @@ namespace FEXCore::IR {
FEXCore::IR::AOTCacheType AOTIRCache;
std::function<int(const fextl::string&)> AOTIRLoader;
std::function<fextl::unique_ptr<FEXCore::Context::AOTIRWriterFD>(const fextl::string&)> AOTIRWriter;
std::function<fextl::unique_ptr<FEXCore::Context::AOTIRWriter>(const fextl::string&)> AOTIRWriter;
std::function<void(const fextl::string&)> AOTIRRenamer;
fextl::unordered_map<fextl::string, FEXCore::IR::AOTIRCaptureCacheEntry> AOTIRCaptureCacheMap;
};
+1 -1
View File
@@ -10,7 +10,7 @@
namespace FEXCore::FileLoading {
template<typename T>
bool LoadFileImpl(T &Data, const fextl::string &Filepath, size_t FixedSize) {
static bool LoadFileImpl(T &Data, const fextl::string &Filepath, size_t FixedSize) {
int FD = open(Filepath.c_str(), O_RDONLY);
if (FD == -1) {
+6 -5
View File
@@ -70,14 +70,15 @@ namespace FEXCore::Context {
std::unique_lock<std::shared_mutex> lock;
};
class AOTIRWriterFD {
/**
* @brief IR Serialization handler class.
*/
class AOTIRWriter {
public:
virtual ~AOTIRWriterFD() {
}
virtual ~AOTIRWriter() = default;
virtual void Write(const void* Data, size_t Size) = 0;
virtual size_t Offset() = 0;
virtual void Close() = 0;
private:
};
struct VDSOSigReturn {
@@ -265,7 +266,7 @@ namespace FEXCore::Context {
FEX_DEFAULT_VISIBILITY virtual void UnloadAOTIRCacheEntry(FEXCore::IR::AOTIRCacheEntry *Entry) = 0;
FEX_DEFAULT_VISIBILITY virtual void SetAOTIRLoader(std::function<int(const fextl::string&)> CacheReader) = 0;
FEX_DEFAULT_VISIBILITY virtual void SetAOTIRWriter(std::function<fextl::unique_ptr<AOTIRWriterFD>(const fextl::string&)> CacheWriter) = 0;
FEX_DEFAULT_VISIBILITY virtual void SetAOTIRWriter(std::function<fextl::unique_ptr<AOTIRWriter>(const fextl::string&)> CacheWriter) = 0;
FEX_DEFAULT_VISIBILITY virtual void SetAOTIRRenamer(std::function<void(const fextl::string&)> CacheRenamer) = 0;
FEX_DEFAULT_VISIBILITY virtual void FinalizeAOTIRCache() = 0;
+1 -1
View File
@@ -171,7 +171,7 @@ public:
}
}
void Serialize(FEXCore::Context::AOTIRWriterFD& stream) const {
void Serialize(FEXCore::Context::AOTIRWriter& stream) const {
void *nul = nullptr;
//void *IRDataInternal;
stream.Write((const char*)&nul, sizeof(nul));
@@ -58,7 +58,7 @@ class FEX_PACKED RegisterAllocationData {
UniquePtr CreateCopy() const;
void Serialize(FEXCore::Context::AOTIRWriterFD& stream) const {
void Serialize(FEXCore::Context::AOTIRWriter& stream) const {
stream.Write((const char*)&SpillSlotCount, sizeof(SpillSlotCount));
stream.Write((const char*)&MapCount, sizeof(MapCount));
// RAData (inline)
+3 -6
View File
@@ -203,13 +203,10 @@ namespace FHU::Filesystem {
/**
* @brief Renames a file and overwrites if it already exists.
*
* @param From
* @param To
*
* @return True if the rename occured, False otherwise.
* @return No error on rename.
*/
inline bool RenameFile(const fextl::string &From, const fextl::string &To) {
return rename(From.c_str(), To.c_str()) == 0;
[[nodiscard]] inline std::error_code RenameFile(const fextl::string &From, const fextl::string &To) {
return rename(From.c_str(), To.c_str()) == 0 ? std::error_code{} : std::make_error_code(std::errc::io_error);
}
inline fextl::string LexicallyNormal(const fextl::string &Path) {
+1
View File
@@ -147,6 +147,7 @@ void AOTGenSection(FEXCore::Context::Context *CTX, ELFCodeLoader::LoadedSection
// All entryproints processed, cleanup this thread
CTX->DestroyThread(Thread);
// This thread is now getting abandoned. Disable glibc allocator checking so glibc can safely cleanup its internal allocations.
FEXCore::Allocator::YesIKnowImNotSupposedToUseTheGlibcAllocator::HardDisable();
});
+41 -37
View File
@@ -121,6 +121,42 @@ namespace FEXServerLogging {
}
}
namespace AOTIR {
class AOTIRWriterFD final : public FEXCore::Context::AOTIRWriter {
public:
AOTIRWriterFD(const fextl::string &Path) {
// Create and truncate if exists.
constexpr int USER_PERMS = S_IRWXU | S_IRWXG | S_IRWXO;
FD = open(Path.c_str(), O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, USER_PERMS);
}
operator bool() const {
return FD != -1;
}
void Write(const void* Data, size_t Size) override {
write(FD, Data, Size);
}
size_t Offset() override {
return lseek(FD, 0, SEEK_CUR);
}
void Close() override {
if (FD != -1) {
close(FD);
FD = -1;
}
}
virtual ~AOTIRWriterFD() {
Close();
}
private:
int FD{-1};
};
}
void InterpreterHandler(fextl::string *Filename, fextl::string const &RootFS, fextl::vector<fextl::string> *args) {
// Open the Filename to determine if it is a shebang file.
int FD = open(Filename->c_str(), O_RDONLY | O_CLOEXEC);
@@ -457,43 +493,9 @@ int main(int argc, char **argv, char **const envp) {
return open(filepath.c_str(), O_RDONLY);
});
class AOTIRWriterFD final : public FEXCore::Context::AOTIRWriterFD {
public:
AOTIRWriterFD(const fextl::string &Path) {
// Create and truncate if exists.
constexpr int USER_PERMS = S_IRWXU | S_IRWXG | S_IRWXO;
FD = open(Path.c_str(), O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, USER_PERMS);
}
operator bool() const {
return FD != -1;
}
void Write(const void* Data, size_t Size) override {
write(FD, Data, Size);
}
size_t Offset() override {
return lseek(FD, 0, SEEK_CUR);
}
void Close() override {
if (FD != -1) {
close(FD);
FD = -1;
}
}
virtual ~AOTIRWriterFD() {
Close();
}
private:
int FD{-1};
};
CTX->SetAOTIRWriter([](const fextl::string& fileid) -> fextl::unique_ptr<AOTIRWriterFD> {
CTX->SetAOTIRWriter([](const fextl::string& fileid) -> fextl::unique_ptr<AOTIR::AOTIRWriterFD> {
const auto filepath = fextl::fmt::format("{}/aotir/{}.aotir.tmp", FEXCore::Config::GetDataDirectory(), fileid);
auto AOTWrite = fextl::make_unique<AOTIRWriterFD>(filepath);
auto AOTWrite = fextl::make_unique<AOTIR::AOTIRWriterFD>(filepath);
if (*AOTWrite) {
LogMan::Msg::IFmt("AOTIR: Storing {}", fileid);
} else {
@@ -507,7 +509,9 @@ int main(int argc, char **argv, char **const envp) {
const auto NewFilepath = fextl::fmt::format("{}/aotir/{}.aotir", FEXCore::Config::GetDataDirectory(), fileid);
// Rename the temporary file to atomically update the file
FHU::Filesystem::RenameFile(TmpFilepath, NewFilepath);
if (!FHU::Filesystem::RenameFile(TmpFilepath, NewFilepath)) {
LogMan::Msg::IFmt("Couldn't rename aotir");
}
});
}