-2

I am dynamically allocating memory at the beginning of a for using:

Candset_t* emptycandset(void) {
    Candset_t* cset;

    cset = (Candset_t*)malloc(sizeof(Candset_t));
    cset->size = 0;
    cset->maxsize = MAXSIZE;
    cset->dptr = (Point_t*)malloc(MAXSIZE * sizeof(Point_t));

    return cset;
}

whereby Candset and Point_t are both typedef. later I free the memory at the end of the loop with:

void destroycandset(Candset_t* cset) {
    free(cset->dptr);
    free(cset);
}

The reason why I am doing this is because I want to reuse the same variable(memory space) in all the iterations of the loop. This actually cause fragmentation of the heap. Hence, decreased performance. How can I solve this? is it possible to reinitiate a dynamically allocated space? How does this affect the dynamic memory? Cheers

I am trying to reuse a dynamically allocated memory. However, using malloc and free cause fragmentation of the heap. Do you know any way to avoid memory fragmentation?

typedef struct point {
    double x, y;
    unsigned int dst;
} Point_t;
    
typedef struct candset {
    unsigned int size;
    unsigned int maxsize;
    Point_t* dptr;
} Candset_t;

This is what the struct data look like.

I can't tell what the size of the dptr and Candset_t will be; Hence, I have to allocate them dynamically. It make no difference when using new and delete for c++ only.

Mooing Duck
  • 64,318
  • 19
  • 100
  • 158
aristocrate90
  • 1
  • 1
  • 4
    If you ever feel the need to use a C-style cast when programming in C++, then you should take it as a sign that you're doing something wrong. The "wrong" in this case is using `malloc` instead of `new`. And not using constructors. And not using vectors. And your own dynamic memory handling in the first place (explicit pointers are likely not needed for your use-case). – Some programmer dude Jan 23 '23 at 13:08
  • 1
    i am doing this to increase performance, but actually performance decreases... thats called premature optimization. Did you acutally measure and see that this needs a tweak? Anyhow the code is max unclear. `Candset_t` and `Point_t` are aliases, ok, but for what? Please read about [mcve] – 463035818_is_not_an_ai Jan 23 '23 at 13:08
  • 1
    `(Candset_t*)malloc( ...)` is generally just wrong in C++. It might be OK, but no way to tell from the code you posted – 463035818_is_not_an_ai Jan 23 '23 at 13:10
  • 1
    `cset = (Candset_t*)malloc(sizeof(Candset_t));` `cset->size = 0;` is [Undefined Behaviour](https://en.cppreference.com/w/cpp/language/ub) pre C++ 20 and we would need to see the declaration of `Candset_t` post C++20. – Richard Critten Jan 23 '23 at 13:11
  • 2
    If you want to reuse the same variable, why don't you just do that? – molbdnilo Jan 23 '23 at 13:15
  • This code shouldn't be improved, but first rewritten as proper C++ i.e. with constructors and destructors, and probably `std::vector`. – MSalters Jan 23 '23 at 13:15
  • Use std::vector and you can reserve size up front to avoid reallocations. The problem with malloc is that it wil NOT call constructors and thus all your objects are in an undefined state. But as said before, your code looks more like "C" then "C++". So I would first learn more C++ before worrying about performance optimization. Can it be you are actually doing some competitive coding? If so still learn C++ first, and if code is too slow... (TLE) then rethink your algorithm. Most speed is usually found there. – Pepijn Kramer Jan 23 '23 at 13:38

1 Answers1

1

For structures smaller than 1 MB whose lifetime is simple, don't use dynamic allocation at all.

Just create a Candset_t in automatic storage (on the stack).

Automatic storage provides memory extremely efficiently.

If your object is "simple" enough to malloc-and-modify, it is also free to create in automatic storage.

There are people who are used to languages like Java or C#, where creating an object is inherently expensive.

In C++, creating an object can be literally free. It can also be expensive. How expensive it is depends on the properties of the object.

Because objects in C++ carry less baggage than in garbage collected languages, the "free" case is really, really free.

To make an object effectively free to use, ensure that it doesn't use dynamically allocated memory at construction time, and to be extra sure avoid virtual functions (sometimes these can also be free). Next, don't use new or malloc to create it.

Here is a simple "free" class:

struct Candset_t {
  std::size_t size;
  Point_t data[MAXSIZE];
};

so long as MAXSIZE isn't really big, like in the many 1000s, this is going to be a better plan than the original.

If this doesn't work, then you can just move the declaration of Candset_t up a scope, and reuse it. You do not want to reallocate dptr, but instead write code to clear the memory in it.

Generally, calls to malloc or new take 100x or more the time of a typical line of C++ code. So avoid them in tight loops.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524