1
0
Fork 0
mirror of https://github.com/TerryCavanagh/VVVVVV.git synced 2024-12-23 18:19:43 +01:00

Refactor loading arrays from XML to not use the STL

The current way "arrays" from XML files are loaded (before this commit
is applied) goes something like this:

1. Read the buffer of the contents of the tag using TinyXML-2.

2. Allocate a buffer on the heap of the same size, and copy the
   existing buffer to it. (This is what the statement `std::string
   TextString = pText;` does.)

3. For each delimiter in the heap-allocated buffer...

   a. Allocate another buffer on the heap, and copy the characters from
      the previous delimiter to the delimiter you just hit.

   b. Then allocate the buffer AGAIN, to copy it into an std::vector.

4. Then re-allocate every single buffer YET AGAIN, because you need to
   make a copy of the std::vector in split() to return it to the caller.

As you can see, the existing way uses a lot of memory allocations and
data marshalling, just to split some text.

The problem here is mostly making a temporary std::vector of split text,
before doing any actual useful work (most likely, putting it into an
array or ANOTHER std::vector - if the latter, then that's yet another
memory allocation on top of the memory allocation you already did; this
memory allocation is unavoidable, unlike the ones mentioned earlier,
which should be removed).

So I noticed that since we're iterating over the entire string once
(just to shove its contents into a temporary std::vector), and then
basically iterating over it again - why can't the whole thing just be
more immediate, and just be iterated over once?

So that's what I've done here. I've axed the split() function (both of
them, actually), and made next_split() and next_split_s().

next_split() will take an existing string and a starting index, and it
will find the next occurrence of the given delimiter in the string. Once
it does so, it will return the length from the previous starting index,
and modify your starting index as well. The price for immediateness is
that you're supposed to handle keeping the index of the previous
starting index around in order to be able to use the function; updating
it after each iteration is also your responsibility.

(By the way, next_split() doesn't use SDL_strchr(), because we can't get
the length of the substring for the last substring. We could handle this
special case specifically, but it'd be uglier; it also introduces
iterating over the last substring twice, when we only need to do it
once.)

next_split_s() does the same thing as next_split(), except it will copy
the resulting substring into a buffer that you provide (along with its
size). Useful if you don't particularly care about the length of the
substring.

All callers have been updated accordingly. This new system does not make
ANY heap allocations at all; at worst, it allocates a temporary buffer
on the stack, but that's only if you use next_split_s(); plus, it'd be a
fixed-size buffer, and stack allocations are negligible anyway.

This improves performance when loading any sort of XML file, especially
loading custom levels - which, on my system at least, I can noticeably
tell (there's less of a freeze when I load in to a custom level with
lots of scripts). It also decreases memory usage, because the heap isn't
being used just to iterate over some delimiters when XML files are
loaded.
This commit is contained in:
Misa 2021-02-12 16:37:29 -08:00 committed by Ethan Lee
parent 52959396bb
commit 84ac4a40c1
4 changed files with 122 additions and 41 deletions

View file

@ -445,11 +445,21 @@ void Game::updatecustomlevelstats(std::string clevel, int cscore)
#define LOAD_ARRAY_RENAME(ARRAY_NAME, DEST) \ #define LOAD_ARRAY_RENAME(ARRAY_NAME, DEST) \
if (pKey == #ARRAY_NAME && pText[0] != '\0') \ if (pKey == #ARRAY_NAME && pText[0] != '\0') \
{ \ { \
std::string TextString = pText; \ /* We're loading in 32-bit integers. If we need more than 16 chars,
std::vector<std::string> values = split(TextString, ','); \ * something is seriously wrong */ \
for (int i = 0; i < VVV_min(SDL_arraysize(DEST), values.size()); i++) \ char buffer[16]; \
size_t start = 0; \
size_t i = 0; \
\
while (next_split_s(buffer, sizeof(buffer), &start, pText, ',')) \
{ \ { \
DEST[i] = help.Int(values[i].c_str()); \ if (i >= SDL_arraysize(DEST)) \
{ \
break; \
} \
\
DEST[i] = help.Int(buffer); \
++i; \
} \ } \
} }
@ -541,11 +551,15 @@ void Game::loadcustomlevelstats()
if (pKey == "customlevelstats" && pText[0] != '\0') if (pKey == "customlevelstats" && pText[0] != '\0')
{ {
std::string TextString = (pText); size_t start = 0;
std::vector<std::string> values = split(TextString,'|'); size_t len = 0;
for(size_t i = 0; i < values.size(); i++) size_t prev_start = 0;
while (next_split(&start, &len, &pText[start], '|'))
{ {
customlevelnames.push_back(values[i]); customlevelnames.push_back(std::string(&pText[prev_start], len));
prev_start = start;
} }
} }
} }

