18 réponses à ce sujet

[MagicalMaster]

Question about efficiency and operations within the toolset.  Let's say I wanted to run a certain operation 30 times and print off a message saying which time it was every time it occurred.  Let's also say we'll have it occur every second.  Which of the following is best for the toolset?

1, Use a psuedoheartbeat which runs every second.  Each times it runs it retrieves a local int, prints off the value, sets a new value for the local int, and repeats.  For example, the local int starts at 30 and is decreased by 1 each time until it hits 0.

2, Use a psuedoheartbeat which runs every second.  Pass an integer to it which is then passed to itself recursively that starts at 30 and counts down.  So keeping it local to the script instead of using a local variable.

3, something else entirely.

I suppose I'm mainly driving at the cost of retrieving and setting local variables as opposed to remembering variables that are being passed to functions and often being passed recursively.  Obviously if another script needs the information then I need the local variables, but otherwise?

[Shadooow]

MagicalMaster wrote...

1, Use a psuedoheartbeat which runs every second.  Each times it runs it retrieves a local int, prints off the value, sets a new value for the local int, and repeats.  For example, the local int starts at 30 and is decreased by 1 each time until it hits 0.

2, Use a psuedoheartbeat which runs every second.  Pass an integer to it which is then passed to itself recursively that starts at 30 and counts down.  So keeping it local to the script instead of using a local variable.

From these two, logically 2 is better, though I dont have any proofs for it. But it should be quite faster without any issue. Problem could be passing a struct data type, or maybe itemproperty, effect etc. Funky claimed in past that after certain repeats it might blow up memory.

Third way would be a delay command with a increased float in loop - I would say that this option is useable too, but only if you have one, max two commands you need to delay. So when you just want to shout a countdown, then its waste to use recursive function.

[MagicalMaster]

Yeah, I'm not talking about shouting a countdown, that was merely an example. Talking about something that might be running every second for 5+ minutes and checking several things -- meaning I'd probably need to be either passing 3+ variables per recursive call or retrieving 3+ variables per recursive call (along with doing other stuff if certain conditions are met).

Like I said, I simply don't know if the cost of manipulating local variables is significantly higher than passing variables.

[Lightfoot8]

I believe that you are splitting hairs either way. But if we are hair splitting, you can get away with just using the same var in the script without passing it or retrieving it at all. Just define your var outside of the main and before your SHB. This just stops the need to create a new local var every time the function runs. Here is a working example.

int nCounter= 30;

void CountDown()
{
   if (nCounter> 0 )
  {

     SpeakString( IntToString(nCounter--));
     DelayCommand(1.0,CountDown());
  }
}


void main()
{
    CountDown();
}

Modifié par Lightfoot8, 10 décembre 2013 - 06:54 .

[leo_x]

MagicalMaster wrote...

Like I said, I simply don't know if the cost of manipulating local variables is significantly higher than passing variables.

Well, internally the vartable is an unsorted array.  So all new variables are add to the end (and might cause a reallocation).  Right there you have worst case N string comparisons everytime you want to look it up.  If N is small probably not going to be much performance difference between the two.  You also have the variable string being copied/allocated on/off the stack.

When you call DelayCommand you're already committed to copying the NWScript stack... so if you're passing an int that means allocating/copying 4 more bytes each recursive call.

Whether there might be a better way... I dunno.

Modifié par pope_leo, 10 décembre 2013 - 07:49 .

[MagicalMaster]

Lightfoot8 wrote...

I believe that you are splitting hairs either way.

That's my hope but I'm trying to be sure.  I've been told a psuedo-heartbeat uses far more resources than a standard heartbeat, for example, so I didn't know if a similar rule applied abouut manipulating local variables versus passing them as arguments.

Good to know about the existence of global variables in NWN scripts -- any downsides to using those specific to NWN?

pope_leo wrote...

When you call DelayCommand you're already committed to copying the NWScript stack... so if you're passing an int that means allocating/copying 4 more bytes each recursive call.

True, but if you have to store like 5-10 variables for a significant amount of time which include ints, floats, strings, objects, and locations then I figured you might be better off not forcing the game to remember all of them and just retrieve them when the function is called again.

[FunkySwerve]

I know acaos prefers to pass the var, since it obviates retrieval, though I don't know if he put any more thought into it than Shad did above. It's only passing of struct-type info that can cause issues (like effects). Even then, it's just a mem leak, resulting in gradual slowdown, if memory serves. Either acaos or vman figured that one out.

Funky

[MagicalMaster]

FunkySwerve wrote...

