Using UserDefault is pretty inefficient

Cocos version this topic is relevant for: 3.15.1

auto ud = UserDefault::getInstance();

ud->setIntegerForKey("key1", ud->getIntegerForKey("key1") + 1);
ud->setIntegerForKey("key2", ud->getIntegerForKey("key2") + 1);
ud->setIntegerForKey("key3", ud->getIntegerForKey("key3") + 1);

ud->flush(); // irrelevant

This piece of code parses the xml file, then retrieves the value for key1 and then destroys the xml document in memory. Now we add 1 to the returned value, setIntegerForKey again parses the xml file, adds or modifies the value for the key key1, writes from memory to disk and then destroys the xml object in memory. Now we do the same 2 more times.

Overall the entire xml with all its elements is parsed and built in the memory 6 times, destroyed 6 times, and written to the disk 3 times.

With keeping the xml representation (could be ValueMap) persistantly in memory, all this can be easily done with just one write to the disk (after calling a proper flush, which is now defined empty in most platforms). The underlying logic is already there, implemented in Cocos, with writeValueMapToFile and getValueMapFromFile.

I have already solved this for my own project similar as to how I’ve described above, but there must be thousands who have their games slowed down by constantly doing unnecessary calls to the OS, doing expensive parses and whatnot, even to read data not even to write.

Now I’ve discovered that steps have been to heal this thing, in CCUserDefault-android.cpp for example there is code already that hooks into java code which is supposedly much more efficient; in iOS code similarly. However these are not called because a macro is defined: KEEP_COMPATABILITY. Can a dev tell us about what it means please and when its removal is due?

1 Like

Why don’t you write I/O in a different thread and you just capture events sent through the game?

Using a different thread should never impact in performance as it will use low use of cpu to perform it.

class UserGameData {
public:
    int retrieve(string key); //template this
    void increment(string key);
};

class UserGameDataManager { //Singleton
    UserGameDataManager() {
        this->getEventDispatcher()->addCustomEventListener("IncrementCoins", [=](EventCustom* event) {
              m_UserGameData->increment("coins");
        }
    }
};

Regardless, a ton of unnecessary work is done by the current implementation of UserDefault. (And the better implementation is there, at least for android and iOS, but its disabled by a macro for unknown reasons). In my own code I’ve already solved it by custom means, I just don’t want the bulk of the community to remain/walk in this trap, unaware.

1 Like

I checked the code in cocos 3.15.1 and I can’t confirm your findings. My understanding of the code is, that it will read from XML is the UserDefault.xml file is available, but if I write a new value, it will be deleted from the xml file and written with the platform implementation, so every change will move defaults away from UserDefault.xml.

Did I oversee something? Please tell me, if it is so.

PS: I didn’t debug, but my next step (if you say I’m wrong) will be to attach a debugger to test it.

cocos2d\cocos\base\CCUserDefault.cpp:
int UserDefault::getIntegerForKey(const char* pKey, int defaultValue)
{
const char* value = nullptr;
tinyxml2::XMLElement* rootNode;
tinyxml2::XMLDocument* doc;
tinyxml2::XMLElement* node;
node = getXMLNodeForKey(pKey, &rootNode, &doc);
// find the node
if (node && node->FirstChild())
{
value = (const char*)(node->FirstChild()->Value());
}

    int ret = defaultValue;

    if (value)
    {
        ret = atoi(value);
    }

    if(doc)
    {
        delete doc;
    }


    return ret;
}

This is the default implementation, but other platforms are essentially identical in effect currently as I have percieved.
5 lines into the function body it calls getXMLNodeForKey.

Its implementation:
static tinyxml2::XMLElement* getXMLNodeForKey(const char* pKey, tinyxml2::XMLElement** rootNode, tinyxml2::XMLDocument *doc)
{
tinyxml2::XMLElement
curNode = nullptr;

    // check the key value
    if (! pKey)
    {
        return nullptr;
    }

    do 
    {
         tinyxml2::XMLDocument* xmlDoc = new (std::nothrow) tinyxml2::XMLDocument();
        *doc = xmlDoc;

        std::string xmlBuffer = FileUtils::getInstance()->getStringFromFile(UserDefault::getInstance()->getXMLFilePath());

        if (xmlBuffer.empty())
        {
            CCLOG("can not read xml file");
            break;
        }
        xmlDoc->Parse(xmlBuffer.c_str(), xmlBuffer.size());

        // get root node
        *rootNode = xmlDoc->RootElement();
        if (nullptr == *rootNode)
        {
            CCLOG("read root node error");
            break;
        }
        // find the node
        curNode = (*rootNode)->FirstChildElement();
        while (nullptr != curNode)
        {
            const char* nodeName = curNode->Value();
            if (!strcmp(nodeName, pKey))
            {
                break;
            }

            curNode = curNode->NextSiblingElement();
        }
    } while (0);

    return curNode;
}

Any time you want to access or modify information through the UserDefault functions, getXMLNodeForKey is called, rebuilding the xml’s structure in memory.

1 Like

But it can’t create something, because the “setter” do use the platform implementation and delete any “legacy” entries in the xml. The XML will only be used, if it is from an old app, where you updated the cocos2d-x framework. So what’s the problem with new/updated apps? I can’t see any.

Are you referring to the condition compilation switch instigated by this macro?:

#define KEEP_COMPATABILITY

In the source files CCUserDefault-apple.mm and CCUserDefault-android.cpp? Because it seems to me it hard-codedly disables the more optimized, platform specific implementations.

Please show here. This is (for example) setBoolForKey within Android. As you can see, it will write new settings with the platform specific implementation and deletes the data within the XML before it.

So, it’s not so bad, as you describe it in your original post, because after some time the XML will be empty and the settings will be within the platform preferences.

deleteNodeByKey calls getXMLNodeForKey as well.

Sure, but the XML will shrink after new data is set. Don’t you understand, what I want to say?

Will it shrink or will it not, this implementation works awful for the type of logic I have written in my first post, doing way too much unnecessary work. Still my main curiosity is towards what the KEEP_COMPATIBILITY's macro purpose is, why it’s there and how long will it be there.

And if you undef it, it will not compile for Android, because it’s not everything within the ifdef block :confused: