Refactored the StringPool and Identifier classes to store the identifiers as Strings, so that they can be shared with other classes like XmlElement without creating temporary or copied String objects. Also added garbage collection for the pooled strings, and changed XmlElement to pool all of the strings it uses, to reduce memory footprint in large XML trees with many identical names. Also refactored NamedValueSet to use an array instead of a linked list.

extras/Demo/Source/Demos/XMLandJSONDemo.cpp

@@ -149,7 +149,7 @@ public:
                     {
                         var& child (json[i]);
                         jassert (! child.isVoid());
-                        addSubItem (new JsonTreeItem (Identifier::null, child));
+                        addSubItem (new JsonTreeItem (Identifier(), child));
                     }
                 }
                 else if (DynamicObject* obj = json.getDynamicObject())
@@ -349,7 +349,7 @@ private:
             return nullptr;
         }
 
-        return new JsonTreeItem (Identifier::null, parsedJson);
+        return new JsonTreeItem (Identifier(), parsedJson);
     }
 
     /** Clears the editor and loads some default text. */

modules/juce_core/containers/juce_DynamicObject.cpp

@@ -85,16 +85,9 @@ void DynamicObject::clear()
 
 void DynamicObject::cloneAllProperties()
 {
-    for (LinkedListPointer<NamedValueSet::NamedValue>* i = &(properties.values);;)
-    {
-        if (NamedValueSet::NamedValue* const v = i->get())
-        {
-            v->value = v->value.clone();
-            i = &(v->nextListItem);
-        }
-        else
-            break;
-    }
+    for (int i = properties.size(); --i >= 0;)
+        if (var* v = properties.getVarPointerAt (i))
+            *v = v->clone();
 }
 
 DynamicObject::Ptr DynamicObject::clone()
@@ -110,32 +103,27 @@ void DynamicObject::writeAsJSON (OutputStream& out, const int indentLevel, const
     if (! allOnOneLine)
         out << newLine;
 
-    for (LinkedListPointer<NamedValueSet::NamedValue>* i = &(properties.values);;)
+    const int numValues = properties.size();
+
+    for (int i = 0; i < numValues; ++i)
     {
-        if (NamedValueSet::NamedValue* const v = i->get())
+        if (! allOnOneLine)
+            JSONFormatter::writeSpaces (out, indentLevel + JSONFormatter::indentSize);
+
+        out << '"';
+        JSONFormatter::writeString (out, properties.getName (i));
+        out << "\": ";
+        JSONFormatter::write (out, properties.getValueAt (i), indentLevel + JSONFormatter::indentSize, allOnOneLine);
+
+        if (i < numValues - 1)
         {
-            if (! allOnOneLine)
-                JSONFormatter::writeSpaces (out, indentLevel + JSONFormatter::indentSize);
-
-            out << '"';
-            JSONFormatter::writeString (out, v->name);
-            out << "\": ";
-            JSONFormatter::write (out, v->value, indentLevel + JSONFormatter::indentSize, allOnOneLine);
-
-            if (v->nextListItem.get() != nullptr)
-            {
-                if (allOnOneLine)
-                    out << ", ";
-                else
-                    out << ',' << newLine;
-            }
-            else if (! allOnOneLine)
-                out << newLine;
-
-            i = &(v->nextListItem);
+            if (allOnOneLine)
+                out << ", ";
+            else
+                out << ',' << newLine;
         }
-        else
-            break;
+        else if (! allOnOneLine)
+            out << newLine;
     }
 
     if (! allOnOneLine)

modules/juce_core/containers/juce_NamedValueSet.cpp

@@ -26,53 +26,37 @@
   ==============================================================================
 */
 
-NamedValueSet::NamedValue::NamedValue() noexcept
+struct NamedValueSet::NamedValue
 {
-}
-
-inline NamedValueSet::NamedValue::NamedValue (Identifier n, const var& v)
-    : name (n), value (v)
-{
-}
-
-NamedValueSet::NamedValue::NamedValue (const NamedValue& other)
-    : name (other.name), value (other.value)
-{
-}
-
-NamedValueSet::NamedValue& NamedValueSet::NamedValue::operator= (const NamedValueSet::NamedValue& other)
-{
-    name = other.name;
-    value = other.value;
-    return *this;
-}
+    NamedValue() noexcept {}
+    NamedValue (Identifier n, const var& v)  : name (n), value (v) {}
+    NamedValue (const NamedValue& other) : name (other.name), value (other.value) {}
+
+   #if JUCE_COMPILER_SUPPORTS_MOVE_SEMANTICS
+    NamedValue (NamedValue&& other) noexcept
+        : name (static_cast<Identifier&&> (other.name)),
+          value (static_cast<var&&> (other.value))
+    {
+    }
 
-#if JUCE_COMPILER_SUPPORTS_MOVE_SEMANTICS
-NamedValueSet::NamedValue::NamedValue (NamedValue&& other) noexcept
-    : nextListItem (static_cast<LinkedListPointer<NamedValue>&&> (other.nextListItem)),
-      name (static_cast<Identifier&&> (other.name)),
-      value (static_cast<var&&> (other.value))
-{
-}
+    NamedValue (Identifier n, var&& v)  : name (n), value (static_cast<var&&> (v))
+    {
+    }
 
-inline NamedValueSet::NamedValue::NamedValue (Identifier n, var&& v)
-    : name (n), value (static_cast<var&&> (v))
-{
-}
+    NamedValue& operator= (NamedValue&& other) noexcept
+    {
+        name = static_cast<Identifier&&> (other.name);
+        value = static_cast<var&&> (other.value);
+        return *this;
+    }
+   #endif
 
-NamedValueSet::NamedValue& NamedValueSet::NamedValue::operator= (NamedValue&& other) noexcept
-{
-    nextListItem = static_cast<LinkedListPointer<NamedValue>&&> (other.nextListItem);
-    name = static_cast<Identifier&&> (other.name);
-    value = static_cast<var&&> (other.value);
-    return *this;
-}
-#endif
+    bool operator== (const NamedValue& other) const noexcept   { return name == other.name && value == other.value; }
+    bool operator!= (const NamedValue& other) const noexcept   { return ! operator== (other); }
 
-bool NamedValueSet::NamedValue::operator== (const NamedValueSet::NamedValue& other) const noexcept
-{
-    return name == other.name && value == other.value;
-}
+    Identifier name;
+    var value;
+};
 
 //==============================================================================
 NamedValueSet::NamedValueSet() noexcept