It's only passing of struct-type info that can cause issues (like effects). Even then, it's just a mem leak, resulting in gradual slowdown, if memory serves.

So passing actual effects is a bad idea?  I don't think I've actually passed an effect recursively but good to know.  What else is struct-type info in NWN?  Locations?  Vectors?

Presumably it's safe to pass ints/floats/strings/objects (assuming the object acts like a pointer to the object in this case -- does it?)...but what's bad?

I realize that it definitely doesn't matter in most cases anyway but would prefer to avoid bad habits.

Modifié par MagicalMaster, 10 décembre 2013 - 09:21 .

[FunkySwerve]

The object type is just a pointer - at least, so I've always assumed, as the module is zero in most if not all cases. I do know that effects and item props are structs. Strings are safe, and ints and floats, so far as I know. Generally, you can avoid passing objects with a get local on the object running the delay, but I honestly don't know if that's worth the hassle - probably not.

Funky

[Lightfoot8]

@Funky: how old is the data tests for the memory leaks? I know that bioware fixed some of them in one of the updates. Are they still leaks?

[FunkySwerve]

Acaos found the issue after 1.69. Prior to that, we reported all the issues we (he, mostly) found to Bioware, and they generally fixed them. On the whole, NWN is much less leak-prone than it used to be.

FWIW, we have a memory-testing function in one of acaos' plugins. I'd offer to run a new round of tests, but I'm way too busy these days. It's nwnx_system, which I THINK is in his eXalt plugins. We use it to force resets if memory use climbs too high, but it hasn't been an issue so far as I'm aware, setting aside one or two incidents with runaway (mis-scripted) pseudos. Here's a snippet from our module heartbeat, where it's checked:

#include "hg_inc"

#include "fky_chat_inc"
#include "hg_client_inc"

#include "ac_effect_inc"
#include "ac_itemprop_inc"


void SendResetBroadcast (string sMessage, int bTell, object oMessenger) {
    sMessage = "<cþ þ>" + sMessage + "</c>";

    object oPC = GetFirstPC();

    while (GetIsObjectValid(oPC)) {
        SendChatLogMessage(oPC, sMessage, oMessenger, (bTell ? 4 : 5));
        oPC = GetNextPC();
    }
}


void main() {
    object oMod = GetModule();
    object oMes = GetMessenger();
    int nUptime = GetLocalInt(oMod, "uptime");
    int nMemory = GetProcessMemoryUsage();
    int nMessages = 0, nPlayers = 0;
    string sServer = GetLocalString(oMod, "ServerNumber");
    string sBootTime = IntToString(GetLocalInt(oMod, "boottime"));

    {
        object oPC;

        for (oPC = GetFirstPC(); GetIsObjectValid(oPC); oPC = GetNextPC()) {
            nPlayers++;
            RecalculateMovementRate(oPC);
            RecalculateDexModifier(oPC);

            int nAlarm = GetLocalInt(oPC, "AlarmUptime");
            if (nAlarm > 0 && nAlarm <= nUptime) {
                DeleteLocalInt(oPC, "AlarmUptime");
                SendChatLogMessage(oPC, C_PINK + "[Alarm] " + GetLocalString(oPC, "AlarmMessage") + C_END, oMes, 4);
            }
        }

        SetLocalInt(oMod, "ServerPlayers", nPlayers);
    }

    SQLExecDirect("SELECT UNIX_TIMESTAMP() - " + sBootTime + ", UTC_TIMESTAMP(), UNIX_TIMESTAMP(), " +
        "COUNT(*) FROM user_messages WHERE um_recipient = '*" + sServer + "'");

    if (SQLFetch() == SQL_SUCCESS) {
        nUptime = StringToInt(SQLGetData(1));
        nMessages = StringToInt(SQLGetData(4));

        SetLocalInt(oMod, "uptime", nUptime);
        SetLocalInt(oMod, "realtime", StringToInt(SQLGetData(3)));
        SetLocalString(oMod, "utctime", SQLGetData(2));

        SQLExecDirect("UPDATE server_list SET srv_utime = " + IntToString(nUptime) + ", srv_memory = " +
            IntToString(nMemory) + ", srv_players = " + IntToString(nPlayers) + " WHERE srv_id = '" + sServer + "'");

        SQLExecDirect("SELECT COUNT(*) FROM user_list WHERE u_active > 0");
        if (SQLFetch() == SQL_SUCCESS)
            SetLocalInt(oMod, "GlobalPlayers", StringToInt(SQLGetData(1)));

        SQLExecDirect("SELECT COUNT(*) FROM user_list WHERE u_active > 0 AND u_server_name REGEXP '" + GetStringLeft(sServer, 1) + ".[1-8]'");
        if (SQLFetch() == SQL_SUCCESS) {
            int nHubPlayers = StringToInt(SQLGetData(1));

            SetLocalInt(oMod, "HubPlayers", nHubPlayers);

            if (GetStringLeft(sServer, 1) != "1" && GetStringRight(sServer, 1) == "1")
                SetNumberOfPlayers(nHubPlayers);
        }
    }


    /* check for server messages if available */
    if (nMessages > 0) {
        int nGuild;
        string sMessage;
        string sSQL = "SELECT um_id, um_guild, um_text FROM user_messages WHERE um_recipient = '*" + sServer + "'";

        SQLExecDirect(sSQL);

        sSQL = "";

        while (SQLFetch() != SQL_ERROR) {
            if (sSQL == "")
                sSQL = "um_id = " + SQLGetData(1);
            else
                sSQL += " OR um_id = " + SQLGetData(1);

            nGuild   = StringToInt(SQLGetData(2));
            sMessage = SQLDecodeSpecialChars(SQLGetData(3));

            if (nGuild == -2) {
                object oPC = GetFirstPC();

                while (GetIsObjectValid(oPC)) {
                    if (!(GetPCFilter(oPC, PCFILTER_CHANNELS) & 1))
                        SendChatLogMessage(oPC, sMessage, oMes);

                    oPC = GetNextPC();
                }
            } else if (nGuild == -3) {
                object oPC = GetFirstPC();

                while (GetIsObjectValid(oPC)) {
                    if (!(GetPCFilter(oPC, PCFILTER_CHANNELS) & 2))
                        SendChatLogMessage(oPC, sMessage, oMes);

                    oPC = GetNextPC();
                }
            } else if (nGuild == -4) {
                object oPC = GetFirstPC();

                while (GetIsObjectValid(oPC)) {
                    if (!(GetPCFilter(oPC, PCFILTER_CHANNELS) & 4))
                        SendChatLogMessage(oPC, sMessage, oMes);

                    oPC = GetNextPC();
                }
            } else if (nGuild < 0) {
                SendMessageToDMDMs(sMessage);
                SendMessageToPCDMs(sMessage);
                SendMessageToDMAdmins(sMessage);
                SendMessageToPCAdmins(sMessage);
            } else if (nGuild > 0) {
                object oPC = GetFirstPC();

                while (GetIsObjectValid(oPC)) {
                    if (GetLocalInt(oPC, "Guild") == nGuild && !(GetPCFilter(oPC, PCFILTER_GUILD) & 1))
                        SendChatLogMessage(oPC, sMessage, oMes);

                    oPC = GetNextPC();
                }
            } else
                AssignCommand(oMod, SpeakString(sMessage, TALKVOLUME_SHOUT));
        }

        if (sSQL != "")
            SQLExecDirect("DELETE FROM user_messages WHERE " + sSQL);
    }


    /* scan players for inter-server messages every minute; also check for auto-reset */
    if (nUptime >= GetLocalInt(oMod, "LastMessageCheck") + 60) {
        SetLocalInt(oMod, "LastMessageCheck", nUptime);

        if (nMemory > GetLocalInt(oMod, "resetmemory") && !GetLocalInt(oMod, "resetforce")) {
            SetLocalInt(oMod, "resetforce", 1);

            if (GetLocalInt(oMod, "resetuptime") > nUptime)
                SetLocalInt(oMod, "resetuptime", nUptime - 1);
        }


        /* update PCs in the user list */
        UpdateAllClients(sServer, sBootTime, oMes);


        /* check for automatic reset */
        int nResetUptime = GetLocalInt(oMod, "resetuptime");
        if (nResetUptime > 0 && nUptime > nResetUptime) {

            if (!GetIsObjectValid(GetFirstPC())) {
                ResetServer();
                return;
            }
            if (nUptime > nResetUptime + 900) {
                SendResetBroadcast("SERVER RESET IN 10 SECONDS - SERVER REBOOT IS NOW COMMITTED - CANCELLATION " +
                    "IS NO LONGER POSSIBLE - PLEASE STAY OUT OF BANK CHESTS - HAVE A NICE DAY!", 1, oMes);
                DelayCommand(9.0, ResetServer());//ResetServer has built-in 1 second delay
            } else {
                int bTell = 0;
                int nSeconds = (nResetUptime + 900) - nUptime;
                string sMinutes = IntToString((nSeconds / 60) + 1);

                if (sMinutes == "1" || sMinutes == "5" || sMinutes == "10" || sMinutes == "15")
                    bTell = 1;

                if (GetLocalInt(oMod, "resetforce"))
                    SendResetBroadcast("AUTOMATIC SERVER RESET IN " + sMinutes + " MINUTE" +
                        (sMinutes != "1" ? "S" : "") + " - THIS CANNOT BE ABORTED DUE TO A LOW MEMORY CONDITION", bTell, oMes);
                else
                    SendResetBroadcast("AUTOMATIC SERVER RESET IN " + sMinutes + " MINUTE" +
                        (sMinutes != "1" ? "S" : "") + " - USE !delayreset TO DELAY THE RESET", bTell, oMes);
            }
        }
    }
*snip*

We also use it in our dm_serverinfo command:

            } else if (sDMText == "serverinfo") {
                DeleteLocalString(oDMPC, "FKY_CHAT_LOCAL_DMTEXT");

                string sMessage = COLOR_WHITE + "Server " + GetLocalString(GetModule(), "ServerNumber") +
                    "\\nModule: Higher Ground - Path of Ascension CEP Legends\\n\\n";

                sMessage += COLOR_WHITE + "Uptime: " + FormatTime(GetLocalInt(GetModule(), "uptime"), TRUE) +
                    " (reset at " + FormatTime(GetLocalInt(GetModule(), "resetuptime")) + ")\\n";

                sMessage += COLOR_WHITE + "Memory: " + IntToString(GetProcessMemoryUsage() / 1048576) + "MB (reset at " +
                    IntToString(GetLocalInt(GetModule(), "resetmemory") / 1048576) + "MB)\\n" + COLOR_END;

                SendChatLogMessage(oDMPC, sMessage, oDMPC, 5);
            }

