Skip to content

Commit

Permalink
Refactor|Cleanup: Internally simpler PathTree
Browse files Browse the repository at this point in the history 
No hashing or string interning. Instead, the node index is an ordered case-insensitive multimap.
  • Loading branch information
@skyjake
skyjake committed Sep 1, 2019
1 parent c50a041 commit f411b7d
Show file tree
Hide file tree
Showing 12 changed files with 229 additions and 207 deletions.
7 changes: 5 additions & 2 deletions doomsday/apps/client/src/resource/clientresources.cpp
View file
Expand Up @@ -720,8 +720,11 @@ DE_PIMPL(ClientResources)
// The "first choice" directory is that in which the model file resides.
try
{
return fileSys().findPath(res::Uri("Models", modelFilePath.toString().fileNamePath() / skinPath.fileName()),
RLF_DEFAULT, self().resClass(RC_GRAPHIC));
return fileSys().findPath(
res::Uri("Models",
modelFilePath.toString().fileNamePath() / skinPath.fileName()),
RLF_DEFAULT,
self().resClass(RC_GRAPHIC));
}
catch (FS1::NotFoundError const &)
{} // Ignore this error.
Expand Down
14 changes: 7 additions & 7 deletions doomsday/libs/core/include/de/data/path.h
View file
Expand Up @@ -52,10 +52,10 @@ class DE_PUBLIC Path : public ISerializable, public LogEntry::Arg::Base
DE_ERROR(OutOfBoundsError);

/// Type used to represent a path segment hash key.
typedef duint32 hash_type;
// typedef duint32 hash_type;

/// Range of a path segment hash key; [0..hash_range)
static hash_type const hash_range;
// static hash_type const hash_range;

/**
* Marks a segment in the path. Makes no copy of the segments in the path,
Expand Down Expand Up @@ -98,13 +98,13 @@ class DE_PUBLIC Path : public ISerializable, public LogEntry::Arg::Base
*/
dsize size() const;

/**
/*
* Returns a somewhat-random number in the range [0..Path::hash_range)
* generated from the segment.
*
* @return The generated hash key.
*/
hash_type hash() const;
// hash_type hash() const;

bool hasWildCard() const;

Expand All @@ -130,13 +130,13 @@ class DE_PUBLIC Path : public ISerializable, public LogEntry::Arg::Base
* Returns @c true if this segment is lexically less than @a other.
* The test is case and separator insensitive.
*/
bool operator<(Segment const &other) const;
bool operator<(const Segment &other) const;

enum Flag { GotHashKey = 0x1, WildCardChecked = 0x2, IncludesWildCard = 0x4 };

private:
mutable Flags flags;
mutable hash_type hashKey;
// mutable hash_type hashKey;
CString range;

friend class Path;
Expand Down Expand Up @@ -349,7 +349,7 @@ class DE_PUBLIC Path : public ISerializable, public LogEntry::Arg::Base
/**
* Returns the file name portion of the path, i.e., the last segment.
*/
String fileName() const;
CString fileName() const;

Block toUtf8() const;

Expand Down
38 changes: 18 additions & 20 deletions doomsday/libs/core/include/de/data/pathtree.h
View file
Expand Up @@ -25,7 +25,7 @@
#include <de/String>
#include <de/Path>

#include <unordered_map>
#include <map>

