11 Jan, 2009, Hades_Kane wrote in the 1st comment:
Votes: 0
I wasn't sure if this was just a problem I was having on my MUD, but at a glance at QuickMUD, it seems it might be a problem there too, and if it is, it might be something affecting all ROM MUDs.

I was having a problem with our mem% getting too high… after about a week or so, the MUD would crash because it ran out of available memory. I narrowed down to a function we had running every area update which would run through the entire MUD, destroy all of the "buried treasure" and go back through and place new treasure. This meant about 300 objects being destroyed then created every area update.

After some more investigating, Midboss concluded that it was likely that the objects weren't being freed properly upon being destroyed, and so that memory was still in use and the new objects were taking up new memory.

I put in some bug calls in the purge, extract_obj, and free_obj functions in order to track how far each function was making it, and I found that at the top of free_obj there is this check:

if (!IS_VALID(obj))
return;


That's where the free_obj function was stopping, so all of the memory freeing that is done lower in the function wasn't being done.

I went back as far as the purge function and put in this check:

if(obj->valid == TRUE)
bug("do_purge: obj is valid",0);
else
bug("do_purge: obj is not valid",0);


And the object was not considered valid. At the top of extract_obj, I added this:

if(obj != NULL && obj->valid == FALSE)
VALIDATE(obj);


And now the free_obj function is making it all the way to the bottom.

After doing that, I compared previous memory % when calling the bury_treasure command 100 times with current memory %, and it has dropped dramatically.

Again, I'm not sure if this was just a problem my game was having, but I thought I would post this as an alert that others might want to double check that their free_obj functions are making it to the bottom and actually freeing the memory. The function we had running is a bit of an extreme example, and I doubt too many other games have something running like that which would cause a problem like we were having, but just in case, I thought I would post this. Didn't think it would hurt for the RaM guys to see this either.

Also, on the off chance I've potentially messed something up by validating the obj at the top of extract_obj, it wouldn't hurt to have someone set me straight on that :p
11 Jan, 2009, Davion wrote in the 2nd comment:
Votes: 0
You might want to look into why the objects aren't valid instead of just validating them to suit your needs. This seems more like a work around then an actual fix. Whenever an object is created it should be validated. Maybe setup a watchpoint to see when an object is validated and invalidated.
11 Jan, 2009, Hades_Kane wrote in the 3rd comment:
Votes: 0
Hrmm, it does seem in new_obj it does validate(obj) and of course, create_object calls new_obj, but a check for IS_VALID(obj) at the end of create_object does not have the object listed as valid.

Any ideas?

Edited to add:
And is this just me, or does this seem to affect anyone else?
11 Jan, 2009, ghasatta wrote in the 4th comment:
Votes: 0
One thing to keep in mind is that a well known property of Diku-derived MUDs is that they *never* free allocated memory during their run. Any memory required is allocated, and then when that object is no longer needed, the allocated memory goes into a pool where it is available for the next new object.

So, your heap is never going to shrink. However, if you are destroying objects, the memory allocated for them should be recycled. If your memory usage is continually increasing, this indicates either that stuff is happening in your game that is causing more objects to be created and persist, or objects are not being added to the recycle pool properly, or are not being pulled from the recycle pool properly. You should verify all these items using a debugger.

I wonder if the original symptom you described is happening because free_obj() is being called multiple times on the same pointer? Have you tried walking through this with a debugger and seeing what happens?
11 Jan, 2009, Hades_Kane wrote in the 5th comment:
Votes: 0
ghasatta said:
Have you tried walking through this with a debugger and seeing what happens?


I'm honestly not quite sure how to do that. Generally any time I've had a problem, either adding in send_to_char or bug calls have been enough to narrow down what's causing a problem, or back tracing a crash in GDB, and generally I've learned things as I've needed to use them.

A debug message checking for whether or not an object is considered valid in the create_object function is not passing as valid, either through an oload or even immediately after a copyover.
11 Jan, 2009, quixadhal wrote in the 6th comment:
Votes: 0
ghasatta said:
One thing to keep in mind is that a well known property of Diku-derived MUDs is that they *never* free allocated memory during their run. Any memory required is allocated, and then when that object is no longer needed, the allocated memory goes into a pool where it is available for the next new object.


Actually, that's only true of muds which derive from the MERC branch, since they were the drunken sots.. errr… clever programmers who wrote the recycling code that subsequent MUD's have been stuck with. :)

Of course, you might STILL not see your heap shrink, since in order for your OS to return memory to the system, there has to be NO blocks in use on a given page. Odds are, you've allocated more than one thing since acquiring a new page, and therefore are probably freeing things from other pages. In systems with high turnover, pages get freed quite often, but MUD's typically only free small bits at a time, so it gets more fragmented.

To walk around the code in gdb, typically you have to set breakpoints and/or triggers to have it print values whenever the program counter passes through the bit of code you're interested in. I admit, it's a rare day I have to remember how to do that, but google will help you if (like me), you hate the GNU people for stubbornly clinging to their "info" documentation format instead of switching to HTML like the rest of the world, or man pages as the UNIX Gods intended.
12 Jan, 2009, Guest wrote in the 7th comment:
Votes: 0
quixadhal said:
ghasatta said:
One thing to keep in mind is that a well known property of Diku-derived MUDs is that they *never* free allocated memory during their run. Any memory required is allocated, and then when that object is no longer needed, the allocated memory goes into a pool where it is available for the next new object.


Actually, that's only true of muds which derive from the MERC branch, since they were the drunken sots.. errr… clever programmers who wrote the recycling code that subsequent MUD's have been stuck with. :)