@@ -80,20 +64,20 @@ NamedValueSet::NamedValueSet() noexcept
 }
 
 NamedValueSet::NamedValueSet (const NamedValueSet& other)
+   : values (other.values)
 {
-    values.addCopyOfList (other.values);
 }
 
 NamedValueSet& NamedValueSet::operator= (const NamedValueSet& other)
 {
     clear();
-    values.addCopyOfList (other.values);
+    values = other.values;
     return *this;
 }
 
 #if JUCE_COMPILER_SUPPORTS_MOVE_SEMANTICS
 NamedValueSet::NamedValueSet (NamedValueSet&& other) noexcept
-    : values (static_cast <LinkedListPointer<NamedValue>&&> (other.values))
+    : values (static_cast <Array<NamedValue>&&> (other.values))
 {
 }
 
@@ -111,24 +95,12 @@ NamedValueSet::~NamedValueSet()
 
 void NamedValueSet::clear()
 {
-    values.deleteAll();
+    values.clear();
 }
 
 bool NamedValueSet::operator== (const NamedValueSet& other) const
 {
-    const NamedValue* i1 = values;
-    const NamedValue* i2 = other.values;
-
-    while (i1 != nullptr && i2 != nullptr)
-    {
-        if (! (*i1 == *i2))
-            return false;
-
-        i1 = i1->nextListItem;
-        i2 = i2->nextListItem;
-    }
-
-    return true;
+    return values == other.values;
 }
 
 bool NamedValueSet::operator!= (const NamedValueSet& other) const
@@ -141,16 +113,15 @@ int NamedValueSet::size() const noexcept
     return values.size();
 }
 
-const var& NamedValueSet::operator[] (Identifier name) const
+const var& NamedValueSet::operator[] (const Identifier& name) const
 {
-    for (NamedValue* i = values; i != nullptr; i = i->nextListItem)
-        if (i->name == name)
-            return i->value;
+    if (const var* v = getVarPointer (name))
+        return *v;
 
     return var::null;
 }
 