Funky

[leo_x]

MagicalMaster wrote...
So passing actual effects is a bad idea?  I don't think I've actually passed an effect recursively but good to know.  What else is struct-type info in NWN?  Locations?  Vectors?

Presumably it's safe to pass ints/floats/strings/objects (assuming the object acts like a pointer to the object in this case -- does it?)...but what's bad?

I realize that it definitely doesn't matter in most cases anyway but would prefer to avoid bad habits.

Along with the ones Funky mentioned in his other post, talents and locations are also engine structs.  Objects are just unsigned 32bit integer IDs, so should be as safe as an int.  Vectors are passed by value as well, internally it just pushes 3 floats on the stack.  I suspect that NWscript structs are just pushed member-wise and so if they don't contain engine structs are probably safe.

[MagicalMaster]

So avoid passing effects, item properties, talents, and locations.

[Shadooow]

Sometimes passing an effect is unavoidable, my question is if the memory leak can be avoided by this method:

void recursivefunction1(effect e)
{
DoSomething();
DelayCommand(1.0,recursivefunction2(e));
}
void recursivefunction2(effect e)
{
DoSomething();
DelayCommand(1.0,recursivefunction1(e));
}

void main()
{
effect e = GetFirstEffect(OBJECT_SELF);
DelayCommand(1.0,recursivefunction1(e));
}

[FunkySwerve]

@ Magical Master: yup.

@ ShaDoOoW: Agreed, sometimes it's unavoidable. We do it in 6 scripts in our mod, based on a quick search for '(effect e', and discounting a number of non-effect-passing search returns.

As to your question about if it can be avoided by juggling recursive functions, I don't know. I suspect not, but it's possible.

It's probably worth reiterating that the leak under discussion is comparatively minor. I've yet to see it trigger a low memory condition on our mod by itself, though we do reset every 12 hours, give or take, to refresh quests.

Funky

[virusman]

For a 20-byte structure, you'd have to leak it 2.5 million times to see a noticeable (50MB) memory bump. That's running a server for 29 days with a pseudo heartbeat that's called every second.

[MagicalMaster]

Is the memory leak a permanent one or one that lasts until the script ends? Not sure how good the "garbage collector" for NWN might be if it even exists.

Also, does this happen each and every time a location/effect/item property/talent is passed to a function or only if it is passed multiple times recursively? And how does it this interact with built-in functions like GetDistanceBetweenLocations (does that always trigger two memory leaks)?

Does returning one of these also cause an issue or only passing as an argument?

[leo_x]

 Pretty sure the memory leak in question is only from an engine structure being passed as an argument to the function argument of DelayCommand, AssignCommand, and ActionDoCommand.  That's how I've interpreted it.  These types of memory leaks would persist until the nwserver process exited, basically they're mallocs without frees

Modifié par pope_leo, 11 décembre 2013 - 09:37 .

[MagicalMaster]

Good to know, thanks, I'll make sure I avoid those as much as possible.