View file

@ -4,6 +4,8 @@
#include <SDL.h> #include <SDL.h>
#include <sstream> #include <sstream>
#include "Maths.h"
static const char* GCChar(const SDL_GameControllerButton button) static const char* GCChar(const SDL_GameControllerButton button)
{ {
switch (button) switch (button)
@ -71,21 +73,61 @@ int ss_toi(const std::string& str)
return retval; return retval;
} }
std::vector<std::string> split( const std::string &s, char delim, std::vector<std::string> &elems ) bool next_split(
size_t* start,
size_t* len,
const char* str,
const char delim
) {
size_t idx = 0;
*len = 0;
if (str[idx] == '\0')
{ {
std::stringstream ss(s); return false;
std::string item;
while(std::getline(ss, item, delim))
{
elems.push_back(item);
}
return elems;
} }
std::vector<std::string> split( const std::string &s, char delim ) while (true)
{ {
std::vector<std::string> elems; if (str[idx] == delim)
return split(s, delim, elems); {
*start += 1;
return true;
}
else if (str[idx] == '\0')
{
return true;
}
idx += 1;
*start += 1;
*len += 1;
}
}
bool next_split_s(
char buffer[],
const size_t buffer_size,
size_t* start,
const char* str,
const char delim
) {
size_t len = 0;
const size_t prev_start = *start;
const bool retval = next_split(start, &len, &str[*start], delim);
if (retval)
{
/* Using SDL_strlcpy() here results in calling SDL_strlen() */
/* on the whole string, which results in a visible freeze */
/* if it's a very large string */
const size_t length = VVV_min(buffer_size, len);
SDL_memcpy(buffer, &str[prev_start], length);
buffer[length] = '\0';
}
return retval;
} }
UtilityClass::UtilityClass() : UtilityClass::UtilityClass() :

View file

@ -7,9 +7,20 @@
int ss_toi(const std::string& str); int ss_toi(const std::string& str);
std::vector<std::string> split(const std::string &s, char delim, std::vector<std::string> &elems); bool next_split(
size_t* start,
size_t* len,
const char* str,
const char delim
);
std::vector<std::string> split(const std::string &s, char delim); bool next_split_s(
char buffer[],
const size_t buffer_size,
size_t* start,
const char* str,
const char delim
);
bool is_number(const char* str); bool is_number(const char* str);

View file

@ -1748,20 +1748,28 @@ bool editorclass::load(std::string& _path)
if (pKey == "contents" && pText[0] != '\0') if (pKey == "contents" && pText[0] != '\0')
{ {
std::string TextString = (pText);
std::vector<std::string> values = split(TextString,',');
int x = 0; int x = 0;
int y = 0; int y = 0;
for(size_t i = 0; i < values.size(); i++)
char buffer[16];
size_t start = 0;
while (next_split_s(buffer, sizeof(buffer), &start, pText, ','))
{ {
contents[x + (maxwidth*40*y)] = help.Int(values[i].c_str()); const int idx = x + maxwidth*40*y;
x++;
if (INBOUNDS_ARR(idx, contents))
{
contents[idx] = help.Int(buffer);
}
++x;
if (x == mapwidth*40) if (x == mapwidth*40)
{ {
x = 0; x = 0;
y++; ++y;
} }
} }
} }
@ -1865,30 +1873,36 @@ bool editorclass::load(std::string& _path)
if (pKey == "script" && pText[0] != '\0') if (pKey == "script" && pText[0] != '\0')
{ {
std::string TextString = (pText);
std::vector<std::string> values = split(TextString,'|');
Script script_; Script script_;
bool headerfound = false; bool headerfound = false;
for(size_t i = 0; i < values.size(); i++)
{
std::string& line = values[i];
if (line.length() && line[line.length() - 1] == ':') size_t start = 0;
size_t len = 0;
size_t prev_start = 0;
while (next_split(&start, &len, &pText[start], '|'))
{
if (len > 0 && pText[prev_start + len - 1] == ':')
{ {
if (headerfound) if (headerfound)
{ {
script.customscripts.push_back(script_); script.customscripts.push_back(script_);
} }
script_.name = line.substr(0, line.length()-1);
script_.name = std::string(&pText[prev_start], len - 1);
script_.contents.clear(); script_.contents.clear();
headerfound = true; headerfound = true;
continue;
goto next;
} }
if (headerfound) if (headerfound)
{ {
script_.contents.push_back(line); script_.contents.push_back(std::string(&pText[prev_start], len));
} }
next:
prev_start = start;
} }
/* Add the last script */ /* Add the last script */