Well it's not true of anything in the Smaug branch. No memory recylcing going on there. Wrappers around calloc() and free() basically. Unless you want to call the string hash a recycler, but even then if only one copy of a string is left, it's free'd out when it gets deleted.
12 Jan, 2009, Scandum wrote in the 8th comment:
Votes: 0
Anyone willing to bet when Hades Kane is gonna get rid of his custom memory manager and runs things through Valgrind before coming here?
12 Jan, 2009, Scandum wrote in the 9th comment:
Votes: 0
Samson said:
Well it's not true of anything in the Smaug branch. No memory recylcing going on there. Wrappers around calloc() and free() basically. Unless you want to call the string hash a recycler, but even then if only one copy of a string is left, it's free'd out when it gets deleted.

There's nothing wrong with a good string hasher, and arguably you need it depending on how much trash can be accumulated in areas and player inventories.

I think ROM has taken things the furthers in the memory department with a separate freed memory list for each data type.
12 Jan, 2009, Sandi wrote in the 10th comment:
Votes: 0
'valid' is a boolean to keep from recycling the same thing twice. It should be set on your objects in new_obj()
12 Jan, 2009, Hades_Kane wrote in the 11th comment:
Votes: 0
quixadhal said:
To walk around the code in gdb, typically you have to set breakpoints and/or triggers to have it print values whenever the program counter passes through the bit of code you're interested in. I admit, it's a rare day I have to remember how to do that, but google will help you if (like me), you hate the GNU people for stubbornly clinging to their "info" documentation format instead of switching to HTML like the rest of the world, or man pages as the UNIX Gods intended.


Ah ok, I have done that before and believe I have documentation lying around somewhere on how to do that :p

Sandi said:
'valid' is a boolean to keep from recycling the same thing twice. It should be set on your objects in new_obj()

It is, and new_obj() is called at the very top of the create_object function, but by the end of that same function, is isn't considered valid. I'm going to crawl through it a bit more and see if I can't figure out why, but I have yet to find anything that would seem to change its valid status.
12 Jan, 2009, Hades_Kane wrote in the 12th comment:
Votes: 0
I notice that when I click on "Recent Posts" that this wasn't showing up, despite having more recent of a post than about half of the posts on that page. I've noticed this with other threads, too. Is this a bug or is there something that determines what shows up on that page other than the most recent posts?
12 Jan, 2009, elanthis wrote in the 13th comment:
Votes: 0
Might just be cached (either on the server or in your browser) and showing stale data. It seems to be up to date when I looked at it.
12 Jan, 2009, Skol wrote in the 14th comment:
Votes: 0
Does that to me sometimes too Hades, like on the Rom Area thing, i had to look and find it even though it was way newer last post than some of the ones it was showing.
12 Jan, 2009, Davion wrote in the 15th comment:
Votes: 0
If you've read every post in the thread it doesn't show up.
12 Jan, 2009, Hades_Kane wrote in the 16th comment:
Votes: 0
Davion said:
If you've read every post in the thread it doesn't show up.


Ah ok, thanks for clearing that up :p

I guess if I don't see a thread I'm following in that, then I'm current :)
12 Jan, 2009, Davion wrote in the 17th comment:
Votes: 0
Heh, ya. Although sometimes, even when you're all caught up they still appear. I can't figure out for the life of me why that is (something to do with the crazy marker system QSF has.) But if you go to the main board page and click (or just click this url ->) 'mark all' it'll reset so nothing shows up.
13 Jan, 2009, Hades_Kane wrote in the 18th comment:
Votes: 0
Ok, I think I've narrowed it down a bit.

This is my new_obj function:

OBJ_DATA *new_obj(void)
{
static OBJ_DATA obj_zero;
OBJ_DATA *obj;

if (obj_free == NULL)
obj = alloc_perm(sizeof(*obj));
else
{
obj = obj_free;
obj_free = obj_free->next;
}
*obj = obj_zero;
VALIDATE(obj);
memset(obj, 0, sizeof(*obj));

return obj;
}


When it reaches line 15 of the function:
memset(obj, 0, sizeof(*obj));
that's where it is no longer valid.

A check of the QuickMud src shows this:

OBJ_DATA *new_obj (void)
{
static OBJ_DATA obj_zero;
OBJ_DATA *obj;

if (obj_free == NULL)
obj = alloc_perm (sizeof (*obj));
else
{
obj = obj_free;
obj_free = obj_free->next;
}
*obj = obj_zero;
VALIDATE (obj);

return obj;
}


That line appears to be the only difference. Any idea what that is supposed to do, and if I need it?

Thanks!
13 Jan, 2009, Davion wrote in the 19th comment:
Votes: 0
That's your issue! memset is setting every value in that to 0. The call, in that instance is redundant any ways. A static variable is automatically set to 0 upon allocation. So doing that is totally pointless and safe to remove entirely.
13 Jan, 2009, Hades_Kane wrote in the 20th comment:
Votes: 0
Davion said:
That's your issue! memset is setting every value in that to 0. The call, in that instance is redundant any ways. A static variable is automatically set to 0 upon allocation. So doing that is totally pointless and safe to remove entirely.


Awesome, thanks! I figured the fact that it wasn't used in stock and doesn't appear anywhere else in the codebase probably meant it wasn't necessary. I'm glad to see this isn't a ROM wide issue and was isolated to my game. I still don't know where that came from, I don't recall adding that in… But yeah, I'll get rid of it. Thanks again.
0.0/20