Skip to content

Commit

Permalink
Utility: port Resource away from STL.
Browse files Browse the repository at this point in the history
The list() function now returns an array of string views and get() is
deprecated in favor of getString() returning a string view as well. The
returned string views have the Global flag set where possible, with null
termination being reserved for a future opt-in feature.

The internals were reworked as well, replacing a lot of ArrayViews with
nicer StringView APIs and getting rid of many unnecessary allocations --
we can now store filenames and group names as string views pointing to
the compiled-in data, so there's no need to allocate anything.

Binary-size-wise, this reduced libCorradeUtility.so by about 3 kB (461
before, 458 after) in Release. Not much, but it's going in the right
direction at least.
  • Loading branch information
mosra committed Mar 10, 2022
1 parent 60f6489 commit 64b922f
Show file tree
Hide file tree
Showing 9 changed files with 315 additions and 266 deletions.
12 changes: 12 additions & 0 deletions doc/corrade-changelog.dox
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,14 @@ namespace Corrade {
@relativeref{Utility::Path,mapWrite()} and
@relativeref{Utility::Path,MapDeleter}, returning the mapped array
wrapped in an @ref Containers::Optional
- The @ref Utility::Resource class was ported to use
@ref Containers::StringView instead of @ref std::string,
- @cpp Utility::Resource::get() @ce is deprecated in favor of
@ref Utility::Resource::getString(). The @cpp get() @ce API is scheduled to
be reintroduced --- with a @ref Containers::ArrayView return type
consistent with @ref Utility::Path::read() and
@ref Utility::Path::readString() --- once enough time passes after the
deprecated API gets removed to avoid silent breakages in existing code.
- @ref Utility::String::split() and
@ref Utility::String::splitWithoutEmptyParts() overloads taking
@ref Containers::BasicStringView are deprecated in favor of
Expand Down Expand Up @@ -511,6 +519,10 @@ namespace Corrade {
- @cpp Utility::Resource::compile() @ce and
@cpp Utility::Resource::compileFrom() @ce APIs are no longer public, but
rather a private detail of the @ref corrade-rc "corrade-rc" utility.
- @cpp Utility::Resource::list() @ce now returns a @ref Containers::Array of
@ref Containers::StringView instead of a @ref std::vector. Because the
reason for this change was to get rid of @cpp #include <vector> @ce, no
backwards compatibility is provided.
- With the ongoing process to make the APIs less STL dependent,
@ref Utility::Unicode::narrow() and @relativeref{Utility::Unicode,widen()}
variants taking a view or a char pointer now return either a
Expand Down
5 changes: 3 additions & 2 deletions doc/snippets/Utility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -806,14 +806,15 @@ static_cast<void>(shader);
/* [Resource-usage] */
Utility::Resource rs{"game-data"};

std::string licenseText = rs.get("license.txt");
Containers::StringView licenseText = rs.getString("license.txt");
Containers::ArrayView<const char> soundData = rs.getRaw("intro.ogg");
DOXYGEN_ELLIPSIS()

std::istringstream in{rs.get("levels/easy.conf")};
std::istringstream in{rs.getString("levels/easy.conf")};
Utility::Configuration easyLevel{in};
DOXYGEN_ELLIPSIS()
/* [Resource-usage] */
static_cast<void>(licenseText);
static_cast<void>(soundData);
}

Expand Down
2 changes: 1 addition & 1 deletion src/Corrade/PluginManager/AbstractManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ AbstractManager::AbstractManager(std::string pluginInterface, std::string plugin
Utility::Configuration configuration;
if(!_state->pluginMetadataSuffix.empty()) {
Utility::Resource rs{std::string{"CorradeStaticPlugin_"} + staticPlugin->plugin};
std::istringstream metadata(rs.get(staticPlugin->plugin + _state->pluginMetadataSuffix));
std::istringstream metadata(rs.getString(staticPlugin->plugin + _state->pluginMetadataSuffix));
configuration = Utility::Configuration{metadata, Utility::Configuration::Flag::ReadOnly};
}

Expand Down
17 changes: 9 additions & 8 deletions src/Corrade/Utility/Implementation/Resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,29 +26,29 @@
DEALINGS IN THE SOFTWARE.
*/

#include <cstring>
#include <algorithm> /* std::lower_bound() */
#include <Corrade/Containers/StringView.h>
#include <Corrade/Containers/ArrayView.h>

namespace Corrade { namespace Utility { namespace Implementation {

inline Containers::ArrayView<const char> resourceFilenameAt(const unsigned int* const positions, const unsigned char* const filenames, const std::size_t i) {
inline Containers::StringView resourceFilenameAt(const unsigned int* const positions, const unsigned char* const filenames, const std::size_t i) {
/* Every position pair denotes end offsets of one file, filename is first */
const std::size_t begin = i == 0 ? 0 : positions[2*(i - 1)];
const std::size_t end = positions[2*i];
return {reinterpret_cast<const char*>(filenames) + begin, end - begin};
return {reinterpret_cast<const char*>(filenames) + begin, end - begin, Containers::StringViewFlag::Global};
}

inline Containers::ArrayView<const char> resourceDataAt(const unsigned int* const positions, const unsigned char* const data, const std::size_t i) {
inline Containers::StringView resourceDataAt(const unsigned int* const positions, const unsigned char* const data, const std::size_t i) {
/* Every position pair denotes end offsets of one file, data is second */
const std::size_t begin = i == 0 ? 0 : positions[2*(i - 1) + 1];
const std::size_t end = positions[2*i + 1];
return {reinterpret_cast<const char*>(data) + begin, end - begin};
return {reinterpret_cast<const char*>(data) + begin, end - begin, Containers::StringViewFlag::Global};
}

/* Assuming the filenames are sorted, look up a particular filename. Returns
either its index or count if not found. */
inline std::size_t resourceLookup(const unsigned int count, const unsigned int* const positionData, const unsigned char* const filenames, const Containers::ArrayView<const char> filename) {
inline std::size_t resourceLookup(const unsigned int count, const unsigned int* const positionData, const unsigned char* const filenames, const Containers::StringView filename) {
/* Like std::map, but without crazy allocations using std::lower_bound and
a std::lexicographical_compare */
struct Position {
Expand All @@ -60,6 +60,8 @@ inline std::size_t resourceLookup(const unsigned int count, const unsigned int*
[positions, filenames](const Position& position, const Containers::ArrayView<const char> filename) {
const std::size_t end = position.filename;
const std::size_t begin = &position == positions ? 0 : (&position - 1)->filename;
/* Not constructing a temporary StringView here as this shall be
faster */
return std::lexicographical_compare(filenames + begin, filenames + end,
filename.begin(), filename.end());
});
Expand All @@ -70,8 +72,7 @@ inline std::size_t resourceLookup(const unsigned int count, const unsigned int*
/* Check that the filenames match --- it only returns a lower bound, not an
exact match */
const std::size_t i = found - positions.begin();
const Containers::ArrayView<const char> foundFilename = resourceFilenameAt(positionData, filenames, i);
if(filename.size() != foundFilename.size() || std::memcmp(filename, foundFilename, filename.size())) return count;
if(filename != resourceFilenameAt(positionData, filenames, i)) return count;

/* Return the found index */
return i;
Expand Down
115 changes: 55 additions & 60 deletions src/Corrade/Utility/Resource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
#include "Corrade/Utility/Assert.h"
#include "Corrade/Utility/Configuration.h"
#include "Corrade/Utility/ConfigurationGroup.h"
#include "Corrade/Utility/DebugStl.h"
#include "Corrade/Utility/Path.h"
#include "Corrade/Utility/Implementation/Resource.h"

Expand All @@ -59,8 +58,9 @@ struct ResourceGlobals {

/* Overridden groups. This is only allocated if the user calls
Resource::overrideGroup() and stores a pointer to a function-local
static variable from there. */
std::map<std::string, std::string>* overrideGroups;
static variable from there. The keys point to names of existing groups,
thus don't need to be allocated. */
std::map<Containers::StringView, Containers::String>* overrideGroups;
};

/* What the hell is going on here with the #ifdefs?! */
Expand Down Expand Up @@ -116,9 +116,11 @@ ResourceGlobals& windowsResourceGlobals() {

struct Resource::OverrideData {
const Configuration conf;
std::map<std::string, Containers::Array<char>> data;
/* Here the key is again pointing to names of existing files, thus no need
to be allocated */
std::map<Containers::StringView, Containers::Array<char>> data;

explicit OverrideData(const std::string& filename): conf(filename) {}
explicit OverrideData(const Containers::StringView filename): conf(filename) {}
};

void Resource::registerData(Implementation::ResourceGroup& resource) {
Expand All @@ -130,57 +132,47 @@ void Resource::unregisterData(Implementation::ResourceGroup& resource) {
}

namespace {
Implementation::ResourceGroup* findGroup(const Containers::ArrayView<const char> name) {
Implementation::ResourceGroup* findGroup(const Containers::StringView name) {
for(Implementation::ResourceGroup* group = resourceGlobals.groups; group; group = Containers::Implementation::forwardListNext(*group)) {
/* std::strncmp() would return equality also if name was just a
prefix of group->name, so test that it ends with a null
terminator */
if(std::strncmp(group->name, name, name.size()) == 0 && group->name[name.size()] == '\0') return group;
if(group->name == name) return group;
}

return nullptr;
}
}

void Resource::overrideGroup(const std::string& group, const std::string& configurationFile) {
void Resource::overrideGroup(const Containers::StringView group, const Containers::StringView configurationFile) {
if(!resourceGlobals.overrideGroups) {
static std::map<std::string, std::string> overrideGroups;
static std::map<Containers::StringView, Containers::String> overrideGroups;
resourceGlobals.overrideGroups = &overrideGroups;
}

CORRADE_ASSERT(findGroup({group.data(), group.size()}),
"Utility::Resource::overrideGroup(): group" << '\'' + group + '\'' << "was not found", );
CORRADE_ASSERT(findGroup(group),
"Utility::Resource::overrideGroup(): group '" << Debug::nospace << group << Debug::nospace << "' was not found", );
/* This group can be already overridden from before, so insert if not there
yet and then update the filename */
resourceGlobals.overrideGroups->emplace(group, std::string{}).first->second = configurationFile;
resourceGlobals.overrideGroups->emplace(group, Containers::String{}).first->second = Containers::String::nullTerminatedGlobalView(configurationFile);
}

bool Resource::hasGroup(const std::string& group) {
return hasGroupInternal({group.data(), group.size()});
}

bool Resource::hasGroupInternal(const Containers::ArrayView<const char> group) {
bool Resource::hasGroup(const Containers::StringView group) {
return findGroup(group);
}

Resource::Resource(const std::string& group): Resource{{group.data(), group.size()}, nullptr} {}

Resource::Resource(const Containers::ArrayView<const char> group, void*): _group{findGroup(group)}, _overrideGroup(nullptr) {
CORRADE_ASSERT(_group, "Utility::Resource: group '" << Debug::nospace << (std::string{group, group.size()}) << Debug::nospace << "' was not found", );
Resource::Resource(const Containers::StringView group): _group{findGroup(group)}, _overrideGroup(nullptr) {
CORRADE_ASSERT(_group, "Utility::Resource: group '" << Debug::nospace << group << Debug::nospace << "' was not found", );

if(resourceGlobals.overrideGroups) {
const std::string groupString{group.data(), group.size()};
auto overridden = resourceGlobals.overrideGroups->find(groupString);
auto overridden = resourceGlobals.overrideGroups->find(group);
if(overridden != resourceGlobals.overrideGroups->end()) {
Debug{}
<< "Utility::Resource: group '" << Debug::nospace << groupString << Debug::nospace << "' overridden with '" << Debug::nospace << overridden->second << Debug::nospace << "\'";
<< "Utility::Resource: group '" << Debug::nospace << group << Debug::nospace << "' overridden with '" << Debug::nospace << overridden->second << Debug::nospace << "\'";
_overrideGroup = new OverrideData(overridden->second);

if(_overrideGroup->conf.value("group") != groupString) Warning{}
if(_overrideGroup->conf.value<Containers::StringView>("group") != group) Warning{}
<< "Utility::Resource: overridden with different group, found '"
<< Debug::nospace << _overrideGroup->conf.value("group")
<< Debug::nospace << _overrideGroup->conf.value<Containers::StringView>("group")
<< Debug::nospace << "' but expected '" << Debug::nospace
<< groupString << Debug::nospace << "'";
<< group << Debug::nospace << "'";
}
}
}
Expand All @@ -189,69 +181,72 @@ Resource::~Resource() {
delete _overrideGroup;
}

std::vector<std::string> Resource::list() const {
Containers::Array<Containers::StringView> Resource::list() const {
CORRADE_INTERNAL_ASSERT(_group);

std::vector<std::string> result;
result.reserve(_group->count);
for(std::size_t i = 0; i != _group->count; ++i) {
Containers::ArrayView<const char> filename = Implementation::resourceFilenameAt(_group->positions, _group->filenames, i);
result.push_back({filename.data(), filename.size()});
}

return result;
Containers::Array<Containers::StringView> out{NoInit, _group->count};
for(std::size_t i = 0; i != _group->count; ++i)
new(&out[i]) Containers::StringView{Implementation::resourceFilenameAt(_group->positions, _group->filenames, i)};
return out;
}

Containers::ArrayView<const char> Resource::getRaw(const std::string& filename) const {
return getInternal({filename.data(), filename.size()});
Containers::ArrayView<const char> Resource::getRaw(const Containers::StringView filename) const {
/* Going this way instead of the other way around because the StringView
can hold information about null termination or global lifetime, which
would be painful to query in a subsequent step */
return getString(filename);
}

Containers::ArrayView<const char> Resource::getInternal(const Containers::ArrayView<const char> filename) const {
Containers::StringView Resource::getString(const Containers::StringView filename) const {
CORRADE_INTERNAL_ASSERT(_group);

/* Look for the file in compiled-in resources. This is done before looking
into an overriden group configuration file to prevent retrieving files
that aren't compiled in. */
const unsigned int i = Implementation::resourceLookup(_group->count, _group->positions, _group->filenames, filename);
CORRADE_ASSERT(i != _group->count,
"Utility::Resource::get(): file '" << Debug::nospace << filename << Debug::nospace << "' was not found in group '" << Debug::nospace << _group->name << Debug::nospace << "'", {});

/* The group is overridden with live data */
if(_overrideGroup) {
const std::string filenameString{filename.data(), filename.size()};

/* The file is already loaded */
auto it = _overrideGroup->data.find(filenameString);
auto it = _overrideGroup->data.find(filename);
if(it != _overrideGroup->data.end())
return it->second;
return Containers::ArrayView<const char>{it->second};

/* Load the file and save it for later use. Linear search is not an
issue, as this shouldn't be used in production code anyway. */
std::vector<const ConfigurationGroup*> files = _overrideGroup->conf.groups("file");
for(auto file: files) {
const std::string name = file->hasValue("alias") ? file->value("alias") : file->value("filename");
if(name != filenameString) continue;
const Containers::StringView name = file->hasValue("alias") ? file->value<Containers::StringView>("alias") : file->value<Containers::StringView>("filename");
if(name != filename) continue;

/* Load the file */
Containers::Optional<Containers::Array<char>> data = Path::read(Path::join(Path::split(_overrideGroup->conf.filename()).first(), file->value("filename")));
if(!data) {
Error() << "Utility::Resource::get(): cannot open file" << file->value("filename") << "from overridden group";
Error{} << "Utility::Resource::get(): cannot open file" << file->value<Containers::StringView>("filename") << "from overridden group";
break;
}

/* Save the file for later use and return */
it = _overrideGroup->data.emplace(filenameString, *std::move(data)).first;
return it->second;
/* Save the file for later use and return. Use a filename from the
compiled-in resources which is guaranteed to be global to avoid
allocating a new string */
it = _overrideGroup->data.emplace(Implementation::resourceFilenameAt(_group->positions, _group->filenames, i), *std::move(data)).first;
return Containers::ArrayView<const char>{it->second};
}

/* The file was not found, fallback to compiled-in ones */
Warning() << "Utility::Resource::get(): file '" << Debug::nospace
<< filenameString << Debug::nospace << "' was not found in overridden group, fallback to compiled-in resources";
Warning{} << "Utility::Resource::get(): file '" << Debug::nospace
<< filename << Debug::nospace << "' was not found in overridden group, fallback to compiled-in resources";
}

const unsigned int i = Implementation::resourceLookup(_group->count, _group->positions, _group->filenames, filename);
CORRADE_ASSERT(i != _group->count,
"Utility::Resource::get(): file '" << Debug::nospace << (std::string{filename, filename.size()}) << Debug::nospace << "' was not found in group '" << Debug::nospace << _group->name << Debug::nospace << "\'", nullptr);

return Implementation::resourceDataAt(_group->positions, _group->data, i);
}

#ifdef CORRADE_BUILD_DEPRECATED
std::string Resource::get(const std::string& filename) const {
Containers::ArrayView<const char> data = getRaw(filename);
return data ? std::string{data, data.size()} : std::string{};
return getString(filename);
}
#endif

}}
Loading

0 comments on commit 64b922f

Please sign in to comment.