Merge pull request #18241 from hrydgard/ini-rewrite

Optimize IniFile for faster save/load of config
This commit is contained in:
Henrik Rydgård 2023-09-30 11:26:04 +02:00 committed by GitHub
commit 2a4d21e53b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 183 additions and 151 deletions

View File

@ -29,7 +29,16 @@
#include "Common/StringUtils.h"
static bool ParseLineKey(const std::string &line, size_t &pos, std::string *keyOut) {
bool StringViewEqualCaseInsensitive(const std::string_view lhs, const std::string_view rhs) {
if (lhs.size() != rhs.size()) {
return false;
}
return ::strncasecmp(lhs.data(), rhs.data(), rhs.size()) == 0;
}
// This unescapes # signs.
// NOTE: These parse functions can make better use of the string_view - the pos argument should not be needed, for example.
static bool ParseLineKey(std::string_view line, size_t &pos, std::string *keyOut) {
std::string key = "";
while (pos < line.size()) {
@ -44,7 +53,8 @@ static bool ParseLineKey(const std::string &line, size_t &pos, std::string *keyO
}
// Escaped.
key += line.substr(pos, next - pos - 1) + "#";
key += line.substr(pos, next - pos - 1);
key.push_back('#');
pos = next + 1;
} else if (line[next] == '=') {
// Hurray, done.
@ -60,11 +70,11 @@ static bool ParseLineKey(const std::string &line, size_t &pos, std::string *keyO
return true;
}
static bool ParseLineValue(const std::string &line, size_t &pos, std::string *valueOut) {
static bool ParseLineValue(std::string_view line, size_t &pos, std::string *valueOut) {
std::string value = "";
std::string strippedLine = StripSpaces(line.substr(pos));
if (strippedLine[0] == '"' && strippedLine[strippedLine.size()-1] == '"') {
std::string_view strippedLine = StripSpaces(line.substr(pos));
if (strippedLine.size() >= 2 && strippedLine[0] == '"' && strippedLine[strippedLine.size() - 1] == '"') {
// Don't remove comment if is surrounded by " "
value += line.substr(pos);
pos = line.npos; // Won't enter the while below
@ -84,7 +94,8 @@ static bool ParseLineValue(const std::string &line, size_t &pos, std::string *va
break;
} else {
// Escaped.
value += line.substr(pos, next - pos - 1) + "#";
value += line.substr(pos, next - pos - 1);
value.push_back('#');
pos = next + 1;
}
}
@ -96,7 +107,7 @@ static bool ParseLineValue(const std::string &line, size_t &pos, std::string *va
return true;
}
static bool ParseLineComment(const std::string& line, size_t &pos, std::string *commentOut) {
static bool ParseLineComment(std::string_view line, size_t &pos, std::string *commentOut) {
// Don't bother with anything if we don't need the comment data.
if (commentOut) {
// Include any whitespace/formatting in the comment.
@ -117,8 +128,7 @@ static bool ParseLineComment(const std::string& line, size_t &pos, std::string *
return true;
}
// Ugh, this is ugly.
static bool ParseLine(const std::string& line, std::string* keyOut, std::string* valueOut, std::string* commentOut)
static bool ParseLine(std::string_view line, std::string* keyOut, std::string* valueOut, std::string* commentOut)
{
// Rules:
// 1. A line starting with ; is commented out.
@ -142,7 +152,7 @@ static bool ParseLine(const std::string& line, std::string* keyOut, std::string*
return true;
}
static std::string EscapeComments(const std::string &value) {
static std::string EscapeHash(std::string_view value) {
std::string result = "";
for (size_t pos = 0; pos < value.size(); ) {
@ -151,7 +161,8 @@ static std::string EscapeComments(const std::string &value) {
result += value.substr(pos);
pos = value.npos;
} else {
result += value.substr(pos, next - pos) + "\\#";
result += value.substr(pos, next - pos);
result += "\\#";
pos = next + 1;
}
}
@ -159,31 +170,44 @@ static std::string EscapeComments(const std::string &value) {
return result;
}
void Section::Clear() {
lines.clear();
void ParsedIniLine::ParseFrom(std::string_view line) {
line = StripSpaces(line);
if (line.empty()) {
key.clear();
value.clear();
comment.clear();
} else if (line[0] == '#') {
key.clear();
value.clear();
comment = line;
} else {
ParseLine(line, &key, &value, &comment);
}
}
std::string* Section::GetLine(const char* key, std::string* valueOut, std::string* commentOut)
{
for (std::vector<std::string>::iterator iter = lines.begin(); iter != lines.end(); ++iter)
{
std::string& line = *iter;
std::string lineKey;
ParseLine(line, &lineKey, valueOut, commentOut);
if (!strcasecmp(lineKey.c_str(), key))
void ParsedIniLine::Reconstruct(std::string *output) const {
if (!key.empty()) {
*output = EscapeHash(key) + " = " + EscapeHash(value) + comment;
} else {
*output = comment;
}
}
void Section::Clear() {
lines_.clear();
}
ParsedIniLine *Section::GetLine(const char *key) {
for (auto &line : lines_) {
if (StringViewEqualCaseInsensitive(line.Key(), key))
return &line;
}
return nullptr;
}
const std::string* Section::GetLine(const char* key, std::string* valueOut, std::string* commentOut) const
{
for (std::vector<std::string>::const_iterator iter = lines.begin(); iter != lines.end(); ++iter)
{
const std::string& line = *iter;
std::string lineKey;
ParseLine(line, &lineKey, valueOut, commentOut);
if (!strcasecmp(lineKey.c_str(), key))
const ParsedIniLine *Section::GetLine(const char* key) const {
for (auto &line : lines_) {
if (StringViewEqualCaseInsensitive(line.Key(), key))
return &line;
}
return nullptr;
@ -209,19 +233,13 @@ void Section::Set(const char* key, int newValue) {
Set(key, StringFromInt(newValue).c_str());
}
void Section::Set(const char* key, const char* newValue)
{
std::string value, commented;
std::string* line = GetLine(key, &value, &commented);
if (line)
{
// Change the value - keep the key and comment
*line = StripSpaces(key) + " = " + EscapeComments(newValue) + commented;
}
else
{
void Section::Set(const char* key, const char* newValue) {
ParsedIniLine *line = GetLine(key);
if (line) {
line->SetValue(newValue);
} else {
// The key did not already exist in this section - let's add it.
lines.emplace_back(std::string(key) + " = " + EscapeComments(newValue));
lines_.emplace_back(ParsedIniLine(key, newValue));
}
}
@ -233,16 +251,15 @@ void Section::Set(const char* key, const std::string& newValue, const std::strin
Delete(key);
}
bool Section::Get(const char* key, std::string* value, const char* defaultValue) const
{
const std::string* line = GetLine(key, value, 0);
if (!line)
{
if (defaultValue)
{
bool Section::Get(const char* key, std::string* value, const char* defaultValue) const {
const ParsedIniLine *line = GetLine(key);
if (!line) {
if (defaultValue) {
*value = defaultValue;
}
return false;
} else {
*value = line->Value();
}
return true;
}
@ -287,7 +304,7 @@ void Section::Set(const char* key, const std::vector<std::string>& newValues)
}
void Section::AddComment(const std::string &comment) {
lines.emplace_back("# " + comment);
lines_.emplace_back(ParsedIniLine::CommentOnly("# " + comment));
}
bool Section::Get(const char* key, std::vector<std::string>& values) const
@ -378,39 +395,29 @@ bool Section::Get(const char* key, double* value, double defaultValue) const
return false;
}
bool Section::Exists(const char *key) const
{
for (std::vector<std::string>::const_iterator iter = lines.begin(); iter != lines.end(); ++iter)
{
std::string lineKey;
ParseLine(*iter, &lineKey, NULL, NULL);
if (!strcasecmp(lineKey.c_str(), key))
bool Section::Exists(const char *key) const {
for (auto &line : lines_) {
if (StringViewEqualCaseInsensitive(key, line.Key()))
return true;
}
return false;
}
std::map<std::string, std::string> Section::ToMap() const
{
std::map<std::string, std::string> Section::ToMap() const {
std::map<std::string, std::string> outMap;
for (std::vector<std::string>::const_iterator iter = lines.begin(); iter != lines.end(); ++iter)
{
std::string lineKey, lineValue;
if (ParseLine(*iter, &lineKey, &lineValue, NULL)) {
outMap[lineKey] = lineValue;
for (auto &line : lines_) {
if (!line.Key().empty()) {
outMap[std::string(line.Key())] = line.Value();
}
}
return outMap;
}
bool Section::Delete(const char *key)
{
std::string* line = GetLine(key, 0, 0);
for (std::vector<std::string>::iterator liter = lines.begin(); liter != lines.end(); ++liter)
{
if (line == &*liter)
{
lines.erase(liter);
bool Section::Delete(const char *key) {
ParsedIniLine *line = GetLine(key);
for (auto liter = lines_.begin(); liter != lines_.end(); ++liter) {
if (line == &*liter) {
lines_.erase(liter);
return true;
}
}
@ -463,27 +470,14 @@ bool IniFile::Exists(const char* sectionName, const char* key) const {
return section->Exists(key);
}
void IniFile::SetLines(const char* sectionName, const std::vector<std::string> &lines)
{
Section* section = GetOrCreateSection(sectionName);
section->lines.clear();
for (std::vector<std::string>::const_iterator iter = lines.begin(); iter != lines.end(); ++iter)
{
section->lines.push_back(*iter);
}
}
bool IniFile::DeleteKey(const char* sectionName, const char* key)
{
bool IniFile::DeleteKey(const char* sectionName, const char* key) {
Section* section = GetSection(sectionName);
if (!section)
return false;
std::string* line = section->GetLine(key, 0, 0);
for (std::vector<std::string>::iterator liter = section->lines.begin(); liter != section->lines.end(); ++liter)
{
if (line == &(*liter))
{
section->lines.erase(liter);
ParsedIniLine *line = section->GetLine(key);
for (auto liter = section->lines_.begin(); liter != section->lines_.end(); ++liter) {
if (line == &(*liter)) {
section->lines_.erase(liter);
return true;
}
}
@ -491,55 +485,18 @@ bool IniFile::DeleteKey(const char* sectionName, const char* key)
}
// Return a list of all keys in a section
bool IniFile::GetKeys(const char* sectionName, std::vector<std::string>& keys) const
{
const Section* section = GetSection(sectionName);
bool IniFile::GetKeys(const char* sectionName, std::vector<std::string>& keys) const {
const Section *section = GetSection(sectionName);
if (!section)
return false;
keys.clear();
for (std::vector<std::string>::const_iterator liter = section->lines.begin(); liter != section->lines.end(); ++liter)
{
std::string key;
ParseLine(*liter, &key, 0, 0);
if (!key.empty())
keys.push_back(key);
for (auto liter = section->lines_.begin(); liter != section->lines_.end(); ++liter) {
if (!liter->Key().empty())
keys.push_back(std::string(liter->Key()));
}
return true;
}
// Return a list of all lines in a section
bool IniFile::GetLines(const char* sectionName, std::vector<std::string>& lines, const bool remove_comments) const
{
const Section* section = GetSection(sectionName);
if (!section)
return false;
lines.clear();
for (std::vector<std::string>::const_iterator iter = section->lines.begin(); iter != section->lines.end(); ++iter)
{
std::string line = StripSpaces(*iter);
if (remove_comments)
{
int commentPos = (int)line.find('#');
if (commentPos == 0)
{
continue;
}
if (commentPos != (int)std::string::npos)
{
line = StripSpaces(line.substr(0, commentPos));
}
}
lines.push_back(line);
}
return true;
}
void IniFile::SortSections()
{
std::sort(sections.begin(), sections.end());
@ -613,7 +570,9 @@ bool IniFile::Load(std::istream &in) {
if (sections.empty()) {
sections.push_back(std::unique_ptr<Section>(new Section("")));
}
sections.back()->lines.push_back(line);
ParsedIniLine parsedLine;
parsedLine.ParseFrom(line);
sections.back()->lines_.push_back(parsedLine);
}
}
}
@ -634,12 +593,13 @@ bool IniFile::Save(const Path &filename)
fprintf(file, "\xEF\xBB\xBF");
for (const auto &section : sections) {
if (!section->name().empty() && (!section->lines.empty() || !section->comment.empty())) {
if (!section->name().empty() && (!section->lines_.empty() || !section->comment.empty())) {
fprintf(file, "[%s]%s\n", section->name().c_str(), section->comment.c_str());
}
for (const std::string &s : section->lines) {
fprintf(file, "%s\n", s.c_str());
for (const auto &line : section->lines_) {
std::string buffer;
line.Reconstruct(&buffer);
fprintf(file, "%s\n", buffer.c_str());
}
}

View File

@ -8,6 +8,7 @@
#include <memory>
#include <map>
#include <string>
#include <string_view>
#include <vector>
#include <cstdint>
@ -15,6 +16,39 @@
class VFSInterface;
class ParsedIniLine {
public:
ParsedIniLine() {}
ParsedIniLine(std::string_view key, std::string_view value) {
this->key = key;
this->value = value;
}
ParsedIniLine(std::string_view key, std::string_view value, std::string_view comment) {
this->key = key;
this->value = value;
this->comment = comment;
}
static ParsedIniLine CommentOnly(std::string_view comment) {
return ParsedIniLine(std::string_view(), std::string_view(), comment);
}
// Comments only come from "ParseFrom".
void ParseFrom(std::string_view line);
void Reconstruct(std::string *output) const;
// Having these as views allows a more efficient internal representation, like one joint string.
std::string_view Key() const { return key; }
std::string_view Value() const { return value; }
std::string_view Comment() const { return comment; }
void SetValue(std::string_view newValue) { value = newValue; }
private:
std::string key;
std::string value;
std::string comment;
};
class Section {
friend class IniFile;
@ -29,8 +63,8 @@ public:
std::map<std::string, std::string> ToMap() const;
std::string *GetLine(const char* key, std::string* valueOut, std::string* commentOut);
const std::string *GetLine(const char* key, std::string* valueOut, std::string* commentOut) const;
ParsedIniLine *GetLine(const char *key);
const ParsedIniLine *GetLine(const char *key) const;
void Set(const char* key, const char* newValue);
void Set(const char* key, const std::string& newValue, const std::string& defaultValue);
@ -80,7 +114,7 @@ public:
}
protected:
std::vector<std::string> lines;
std::vector<ParsedIniLine> lines_;
std::string name_;
std::string comment;
};
@ -88,12 +122,10 @@ protected:
class IniFile {
public:
bool Load(const Path &path);
bool Load(const std::string &filename) { return Load(Path(filename)); }
bool Load(std::istream &istream);
bool LoadFromVFS(VFSInterface &vfs, const std::string &filename);
bool Save(const Path &path);
bool Save(const std::string &filename) { return Save(Path(filename)); }
// Returns true if key exists in section
bool Exists(const char* sectionName, const char* key) const;
@ -138,9 +170,6 @@ public:
bool GetKeys(const char* sectionName, std::vector<std::string>& keys) const;
void SetLines(const char* sectionName, const std::vector<std::string> &lines);
bool GetLines(const char* sectionName, std::vector<std::string>& lines, const bool remove_comments = true) const;
bool DeleteKey(const char* sectionName, const char* key);
bool DeleteSection(const char* sectionName);

View File

@ -239,18 +239,15 @@ std::string StringFromFormat(const char* format, ...)
return temp;
}
std::string StringFromInt(int value)
{
std::string StringFromInt(int value) {
char temp[16];
snprintf(temp, sizeof(temp), "%d", value);
return temp;
}
// Turns " hej " into "hej". Also handles tabs.
std::string StripSpaces(const std::string &str)
{
std::string StripSpaces(const std::string &str) {
const size_t s = str.find_first_not_of(" \t\r\n");
if (str.npos != s)
return str.substr(s, str.find_last_not_of(" \t\r\n") - s + 1);
else
@ -268,6 +265,25 @@ std::string StripQuotes(const std::string& s)
return s;
}
// Turns " hej " into "hej". Also handles tabs.
std::string_view StripSpaces(std::string_view str) {
const size_t s = str.find_first_not_of(" \t\r\n");
if (str.npos != s)
return str.substr(s, str.find_last_not_of(" \t\r\n") - s + 1);
else
return "";
}
// "\"hello\"" is turned to "hello"
// This one assumes that the string has already been space stripped in both
// ends, as done by StripSpaces above, for example.
std::string_view StripQuotes(std::string_view s) {
if (s.size() && '\"' == s[0] && '\"' == *s.rbegin())
return s.substr(1, s.size() - 2);
else
return s;
}
void SplitString(const std::string& str, const char delim, std::vector<std::string>& output)
{
size_t next = 0;

View File

@ -21,6 +21,7 @@
#include <cstdint>
#include <string>
#include <cstring>
#include <string_view>
#include <vector>
#ifdef _MSC_VER
@ -83,6 +84,9 @@ std::string StringFromInt(int value);
std::string StripSpaces(const std::string &s);
std::string StripQuotes(const std::string &s);
std::string_view StripSpaces(std::string_view s);
std::string_view StripQuotes(std::string_view s);
void SplitString(const std::string& str, const char delim, std::vector<std::string>& output);
void GetQuotedStrings(const std::string& str, std::vector<std::string>& output);

View File

@ -1068,6 +1068,8 @@ void Config::UpdateAfterSettingAutoFrameSkip() {
}
void Config::Load(const char *iniFileName, const char *controllerIniFilename) {
double startTime = time_now_d();
if (!bUpdatedInstanceCounter) {
InitInstanceCounter();
bUpdatedInstanceCounter = true;
@ -1209,10 +1211,11 @@ void Config::Load(const char *iniFileName, const char *controllerIniFilename) {
PostLoadCleanup(false);
INFO_LOG(LOADER, "Config loaded: '%s'", iniFilename_.c_str());
INFO_LOG(LOADER, "Config loaded: '%s' (%0.1f ms)", iniFilename_.c_str(), (time_now_d() - startTime) * 1000.0);
}
bool Config::Save(const char *saveReason) {
double startTime = time_now_d();
if (!IsFirstInstance()) {
// TODO: Should we allow saving config if started from a different directory?
// How do we tell?
@ -1291,7 +1294,7 @@ bool Config::Save(const char *saveReason) {
ERROR_LOG(LOADER, "Error saving config (%s)- can't write ini '%s'", saveReason, iniFilename_.c_str());
return false;
}
INFO_LOG(LOADER, "Config saved (%s): '%s'", saveReason, iniFilename_.c_str());
INFO_LOG(LOADER, "Config saved (%s): '%s' (%0.1f ms)", saveReason, iniFilename_.c_str(), (time_now_d() - startTime) * 1000.0);
if (!bGameSpecific) //otherwise we already did this in saveGameConfig()
{

View File

@ -59,6 +59,7 @@
#include "Common/System/NativeApp.h"
#include "Common/System/System.h"
#include "Common/Thread/ThreadUtil.h"
#include "Common/Data/Format/IniFile.h"
#include "Common/ArmEmitter.h"
#include "Common/BitScan.h"
@ -978,6 +979,24 @@ bool TestSubstitutions() {
return true;
}
bool TestIniFile() {
const std::string testLine = "adsf\\#asdf = jkl\\# # comment";
const std::string testLine2 = "# Just a comment";
std::string temp;
ParsedIniLine line;
line.ParseFrom(testLine);
line.Reconstruct(&temp);
EXPECT_EQ_STR(testLine, temp);
temp.clear();
line.ParseFrom(testLine2);
line.Reconstruct(&temp);
EXPECT_EQ_STR(testLine2, temp);
return true;
}
typedef bool (*TestFunc)();
struct TestItem {
const char *name;
@ -1036,6 +1055,7 @@ TestItem availableTests[] = {
TEST_ITEM(EscapeMenuString),
TEST_ITEM(VFS),
TEST_ITEM(Substitutions),
TEST_ITEM(IniFile),
};
int main(int argc, const char *argv[]) {