-var NamedValueSet::getWithDefault (Identifier name, const var& defaultReturnValue) const
+var NamedValueSet::getWithDefault (const Identifier& name, const var& defaultReturnValue) const
 {
     if (const var* const v = getVarPointer (name))
         return *v;
@@ -158,9 +129,9 @@ var NamedValueSet::getWithDefault (Identifier name, const var& defaultReturnValu
     return defaultReturnValue;
 }
 
-var* NamedValueSet::getVarPointer (Identifier name) const noexcept
+var* NamedValueSet::getVarPointer (const Identifier& name) const noexcept
 {
-    for (NamedValue* i = values; i != nullptr; i = i->nextListItem)
+    for (NamedValue* e = values.end(), *i = values.begin(); i != e; ++i)
         if (i->name == name)
             return &(i->value);
 
@@ -170,145 +141,121 @@ var* NamedValueSet::getVarPointer (Identifier name) const noexcept
 #if JUCE_COMPILER_SUPPORTS_MOVE_SEMANTICS
 bool NamedValueSet::set (Identifier name, var&& newValue)
 {
-    LinkedListPointer<NamedValue>* i = &values;
-
-    while (i->get() != nullptr)
+    if (var* const v = getVarPointer (name))
     {
-        NamedValue* const v = i->get();
-
-        if (v->name == name)
-        {
-            if (v->value.equalsWithSameType (newValue))
-                return false;
-
-            v->value = static_cast <var&&> (newValue);
-            return true;
-        }
+        if (v->equalsWithSameType (newValue))
+            return false;
 
-        i = &(v->nextListItem);
+        *v = static_cast<var&&> (newValue);
+        return true;
     }
 
-    i->insertNext (new NamedValue (name, static_cast <var&&> (newValue)));
+    values.add (NamedValue (name, static_cast<var&&> (newValue)));
     return true;
 }
 #endif
 
 bool NamedValueSet::set (Identifier name, const var& newValue)
 {
-    LinkedListPointer<NamedValue>* i = &values;
-
-    while (i->get() != nullptr)
+    if (var* const v = getVarPointer (name))
     {
-        NamedValue* const v = i->get();
-
-        if (v->name == name)
-        {
-            if (v->value.equalsWithSameType (newValue))
-                return false;
-
-            v->value = newValue;
-            return true;
-        }
+        if (v->equalsWithSameType (newValue))
+            return false;
 
-        i = &(v->nextListItem);
+        *v = newValue;
+        return true;
     }
 
-    i->insertNext (new NamedValue (name, newValue));
+    values.add (NamedValue (name, newValue));
     return true;
 }
 
-bool NamedValueSet::contains (Identifier name) const
+bool NamedValueSet::contains (const Identifier& name) const
 {
     return getVarPointer (name) != nullptr;
 }
 
-int NamedValueSet::indexOf (Identifier name) const noexcept
+int NamedValueSet::indexOf (const Identifier& name) const noexcept
 {
-    int index = 0;
-
-    for (NamedValue* i = values; i != nullptr; i = i->nextListItem)
-    {
-        if (i->name == name)
-            return index;
+    const int numValues = values.size();
 
-        ++index;
-    }
+    for (int i = 0; i < numValues; ++i)
+        if (values.getReference(i).name == name)
+            return i;
 
     return -1;
 }
 
-bool NamedValueSet::remove (Identifier name)
+bool NamedValueSet::remove (const Identifier& name)
 {
-    LinkedListPointer<NamedValue>* i = &values;
+    const int numValues = values.size();
 
-    for (;;)
+    for (int i = 0; i < numValues; ++i)
     {
-        NamedValue* const v = i->get();
-
-        if (v == nullptr)
-            break;
-
-        if (v->name == name)
+        if (values.getReference(i).name == name)
         {
-            delete i->removeNext();
+            values.remove (i);
             return true;
         }
-
-        i = &(v->nextListItem);
     }
 
     return false;
 }
 
-Identifier NamedValueSet::getName (const int index) const
+Identifier NamedValueSet::getName (const int index) const noexcept
 {
-    const NamedValue* const v = values[index];
-    jassert (v != nullptr);
-    return v->name;
+    if (isPositiveAndBelow (index, values.size()))
+        return values.getReference (index).name;
+
+    jassertfalse;
+    return Identifier();
 }
 
-const var& NamedValueSet::getValueAt (const int index) const
+const var& NamedValueSet::getValueAt (const int index) const noexcept
 {
-    const NamedValue* const v = values[index];
-    jassert (v != nullptr);
-    return v->value;
+    if (isPositiveAndBelow (index, values.size()))
+        return values.getReference (index).value;
+
+    jassertfalse;
+    return var::null;
 }
 
-void NamedValueSet::setFromXmlAttributes (const XmlElement& xml)
+var* NamedValueSet::getVarPointerAt (int index) const noexcept
 {
-    clear();
-    LinkedListPointer<NamedValue>::Appender appender (values);
+    if (isPositiveAndBelow (index, values.size()))
+        return &(values.getReference (index).value);
+
+    return nullptr;
+}
 
-    const int numAtts = xml.getNumAttributes(); // xxx inefficient - should write an att iterator..
+void NamedValueSet::setFromXmlAttributes (const XmlElement& xml)
+{
+    values.clearQuick();
 
-    for (int i = 0; i < numAtts; ++i)
+    for (const XmlElement::XmlAttributeNode* att = xml.attributes; att != nullptr; att = att->nextListItem)
     {
-        const String& name  = xml.getAttributeName (i);
-        const String& value = xml.getAttributeValue (i);
-
-        if (name.startsWith ("base64:"))
+        if (att->name.toString().startsWith ("base64:"))
         {
             MemoryBlock mb;
 
-            if (mb.fromBase64Encoding (value))
+            if (mb.fromBase64Encoding (att->value))
             {
-                appender.append (new NamedValue (name.substring (7), var (mb)));
+                values.add (NamedValue (att->name.toString().substring (7), var (mb)));
                 continue;
             }
         }
 
-        appender.append (new NamedValue (name, var (value)));
+        values.add (NamedValue (att->name, var (att->value)));
     }
 }
 
 void NamedValueSet::copyToXmlAttributes (XmlElement& xml) const
 {
-    for (NamedValue* i = values; i != nullptr; i = i->nextListItem)
+    for (NamedValue* e = values.end(), *i = values.begin(); i != e; ++i)
     {
         if (const MemoryBlock* mb = i->value.getBinaryData())
         {
-            xml.setAttribute ("base64:" + i->name.toString(),
-                              mb->toBase64Encoding());
+            xml.setAttribute ("base64:" + i->name.toString(), mb->toBase64Encoding());
         }
         else
         {

modules/juce_core/containers/juce_NamedValueSet.h

@@ -67,12 +67,12 @@ public:
         If the name isn't found, this will return a void variant.
         @see getProperty
     */
-    const var& operator[] (Identifier name) const;
+    const var& operator[] (const Identifier& name) const;
 
     /** Tries to return the named value, but if no such value is found, this will
         instead return the supplied default value.
     */
-    var getWithDefault (Identifier name, const var& defaultReturnValue) const;
+    var getWithDefault (const Identifier& name, const var& defaultReturnValue) const;
 
     /** Changes or adds a named value.
         @returns    true if a value was changed or added; false if the
@@ -89,39 +89,43 @@ public:
    #endif
 
     /** Returns true if the set contains an item with the specified name. */
-    bool contains (Identifier name) const;
+    bool contains (const Identifier& name) const;
 
     /** Removes a value from the set.
         @returns    true if a value was removed; false if there was no value
                     with the name that was given.
     */
-    bool remove (Identifier name);
+    bool remove (const Identifier& name);
 
     /** Returns the name of the value at a given index.
         The index must be between 0 and size() - 1.
     */
-    Identifier getName (int index) const;
+    Identifier getName (int index) const noexcept;
+
+    /** Returns a pointer to the var that holds a named value, or null if there is
+        no value with this name.
+
+        Do not use this method unless you really need access to the internal var object
+        for some reason - for normal reading and writing always prefer operator[]() and set().
+    */
+    var* getVarPointer (const Identifier& name) const noexcept;
 
     /** Returns the value of the item at a given index.
         The index must be between 0 and size() - 1.
     */
-    const var& getValueAt (int index) const;
+    const var& getValueAt (int index) const noexcept;
+
+    /** Returns the value of the item at a given index.
+        The index must be between 0 and size() - 1, or this will return a nullptr
+    */
+    var* getVarPointerAt (int index) const noexcept;
 
     /** Returns the index of the given name, or -1 if it's not found. */
-    int indexOf (Identifier name) const noexcept;
+    int indexOf (const Identifier& name) const noexcept;
 
     /** Removes all values. */
     void clear();
 
-    //==============================================================================
-    /** Returns a pointer to the var that holds a named value, or null if there is
-        no value with this name.
-
-        Do not use this method unless you really need access to the internal var object
-        for some reason - for normal reading and writing always prefer operator[]() and set().
-    */
-    var* getVarPointer (Identifier name) const noexcept;
-
     //==============================================================================
     /** Sets properties to the values of all of an XML element's attributes. */
     void setFromXmlAttributes (const XmlElement& xml);
@@ -133,32 +137,8 @@ public:
 
 private:
     //==============================================================================
-    class NamedValue
-    {
-    public:
-        NamedValue() noexcept;
-        NamedValue (const NamedValue&);
-        NamedValue (Identifier, const var&);
-        NamedValue& operator= (const NamedValue&);
-       #if JUCE_COMPILER_SUPPORTS_MOVE_SEMANTICS
-        NamedValue (NamedValue&&) noexcept;
-        NamedValue (Identifier, var&&);
-        NamedValue& operator= (NamedValue&&) noexcept;
-       #endif
-        bool operator== (const NamedValue&) const noexcept;
-
-        LinkedListPointer<NamedValue> nextListItem;
-        Identifier name;
-        var value;
-
-    private:
-        JUCE_LEAK_DETECTOR (NamedValue)
-    };
-
-    friend class LinkedListPointer<NamedValue>;
-    LinkedListPointer<NamedValue> values;
-
-    friend class DynamicObject;
+    struct NamedValue;
+    Array<NamedValue> values;
 };
 
 

modules/juce_core/javascript/juce_JSON.cpp

@@ -243,9 +243,9 @@ private:
                 if (r.failed())
                     return r;
 
-                const String propertyName (propertyNameVar.toString());
+                const Identifier propertyName (propertyNameVar.toString());
 
-                if (propertyName.isNotEmpty())
+                if (propertyName.isValid())
                 {
                     t = t.findEndOfWhitespace();
                     oldT = t;

modules/juce_core/text/juce_Identifier.cpp

@@ -26,21 +26,10 @@
   ==============================================================================
 */
 
-StringPool& Identifier::getPool()
-{
-    static StringPool pool;
-    return pool;
-}
+Identifier::Identifier() noexcept {}
+Identifier::~Identifier() noexcept {}
 
-Identifier::Identifier() noexcept
-    : name (nullptr)
-{
-}
-
-Identifier::Identifier (const Identifier& other) noexcept
-    : name (other.name)
-{
-}
+Identifier::Identifier (const Identifier& other) noexcept  : name (other.name) {}
 
 Identifier& Identifier::operator= (const Identifier other) noexcept
 {
@@ -49,20 +38,27 @@ Identifier& Identifier::operator= (const Identifier other) noexcept
 }
 
 Identifier::Identifier (const String& nm)
-    : name (Identifier::getPool().getPooledString (nm))
+    : name (StringPool::getGlobalPool().getPooledString (nm))
 {
+    /* An Identifier string must be suitable for use as a script variable or XML
+       attribute, so it can only contain this limited set of characters.. */
+    jassert (isValidIdentifier (toString()));
 }
 
-Identifier::Identifier (const char* const nm)
-    : name (Identifier::getPool().getPooledString (nm))
+Identifier::Identifier (const char* nm)
+    : name (StringPool::getGlobalPool().getPooledString (nm))
 {
     /* An Identifier string must be suitable for use as a script variable or XML
        attribute, so it can only contain this limited set of characters.. */
     jassert (isValidIdentifier (toString()));
 }
 
-Identifier::~Identifier()
+Identifier::Identifier (String::CharPointerType start, String::CharPointerType end)
+    : name (StringPool::getGlobalPool().getPooledString (start, end))
 {
+    /* An Identifier string must be suitable for use as a script variable or XML
+       attribute, so it can only contain this limited set of characters.. */
+    jassert (isValidIdentifier (toString()));
 }
 
 Identifier Identifier::null;

modules/juce_core/text/juce_Identifier.h

@@ -34,9 +34,9 @@
 /**
     Represents a string identifier, designed for accessing properties by name.
 
-    Identifier objects are very light and fast to copy, but slower to initialise
-    from a string, so it's much faster to keep a static identifier object to refer
-    to frequently-used names, rather than constructing them each time you need it.
+    Comparing two Identifier objects is very fast (an O(1) operation), but creating
+    them can be slower than just using a String directly, so the optimal way to use them
+    is to keep some static Identifier objects for the things you use often.
 
     @see NamedValueSet, ValueTree
 */
@@ -58,6 +58,12 @@ public:
     */
     Identifier (const String& name);
 
+    /** Creates an identifier with a specified name.
+        Because this name may need to be used in contexts such as script variables or XML
+        tags, it must only contain ascii letters and digits, or the underscore character.
+    */
+    Identifier (String::CharPointerType nameStart, String::CharPointerType nameEnd);
+
     /** Creates a copy of another identifier. */
     Identifier (const Identifier& other) noexcept;
 
@@ -65,37 +71,37 @@ public:
     Identifier& operator= (const Identifier other) noexcept;
 
     /** Destructor */
-    ~Identifier();
+    ~Identifier() noexcept;
 
     /** Compares two identifiers. This is a very fast operation. */
-    inline bool operator== (Identifier other) const noexcept            { return name == other.name; }
+    inline bool operator== (Identifier other) const noexcept            { return name.getCharPointer() == other.name.getCharPointer(); }
 
     /** Compares two identifiers. This is a very fast operation. */
-    inline bool operator!= (Identifier other) const noexcept            { return name != other.name; }
+    inline bool operator!= (Identifier other) const noexcept            { return name.getCharPointer() != other.name.getCharPointer(); }
 
     /** Compares the identifier with a string. */
-    inline bool operator== (StringRef other) const noexcept             { return name.compare (other.text) == 0; }
+    inline bool operator== (StringRef other) const noexcept             { return name == other; }
 
     /** Compares the identifier with a string. */
-    inline bool operator!= (StringRef other) const noexcept             { return name.compare (other.text) != 0; }
+    inline bool operator!= (StringRef other) const noexcept             { return name != other; }
 
     /** Returns this identifier as a string. */
-    String toString() const                                             { return name; }
+    const String& toString() const noexcept                             { return name; }
 
     /** Returns this identifier's raw string pointer. */
-    operator String::CharPointerType() const noexcept                   { return name; }
+    operator String::CharPointerType() const noexcept                   { return name.getCharPointer(); }
 
     /** Returns this identifier's raw string pointer. */
-    String::CharPointerType getCharPointer() const noexcept             { return name; }
+    String::CharPointerType getCharPointer() const noexcept             { return name.getCharPointer(); }
 
     /** Returns this identifier as a StringRef. */
     operator StringRef() const noexcept                                 { return name; }
 
     /** Returns true if this Identifier is not null */
-    bool isValid() const noexcept                                       { return name.getAddress() != nullptr; }
+    bool isValid() const noexcept                                       { return name.isNotEmpty(); }
 
     /** Returns true if this Identifier is null */
-    bool isNull() const noexcept                                        { return name.getAddress() == nullptr; }
+    bool isNull() const noexcept                                        { return name.isEmpty(); }
 
     /** A null identifier. */
     static Identifier null;
@@ -106,12 +112,8 @@ public:
     */
     static bool isValidIdentifier (const String& possibleIdentifier) noexcept;
 
-
 private:
-    //==============================================================================
-    String::CharPointerType name;
-
-    static StringPool& getPool();
+    String name;
 };
 
 

modules/juce_core/text/juce_String.cpp

@@ -180,6 +180,11 @@ public:
         release (bufferFromText (text));
     }
 
+    static inline int getReferenceCount (const CharPointerType text) noexcept
+    {
+        return bufferFromText (text)->refCount.get() + 1;
+    }
+
     //==============================================================================
     static CharPointerType makeUniqueWithByteSize (const CharPointerType text, size_t numBytes)
     {
@@ -285,7 +290,7 @@ String& String::operator= (String&& other) noexcept
 }
 #endif
 
-inline String::PreallocationBytes::PreallocationBytes (const size_t numBytes_) : numBytes (numBytes_) {}
+inline String::PreallocationBytes::PreallocationBytes (const size_t num) noexcept : numBytes (num) {}
 
 String::String (const PreallocationBytes& preallocationSize)
     : text (StringHolder::createUninitialisedBytes (preallocationSize.numBytes + sizeof (CharPointerType::CharType)))
@@ -297,6 +302,11 @@ void String::preallocateBytes (const size_t numBytesNeeded)
     text = StringHolder::makeUniqueWithByteSize (text, numBytesNeeded + sizeof (CharPointerType::CharType));
 }
 
+int String::getReferenceCount() const noexcept
+{
+    return StringHolder::getReferenceCount (text);
+}
+
 //==============================================================================
 String::String (const char* const t)
     : text (StringHolder::createFromCharPointer (CharPointer_ASCII (t)))

modules/juce_core/text/juce_String.h

@@ -1218,6 +1218,11 @@ public:
     String convertToPrecomposedUnicode() const;
    #endif
 
+    /** Returns the number of String objects which are currently sharing the same internal
+        data as this one.
+    */
+    int getReferenceCount() const noexcept;
+
 private:
     //==============================================================================
     CharPointerType text;
@@ -1225,7 +1230,7 @@ private:
     //==============================================================================
     struct PreallocationBytes
     {
-        explicit PreallocationBytes (size_t);
+        explicit PreallocationBytes (size_t) noexcept;
         size_t numBytes;
     };
 

modules/juce_core/text/juce_StringPool.cpp

@@ -26,88 +26,131 @@
   ==============================================================================
 */
 
-StringPool::StringPool() noexcept   {}
-StringPool::~StringPool()           {}
+static const uint32 minNumberOfStringsForGarbageCollection = 300;
+static const uint32 garbageCollectionInterval = 30000;
 
-namespace StringPoolHelpers
+
+StringPool::StringPool() noexcept  : lastGarbageCollectionTime (0) {}
+StringPool::~StringPool() {}
+
+struct StartEndString
 {
-    template <class StringType>
-    String::CharPointerType getPooledStringFromArray (Array<String>& strings,
-                                                      StringType newString,
-                                                      const CriticalSection& lock)
-    {
-        const ScopedLock sl (lock);
-        int start = 0;
-        int end = strings.size();
+    StartEndString (String::CharPointerType s, String::CharPointerType e) noexcept : start (s), end (e) {}
+    operator String() const   { return String (start, end); }
 
-        for (;;)
-        {
-            if (start >= end)
-            {
-                jassert (start <= end);
-                strings.insert (start, newString);
-                return strings.getReference (start).getCharPointer();
-            }
+    String::CharPointerType start, end;
+};
+
+static int compareStrings (const String& s1, const String& s2) noexcept     { return s1.compare (s2); }
+static int compareStrings (CharPointer_UTF8 s1, const String& s2) noexcept  { return s1.compare (s2.toUTF8()); }
+
+static int compareStrings (const StartEndString& string1, const String& string2) noexcept
+{
+    String::CharPointerType s1 (string1.start), s2 (string2.getCharPointer());
 
-            const String& startString = strings.getReference (start);
+    for (;;)
+    {
+        const int c1 = s1 < string1.end ? (int) s1.getAndAdvance() : 0;
+        const int c2 = (int) s2.getAndAdvance();
+        const int diff = c1 - c2;
 
-            if (startString == newString)
-                return startString.getCharPointer();
+        if (diff != 0)  return diff < 0 ? -1 : 1;
+        if (c1 == 0)    break;
+    }
 
-            const int halfway = (start + end) >> 1;
+    return 0;
+}
 
-            if (halfway == start)
-            {
-                if (startString.compare (newString) < 0)
-                    ++start;
+template <typename NewStringType>
+static String addPooledString (Array<String>& strings, const NewStringType& newString)
+{
+    int start = 0;
+    int end = strings.size();
 
-                strings.insert (start, newString);
-                return strings.getReference (start).getCharPointer();
-            }
+    while (start < end)
+    {
+        const String& startString = strings.getReference (start);
+        const int startComp = compareStrings (newString, startString);
 
-            const int comp = strings.getReference (halfway).compare (newString);
+        if (startComp == 0)
+            return startString;
 
-            if (comp == 0)
-                return strings.getReference (halfway).getCharPointer();
+        const int halfway = (start + end) / 2;
 
-            if (comp < 0)
-                start = halfway;
-            else
-                end = halfway;
+        if (halfway == start)
+        {
+            if (startComp > 0)
+                ++start;
+
+            break;
         }
+
+        const String& halfwayString = strings.getReference (halfway);
+        const int halfwayComp = compareStrings (newString, halfwayString);
+
+        if (halfwayComp == 0)
+            return halfwayString;
+
+        if (halfwayComp > 0)
+            start = halfway;
+        else
+            end = halfway;
     }
+
+    strings.insert (start, newString);
+    return strings.getReference (start);
 }
 
-String::CharPointerType StringPool::getPooledString (const String& s)
+String StringPool::getPooledString (const char* const newString)
 {
-    if (s.isEmpty())
-        return String().getCharPointer();
+    if (newString == nullptr || *newString == 0)
+        return String();
 
-    return StringPoolHelpers::getPooledStringFromArray (strings, s, lock);
+    const ScopedLock sl (lock);
+    garbageCollectIfNeeded();
+    return addPooledString (strings, CharPointer_UTF8 (newString));
 }
 
-String::CharPointerType StringPool::getPooledString (const char* const s)
+String StringPool::getPooledString (String::CharPointerType start, String::CharPointerType end)
 {
-    if (s == nullptr || *s == 0)
-        return String().getCharPointer();
+    if (start.isEmpty() || start == end)
+        return String();
 
-    return StringPoolHelpers::getPooledStringFromArray (strings, s, lock);
+    const ScopedLock sl (lock);
+    garbageCollectIfNeeded();
+    return addPooledString (strings, StartEndString (start, end));
 }
 
-String::CharPointerType StringPool::getPooledString (const wchar_t* const s)
+String StringPool::getPooledString (const String& newString)
 {
-    if (s == nullptr || *s == 0)
-        return String().getCharPointer();
+    if (newString.isEmpty())
+        return String();
 
-    return StringPoolHelpers::getPooledStringFromArray (strings, s, lock);
+    const ScopedLock sl (lock);
+    garbageCollectIfNeeded();
+    return addPooledString (strings, newString);
 }
 
-int StringPool::size() const noexcept
+void StringPool::garbageCollectIfNeeded()
 {
-    return strings.size();
+    if (strings.size() > minNumberOfStringsForGarbageCollection
+         && Time::getApproximateMillisecondCounter() > lastGarbageCollectionTime + garbageCollectionInterval)
+        garbageCollect();
+}
+
+void StringPool::garbageCollect()
+{
+    const ScopedLock sl (lock);
+
+    for (int i = strings.size(); --i >= 0;)
+        if (strings.getReference(i).getReferenceCount() == 1)
+            strings.remove (i);
+
+    lastGarbageCollectionTime = Time::getApproximateMillisecondCounter();
 }
 
-String::CharPointerType StringPool::operator[] (const int index) const noexcept
+StringPool& StringPool::getGlobalPool() noexcept
 {
-    return strings [index].getCharPointer();
+    static StringPool pool;
+    return pool;
 }

modules/juce_core/text/juce_StringPool.h

@@ -52,40 +52,37 @@ public:
     ~StringPool();
 
     //==============================================================================
-    /** Returns a pointer to a copy of the string that is passed in.
-
-        The pool will always return the same pointer when asked for a string that matches it.
-        The pool will own all the pointers that it returns, deleting them when the pool itself
-        is deleted.
+    /** Returns a pointer to a shared copy of the string that is passed in.
+        The pool will always return the same String object when asked for a string that matches it.
     */
-    String::CharPointerType getPooledString (const String& original);
+    String getPooledString (const String& original);
 
     /** Returns a pointer to a copy of the string that is passed in.
-
-        The pool will always return the same pointer when asked for a string that matches it.
-        The pool will own all the pointers that it returns, deleting them when the pool itself
-        is deleted.
+        The pool will always return the same String object when asked for a string that matches it.
     */
-    String::CharPointerType getPooledString (const char* original);
+    String getPooledString (const char* original);
 
     /** Returns a pointer to a copy of the string that is passed in.
-
-        The pool will always return the same pointer when asked for a string that matches it.
-        The pool will own all the pointers that it returns, deleting them when the pool itself
-        is deleted.
+        The pool will always return the same String object when asked for a string that matches it.
     */
-    String::CharPointerType getPooledString (const wchar_t* original);
+    String getPooledString (String::CharPointerType start, String::CharPointerType end);
 
     //==============================================================================
-    /** Returns the number of strings in the pool. */
-    int size() const noexcept;
+    /** Scans the pool, and removes any strings that are unreferenced.
+        You don't generally need to call this - it'll be called automatically when the pool grows
+        large enough to warrant it.
+    */
+    void garbageCollect();
 
-    /** Returns one of the strings in the pool, by index. */
-    String::CharPointerType operator[] (int index) const noexcept;
+    /** Returns a shared global pool which is used for things like Identifiers, XML parsing. */
+    static StringPool& getGlobalPool() noexcept;
 
 private:
-    Array <String> strings;
+    Array<String> strings;
     CriticalSection lock;
+    uint32 lastGarbageCollectionTime;
+
+    void garbageCollectIfNeeded();
 };
 
 

modules/juce_core/xml/juce_XmlDocument.cpp

@@ -408,7 +408,7 @@ XmlElement* XmlDocument::readNextElement (const bool alsoParseSubElements)
             }
         }
 
-        node = new XmlElement (String (input, endOfToken));
+        node = new XmlElement (input, endOfToken);
         input = endOfToken;
         LinkedListPointer<XmlElement::XmlAttributeNode>::Appender attributeAppender (node->attributes);
 
@@ -458,8 +458,7 @@ XmlElement* XmlDocument::readNextElement (const bool alsoParseSubElements)
                         if (nextChar == '"' || nextChar == '\'')
                         {
                             XmlElement::XmlAttributeNode* const newAtt
-                                = new XmlElement::XmlAttributeNode (String (attNameStart, attNameEnd),
-                                                                    String::empty);
+                                = new XmlElement::XmlAttributeNode (attNameStart, attNameEnd);
 
                             readQuotedString (newAtt->value);
                             attributeAppender.append (newAtt);

modules/juce_core/xml/juce_XmlElement.cpp

@@ -32,7 +32,7 @@ XmlElement::XmlAttributeNode::XmlAttributeNode (const XmlAttributeNode& other) n
 {
 }
 
-XmlElement::XmlAttributeNode::XmlAttributeNode (const String& n, const String& v) noexcept
+XmlElement::XmlAttributeNode::XmlAttributeNode (const Identifier& n, const String& v) noexcept
     : name (n), value (v)
 {
    #if JUCE_DEBUG
@@ -42,15 +42,16 @@ XmlElement::XmlAttributeNode::XmlAttributeNode (const String& n, const String& v
    #endif
 }
 
-bool XmlElement::XmlAttributeNode::hasName (StringRef nameToMatch) const noexcept
+XmlElement::XmlAttributeNode::XmlAttributeNode (String::CharPointerType nameStart, String::CharPointerType nameEnd)
+    : name (nameStart, nameEnd)
 {
-    return name.equalsIgnoreCase (nameToMatch);
 }
 
 //==============================================================================
-XmlElement::XmlElement (const String& tag) noexcept
-    : tagName (tag)
+static void sanityCheckTagName (const String& tag)
 {
+    (void) tag;
+
     // the tag name mustn't be empty, or it'll look like a text element!
     jassert (tag.containsNonWhitespaceChars())
 
@@ -58,6 +59,30 @@ XmlElement::XmlElement (const String& tag) noexcept
     jassert (! tag.containsAnyOf (" <>/&(){}"));
 }
 
+XmlElement::XmlElement (const String& tag)
+    : tagName (StringPool::getGlobalPool().getPooledString (tag))
+{
+    sanityCheckTagName (tagName);
+}
+
+XmlElement::XmlElement (const char* tag)
+    : tagName (StringPool::getGlobalPool().getPooledString (tag))
+{
+    sanityCheckTagName (tagName);
+}
+
+XmlElement::XmlElement (const Identifier& tag)
+    : tagName (tag.toString())
+{
+    sanityCheckTagName (tagName);
+}
+
+XmlElement::XmlElement (String::CharPointerType tagNameStart, String::CharPointerType tagNameEnd)
+    : tagName (StringPool::getGlobalPool().getPooledString (tagNameStart, tagNameEnd))
+{
+    sanityCheckTagName (tagName);
+}
+
 XmlElement::XmlElement (int /*dummy*/) noexcept
 {
 }
@@ -407,7 +432,7 @@ int XmlElement::getNumAttributes() const noexcept
 const String& XmlElement::getAttributeName (const int index) const noexcept
 {
     if (const XmlAttributeNode* const att = attributes [index])
-        return att->name;
+        return att->name.toString();
 
     return String::empty;
 }
@@ -423,7 +448,7 @@ const String& XmlElement::getAttributeValue (const int index) const noexcept
 XmlElement::XmlAttributeNode* XmlElement::getAttribute (StringRef attributeName) const noexcept
 {
     for (XmlAttributeNode* att = attributes; att != nullptr; att = att->nextListItem)
-        if (att->hasName (attributeName))
+        if (att->name == attributeName)
             return att;
 
     return nullptr;
@@ -495,7 +520,7 @@ bool XmlElement::compareAttribute (StringRef attributeName,
 }
 
 //==============================================================================
-void XmlElement::setAttribute (const String& attributeName, const String& value)
+void XmlElement::setAttribute (const Identifier& attributeName, const String& value)
 {
     if (attributes == nullptr)
     {
@@ -505,7 +530,7 @@ void XmlElement::setAttribute (const String& attributeName, const String& value)
     {
         for (XmlAttributeNode* att = attributes; ; att = att->nextListItem)
         {
-            if (att->hasName (attributeName))
+            if (att->name == attributeName)
             {
                 att->value = value;
                 break;
@@ -520,23 +545,23 @@ void XmlElement::setAttribute (const String& attributeName, const String& value)
     }
 }
 
-void XmlElement::setAttribute (const String& attributeName, const int number)
+void XmlElement::setAttribute (const Identifier& attributeName, const int number)
 {
     setAttribute (attributeName, String (number));
 }
 
-void XmlElement::setAttribute (const String& attributeName, const double number)
+void XmlElement::setAttribute (const Identifier& attributeName, const double number)
 {
     setAttribute (attributeName, String (number, 20));
 }
 
-void XmlElement::removeAttribute (const String& attributeName) noexcept
+void XmlElement::removeAttribute (const Identifier& attributeName) noexcept
 {
     for (LinkedListPointer<XmlAttributeNode>* att = &attributes;
          att->get() != nullptr;
          att = &(att->get()->nextListItem))
     {
-        if (att->get()->hasName (attributeName))
+        if (att->get()->name == attributeName)
         {
             delete att->removeNext();
             break;
@@ -615,7 +640,7 @@ void XmlElement::prependChildElement (XmlElement* newNode) noexcept
     }
 }
 
-XmlElement* XmlElement::createNewChildElement (const String& childTagName)
+XmlElement* XmlElement::createNewChildElement (StringRef childTagName)
 {
     XmlElement* const newElement = new XmlElement (childTagName);
     addChildElement (newElement);

modules/juce_core/xml/juce_XmlElement.h

@@ -144,20 +144,29 @@ class JUCE_API  XmlElement
 public:
     //==============================================================================
     /** Creates an XmlElement with this tag name. */
-    explicit XmlElement (const String& tagName) noexcept;
+    explicit XmlElement (const String& tagName);
+
+    /** Creates an XmlElement with this tag name. */
+    explicit XmlElement (const char* tagName);
+
+    /** Creates an XmlElement with this tag name. */
+    explicit XmlElement (const Identifier& tagName);
+
+    /** Creates an XmlElement with this tag name. */
+    XmlElement (String::CharPointerType tagNameBegin, String::CharPointerType tagNameEnd);
 
     /** Creates a (deep) copy of another element. */
-    XmlElement (const XmlElement& other);
+    XmlElement (const XmlElement&);
 
     /** Creates a (deep) copy of another element. */
-    XmlElement& operator= (const XmlElement& other);
+    XmlElement& operator= (const XmlElement&);
 
    #if JUCE_COMPILER_SUPPORTS_MOVE_SEMANTICS
-    XmlElement (XmlElement&& other) noexcept;
-    XmlElement& operator= (XmlElement&& other) noexcept;
+    XmlElement (XmlElement&&) noexcept;
+    XmlElement& operator= (XmlElement&&) noexcept;
    #endif
 
-    /** Deleting an XmlElement will also delete all its child elements. */
+    /** Deleting an XmlElement will also delete all of its child elements. */
     ~XmlElement() noexcept;
 
     //==============================================================================
@@ -303,13 +312,11 @@ public:
     bool hasAttribute (StringRef attributeName) const noexcept;
 
     /** Returns the value of a named attribute.
-
         @param attributeName        the name of the attribute to look up
     */
     const String& getStringAttribute (StringRef attributeName) const noexcept;
 
     /** Returns the value of a named attribute.
-
         @param attributeName        the name of the attribute to look up
         @param defaultReturnValue   a value to return if the element doesn't have an attribute
                                     with this name
@@ -377,7 +384,7 @@ public:
         @param newValue             the value to set it to
         @see removeAttribute
     */
-    void setAttribute (const String& attributeName, const String& newValue);
+    void setAttribute (const Identifier& attributeName, const String& newValue);
 
     /** Adds a named attribute to the element, setting it to an integer value.
 
@@ -391,7 +398,7 @@ public:
         @param attributeName        the name of the attribute to set
         @param newValue             the value to set it to
     */
-    void setAttribute (const String& attributeName, int newValue);
+    void setAttribute (const Identifier& attributeName, int newValue);
 
     /** Adds a named attribute to the element, setting it to a floating-point value.
 
@@ -405,14 +412,14 @@ public:
         @param attributeName        the name of the attribute to set
         @param newValue             the value to set it to
     */
-    void setAttribute (const String& attributeName, double newValue);
+    void setAttribute (const Identifier& attributeName, double newValue);
 
     /** Removes a named attribute from the element.
 
         @param attributeName    the name of the attribute to remove
         @see removeAllAttributes
     */
-    void removeAttribute (const String& attributeName) noexcept;
+    void removeAttribute (const Identifier& attributeName) noexcept;
 
     /** Removes all attributes from this element. */
     void removeAllAttributes() noexcept;
@@ -555,7 +562,7 @@ public:
         XmlElement* newElement = myParentElement->createNewChildElement ("foobar");
         @endcode
     */
-    XmlElement* createNewChildElement (const String& tagName);
+    XmlElement* createNewChildElement (StringRef tagName);
 
     /** Replaces one of this element's children with another node.
 
@@ -712,25 +719,26 @@ private:
     struct XmlAttributeNode
     {
         XmlAttributeNode (const XmlAttributeNode&) noexcept;
-        XmlAttributeNode (const String& name, const String& value) noexcept;
+        XmlAttributeNode (const Identifier&, const String&) noexcept;
+        XmlAttributeNode (String::CharPointerType, String::CharPointerType);
 
         LinkedListPointer<XmlAttributeNode> nextListItem;
-        String name, value;
-
-        bool hasName (StringRef) const noexcept;
+        Identifier name;
+        String value;
 
     private:
-        XmlAttributeNode& operator= (const XmlAttributeNode&);
+        XmlAttributeNode& operator= (const XmlAttributeNode&) JUCE_DELETED_FUNCTION;
     };
 
     friend class XmlDocument;
-    friend class LinkedListPointer <XmlAttributeNode>;
-    friend class LinkedListPointer <XmlElement>;
-    friend class LinkedListPointer <XmlElement>::Appender;
-
-    LinkedListPointer <XmlElement> nextListItem;
-    LinkedListPointer <XmlElement> firstChildElement;
-    LinkedListPointer <XmlAttributeNode> attributes;
+    friend class LinkedListPointer<XmlAttributeNode>;
+    friend class LinkedListPointer<XmlElement>;
+    friend class LinkedListPointer<XmlElement>::Appender;
+    friend class NamedValueSet;
+
+    LinkedListPointer<XmlElement> nextListItem;
+    LinkedListPointer<XmlElement> firstChildElement;
+    LinkedListPointer<XmlAttributeNode> attributes;
     String tagName;
 
     XmlElement (int) noexcept;
@@ -740,6 +748,11 @@ private:
     void reorderChildElements (XmlElement**, int) noexcept;
     XmlAttributeNode* getAttribute (StringRef) const noexcept;
 
+    // Sigh.. L"" or _T("") string literals are problematic in general, and really inappropriate
+    // for XML tags. Use a UTF-8 encoded literal instead, or if you're really determined to use
+    // UTF-16, cast it to a String and use the other constructor.
+    XmlElement (const wchar_t*) JUCE_DELETED_FUNCTION;
+
     JUCE_LEAK_DETECTOR (XmlElement)
 };
 

modules/juce_data_structures/values/juce_ValueTree.cpp

@@ -409,7 +409,7 @@ public:
 
     XmlElement* createXml() const
     {
-        XmlElement* const xml = new XmlElement (type.toString());
+        XmlElement* const xml = new XmlElement (type);
         properties.copyToXmlAttributes (*xml);
 
         // (NB: it's faster to add nodes to XML elements in reverse order)