0

I wrote a small queue class in C++, on Windows.

Just to test, I'm allocating 1,0000,000 ints and reading them back.

The queue is doing its job correctly and is very fast, but the problem is that after complete deallocation, there is some memory usage reported by the task manager. For example, I still get 2MB of memory although all those integers lead to 400MB+ of memory.

I thought it was due to the operating system not performing some sort of GC or whatever it is, but now I found that this value (2MB) becomes 13MB when I allocate 100,000,000 integers (10x the previous number). I cannot find the leak.

Here is the code; as you can see, I deallocate everything:

#pragma once

#include <exception>

template <class T>
class colist_item
{

public:

    colist_item() { }
    ~colist_item() { }

    T value;
    colist_item *next;

};

template <class T>
class colist
{

public:

    colist() { this->m_root = NULL; this->m_count = 0; }
    ~colist() { }

    void enqueue(T value);
    T dequeue();
    T *peek();
    int count();

private:
    colist_item<T> *m_root;
    int m_count;

};

template <class T>
void colist<T>::enqueue(T value)
{
    if (this->m_root == NULL) {
        this->m_root = new colist_item<T>();
        this->m_root->value = value;
        this->m_root->next = NULL;
    } else {
        colist_item<T> *tempitem = new colist_item<T>();
        tempitem->value = value;
        tempitem->next = this->m_root;

        this->m_root = tempitem;
    }

    this->m_count++;
}

template <class T>
T colist<T>::dequeue()
{
    if (this->m_root == NULL) {
        throw std::exception();
    } else {
        T retval = this->m_root->value;
        colist_item<T> *next = this->m_root->next;
        delete this->m_root;
        this->m_root = next;

        this->m_count--;
        return retval;

    }
}

template <class T>
T *colist<T>::peek()
{
    if (this->m_root == NULL) {
        return NULL;
    } else {
        T retval = this->m_root->value;
        return &retval;
    }
}

template <class T>
int colist<T>::count()
{
    return this->m_count;
}

Main:

#include <iostream>
#include <limits>
#include "colist.h"
#include <time.h>

const int kNumItems = 100000000;

using namespace std;

void puttest(colist<int> *list)
{
    for(int i = 0; i < kNumItems; i++)
        list->enqueue(i);
}

void readtest(colist<int> *list)
{
    for(int i = 0; i < kNumItems; i++)
        list->dequeue();
}

int main(int argc, char *argv[])
{
    colist<int> list;

    cout << "Testing with : " << kNumItems << " elements" << endl;

    clock_t start = clock();
    puttest(&list);
    clock_t end = clock();

    double ms = ((end - start));
    cout << "puttest: " << ms << endl;

    start = clock();
    readtest(&list);
    end = clock();

    ms = ((end - start));
    cout << "readtest: " << ms << endl;

    cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
}
Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
goodolddays
  • 2,595
  • 4
  • 34
  • 51
  • 8
    The task manager is not a good way to test memory allocation. When the OS allocates memory to the application it practically never gives it back. The standard memory management routines keep track of allocation/deallocation so that it can re-use the meory and so it does not disturb the OS with repeated requests for more memory. – Martin York Jul 16 '11 at 16:05
  • So, in your opinion it is due to the fact that the OS is allocating lots of memory for me without releasing it completely because it expects other allocations from my program? If this is the case, how can I tell the operating system that I'm done with allocations and it can release the unsued memory? – goodolddays Jul 16 '11 at 16:07
  • 2
    @hkproj: Memory leak detection tools just do that for you. Use valgrind(on Linux) or Rational Purify(on Windows) to profile your code and it should give you exact details of where the memory leaks are if any. – Alok Save Jul 16 '11 at 16:10
  • @hkproj - You are allocating *virtual* memory. What is there to reclaim? – Bo Persson Jul 16 '11 at 16:34
  • @hkproj: Do not worry about giving memory back to the OS. You just need to make your application work without leaking. The standard memory allocators will interact with the OS on your behalf in the most efficient method. The fact that your application has some meory allocated does not affect the available memory for other applications (as it is all virtual). If it is unused it will be paged off to disk. – Martin York Jul 16 '11 at 16:37
  • @hkproj: You don't. Let the runtime handle it. – Lightness Races in Orbit Jul 16 '11 at 17:30
  • 2
    btw `time.h` is deprecated in C++. Use `ctime`. – Lightness Races in Orbit Jul 16 '11 at 17:31

3 Answers3

2

OK a couple of bugs.

No destructor.

You are not leaking any memory because you remove all the items. But if you did not remove them all manually then your class would leak when it goes out of scope. Add a destructor that just deletes all the items on the list.

Dequeue is not exception safe:

    T retval = this->m_root->value;
    colist_item<T> *next = this->m_root->next;
    delete this->m_root;   // If T is badly designed it could potentially throw an exception.
    this->m_root = next;   // Now you have m_root pointing at a bad object. The idea is to
                           // make sure an exception can not damage your internal structure
                           // of your object. Thus remove it from the chain before you call
                           // delete.

   // Like this
   T               retval    = m_root.value;
   colist_item<T>* oldhead   = m_root;
   m_root                    = oldhead->next;

   // Internal structure updated.
   // Safe to do dangerous tasks.
   delete oldhead;

Use the initializer list when constructing

colist() { this->m_root = NULL; this->m_count = 0; }

// should be

colist():  m_root(NULL), m_count(0) {}

You don't need to use this everywhere.

I know it is encouraged in java but in C++ it is usually discouraged but recognized as a style thing. Personally I find its use cluttering.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
Martin York
  • 257,169
  • 86
  • 333
  • 562
2

Run http://valgrind.org/ to ensure you don't have memory leaks.

how can I tell the operating system that I'm done with allocations and it can release the unsued memory?

As you will request more memory the c++ library will re-use what was given by the OS. So, generally, this is a non-problem.

Sometimes, because of memory fragmentation, some chunks of memory are really hard to reclaim. In this cases, you might want to use an Arena Allocator.

diego
  • 15
  • 2
  • 1
    @hkproj then you have no leaks! – Sam Miller Jul 16 '11 at 16:35
  • @hkproj: (as I noted in my answer) Your program does not leak as you manually remove all the items from the queue. But if you did not remove them manually (which I am sure is quite possible in a normal program) then it would leak. As a result you should delete the list in the destructor. – Martin York Jul 16 '11 at 16:46
0

As has been mentioned in the comments, you should not rely on the task-manager for analyzing the memory-usage of you program. This question lists several tools for Windows that are suited for memory-leak detection.

Community
  • 1
  • 1
Björn Pollex
  • 75,346
  • 28
  • 201
  • 283