namespace de {

Expand Down Expand Up @@ -67,7 +67,7 @@ class DE_PUBLIC PathTree : public Lockable
public:
class Node; // forward declaration

typedef std::unordered_multimap<Path::hash_type, Node *> Nodes;
typedef std::multimap<String, Node *, String::InsensitiveLessThan> Nodes;
typedef StringList FoundPaths;

/**
Expand Down Expand Up @@ -104,7 +104,7 @@ class DE_PUBLIC PathTree : public Lockable
using ComparisonFlags = Flags;

/// Identifier associated with each unique path segment.
typedef duint32 SegmentId;
//typedef duint32 SegmentId;

/// Node type identifiers.
enum NodeType
Expand All @@ -121,7 +121,7 @@ class DE_PUBLIC PathTree : public Lockable
* a hash when the user does not wish to narrow the set of considered
* nodes.
*/
static Path::hash_type const no_hash;
// static Path::hash_type const no_hash;

#ifdef DE_DEBUG
void debugPrint(Char separator = '/') const;
Expand All @@ -140,11 +140,11 @@ class DE_PUBLIC PathTree : public Lockable
{
PathTree &tree;
NodeType type;
SegmentId segmentId;
CString segment;
Node *parent;

NodeArgs(PathTree &pt, NodeType nt, SegmentId id, Node *p = 0)
: tree(pt), type(nt), segmentId(id), parent(p) {}
NodeArgs(PathTree &pt, NodeType nt, const CString &segment, /*SegmentId id, */Node *p = 0)
: tree(pt), type(nt), segment(segment), /*segmentId(id), */parent(p) {}
};

/**
Expand Down Expand Up @@ -203,7 +203,7 @@ class DE_PUBLIC PathTree : public Lockable
String const &name() const;

/// @return Hash for this node's path segment.
Path::hash_type hash() const;
// Path::hash_type hash() const;

/**
* @param searchPattern Mapped search pattern (path).
Expand Down Expand Up @@ -239,7 +239,7 @@ class DE_PUBLIC PathTree : public Lockable
friend struct PathTree::Impl;

protected:
SegmentId segmentId() const;
// SegmentId segmentId() const;
void addChild(Node &node);
void removeChild(Node &node);
Nodes &childNodes(NodeType type);
Expand Down Expand Up @@ -357,14 +357,12 @@ class DE_PUBLIC PathTree : public Lockable
* @param parent Used in combination with ComparisonFlag::MatchParent
* to limit the traversal to only the child nodes of
* this node.
* @param hashKey If not @c PathTree::no_hash only consider nodes whose
* hashed name matches this.
* @param callback Callback function ptr.
* @param parameters Passed to the callback.
*
* @return @c 0 iff iteration completed wholly.
*/
int traverse(ComparisonFlags flags, Node const *parent, Path::hash_type hashKey,
int traverse(ComparisonFlags flags, Node const *parent,
int (*callback) (Node &node, void *parameters), void *parameters = 0) const;

/**
Expand Down Expand Up @@ -399,11 +397,11 @@ class DE_PUBLIC PathTree : public Lockable
* Methods usually only needed by Node (or derivative classes).
*/

/// @return The path segment associated with @a segmentId.
String const &segmentName(SegmentId segmentId) const;
// /// @return The path segment associated with @a segmentId.
// String const &segmentName(SegmentId segmentId) const;

/// @return Hash associated with @a segmentId.
Path::hash_type segmentHash(SegmentId segmentId) const;
// /// @return Hash associated with @a segmentId.
// Path::hash_type segmentHash(SegmentId segmentId) const;

Node const &rootBranch() const;

Expand Down Expand Up @@ -478,7 +476,7 @@ class PathTreeIterator
return val;
}

Path::hash_type key() const {
String key() const {
DE_ASSERT(_current != _nodes.end());
return _current->first;
}
Expand All @@ -501,7 +499,7 @@ class PathTreeT : public PathTree
{
public:
typedef Type Node; // shadow PathTree::Node
typedef std::unordered_multimap<Path::hash_type, Type *> Nodes;
typedef std::multimap<String, Type *, String::InsensitiveLessThan> Nodes;
typedef List<Type *> FoundNodes;

public:
Expand Down Expand Up @@ -542,9 +540,9 @@ class PathTreeT : public PathTree
return found.size() - numFoundSoFar;
}

inline int traverse(ComparisonFlags flags, Type const *parent, Path::hash_type hashKey,
inline int traverse(ComparisonFlags flags, Type const *parent, //Path::hash_type hashKey,
int (*callback) (Type &node, void *context), void *context = 0) const {
return PathTree::traverse(flags, parent, hashKey,
return PathTree::traverse(flags, parent, //hashKey,
reinterpret_cast<int (*)(PathTree::Node &, void *)>(callback),
context);
}
Expand Down
48 changes: 24 additions & 24 deletions doomsday/libs/core/src/data/path.cpp
View file
Expand Up @@ -32,18 +32,18 @@ namespace de {

//---------------------------------------------------------------------------------------

Path::hash_type const Path::hash_range = 0xffffffff;

Path::hash_type Path::Segment::hash() const
{
// Is it time to compute the hash?
if (!(flags & GotHashKey))
{
hashKey = de::crc32(range.lower()) % hash_range;
flags |= GotHashKey;
}
return hashKey;
}
//Path::hash_type const Path::hash_range = 0xffffffff;

//Path::hash_type Path::Segment::hash() const
//{
// // Is it time to compute the hash?
// if (!(flags & GotHashKey))
// {
// hashKey = de::crc32(range.lower()) % hash_range;
// flags |= GotHashKey;
// }
// return hashKey;
//}

bool Path::Segment::hasWildCard() const
{
Expand All @@ -57,12 +57,12 @@ bool Path::Segment::hasWildCard() const
return isWild;
}

bool Path::Segment::operator==(Segment const &other) const
bool Path::Segment::operator==(const Segment &other) const
{
return !range.compare(other.range, CaseInsensitive);
}

bool Path::Segment::operator < (Segment const &other) const
bool Path::Segment::operator<(const Segment &other) const
{
return range.compare(other.range, CaseInsensitive) < 0;
}
Expand Down Expand Up @@ -364,11 +364,11 @@ bool Path::operator == (Path const &other) const
if (segmentCount() != other.segmentCount()) return false;

// If the hashes are different, the segments can't be the same.
for (dsize i = 0; i < d->segmentCount; ++i)
{
if (segment(i).hash() != other.segment(i).hash())
return false;
}
// for (dsize i = 0; i < d->segmentCount; ++i)
// {
// if (segment(i).hash() != other.segment(i).hash())
// return false;
// }

// Probably the same, but we have to make sure by comparing
// the textual segments.
Expand All @@ -390,10 +390,10 @@ bool Path::operator == (Path const &other) const

bool Path::operator==(const char *cstr) const
{
return d->path == cstr;
return d->path.compareWithoutCase(cstr) == 0;
}

bool Path::operator < (Path const &other) const
bool Path::operator<(const Path &other) const
{
if (d->separator == other.d->separator)
{
Expand Down Expand Up @@ -536,10 +536,10 @@ void Path::addTerminatingSeparator()
}
}

String Path::fileName() const
CString Path::fileName() const
{
if (last() == d->separator) return "";
return lastSegment().toRange();
return lastSegment();
}

Block Path::toUtf8() const
Expand Down Expand Up @@ -712,7 +712,7 @@ static int Path_UnitTest()
DE_ASSERT(b.segment(1).toString() == "variable");
}

// .
// Test fileName().
{
Path p;
Path a("hello");
Expand Down

0 comments on commit f411b7d

Please sign in to comment.