-1

If I have a struct instanceData:

struct InstanceData
{
    unsigned usedInstances;
    unsigned allocatedInstances;
    void* buffer;

    Entity* entity;
    std::vector<float> *vertices;
};

And I allocate enough memory for an Entity and std::vector:

newData.buffer = size * (sizeof(Entity) + sizeof(std::vector<float>)); // Pseudo code
newData.entity = (Entity *)(newData.buffer);
newData.vertices = (std::vector<float> *)(newData.entity + size);

And then attempt to copy a vector of any size to it:

SetVertices(unsigned i, std::vector<float> vertices)
{
    instanceData.vertices[i] = vertices;
}

I get an Access Violation Reading location error.

I've chopped up my code to make it concise, but it's based on Bitsquid's ECS. so just assume it works if I'm not dealing with vectors (it does). With this in mind, I'm assuming it's having issues because it doesn't know what size the vector is going to scale to. However, I thought the vectors might increase along another dimension, like this?:

enter image description here

Am I wrong? Either way, how can I allocate memory for a vector in a buffer like this?

And yes, I know vectors manage their own memory. That's besides the point. I'm trying to do something different.

Community
  • 1
  • 1
Stradigos
  • 814
  • 1
  • 13
  • 29
  • What is it you're trying to achieve here? – Oliver Charlesworth Jun 24 '15 at 21:01
  • 2
    Nearly every time an object contains a pointer to a `vector`, `string` or `map`, you are PROBABLY doing SOMETHING wrong. – Mats Petersson Jun 24 '15 at 21:03
  • I'm allocating X amount of components, where components are an instance of the instance data. So I'm saying, "Hey, allocate enough memory for 10 components!" And then getting/setting those instances as needed. – Stradigos Jun 24 '15 at 21:03
  • @MatsPetersson, I think your missing the exercise here. How else would you allocate the entire memory buffer as a single allocation and then just let entity, vertices, etc, point to different parts of that buffer? – Stradigos Jun 24 '15 at 21:06
  • @Stradigos Don't use raw pointers, `new()`,`delete`,`malloc()`,`free()`, etc. yourself please. For most of the common use cases you're just doing it wrong. You rather use one of the [Standard container](http://en.cppreference.com/w/cpp/container) or [Dynamic memory management](http://en.cppreference.com/w/cpp/memory) classes. – πάντα ῥεῖ Jun 24 '15 at 21:09
  • @Stradigos _" I think your missing the exercise here ..."_ I suppose no one is missing anything from your question, besides you need to be clearer what you're actually asking about. – πάντα ῥεῖ Jun 24 '15 at 21:12
  • @Stradigos you have found out what happens when you use a vector and no one called the constructor to set up all of its book-keeping and internal structures. Lot more going on in your average constructor than memory allocation. – user4581301 Jun 24 '15 at 21:13
  • @user4581301, even when I do this: std::vector *vertices = new std::vector(0); in the initialization I get the same error. Or is that not sufficient enough? – Stradigos Jun 24 '15 at 21:17
  • @ πάντα ῥεῖ, What don't you understand about wanting to allocate a single chunk of memory for 10 instances of the component and letting the pointers point to different parts of the allocated chunk? It works, except with vectors, and I'm trying to learn why... I posted links for a reason. Check them out if you're still lost. Maybe you'll learn something too. – Stradigos Jun 24 '15 at 21:20
  • If you want to allocate one large lump of memory for "everything", then you should not use a pointer to a vector - in fact, you probably shouldn't use a vector at all - although I'm reasonably sure it makes absolutely no difference with the overhead of a pointer to a vector or not, unless you use one global array - as soon as you use SOME sort of pointer to allocated memory, you may just as well use a vector. – Mats Petersson Jun 24 '15 at 21:23
  • On it's own `std::vector *vertices = new std::vector(0);` is sufficient. Unfortunately, assigning it willy-nilly to an array of unaligned bytes is going to blow up your CPU when it tries to assign an int to an odd-numbered memory address. – user4581301 Jun 24 '15 at 21:25
  • @Stradigos _"Maybe you'll learn something too."_ Bub [**(-_-)**](http://www.nwliandmedispa.com/wp-content/uploads/2015/01/lash2.bmp) ! – πάντα ῥεῖ Jun 24 '15 at 21:26
  • @MatsPetersson, This is for a game engine, so I don't want to be allocating and deallocating memory, especially non-contiguous memory, at a whim. By pre-allocating chunks, I can reuse the same memory until I outgrow the need for it, at which time I would allocate a new chunk. I could use the swap + pop_back trick on vectors to minimize the performance impact and keep it somewhat contiguous, but I've read reports that indicate this method is better. Thank you for the intelligent responses. I'm interested in hearing any more wisdom you have. – Stradigos Jun 24 '15 at 21:42
  • @user4581301, the memory allocator I'm using (https://bitbucket.org/bitsquid/foundation/src) uses forward aligned bytes, I think. I'll have to confirm that, but if that's true does that mitigate the issue or is there something more I need to watch out for? – Stradigos Jun 24 '15 at 21:43
  • 1
    @Stradigos Better [**Büebli**](http://www.falleri.ch/index.php?option=com_content&view=article&id=263:es-isch-emal-es-bueebli-gsi) (our nearest neighbours have ingeniously subtle speech). Have a look at [placement `new()`](http://stackoverflow.com/questions/222557/what-uses-are-there-for-placement-new) may be. – πάντα ῥεῖ Jun 24 '15 at 21:44
  • Ran a bit of test code. Looks like my AMD chip is a lot kinder on misaligned data than I'm used to. I don't know what the performance impact was, but the program did not crash reading a misaligned long long. @Stradigos, I know where you're coming from. The less time spent on memory management, the better. I do this sort of shtick all the time in embedded systems. If you need a pool of pre-allocated vectors to pull vectors from at need, make a huge array of vectors. Don't malloc the suckers or mix them with other data. They weren't made to be used that way. – user4581301 Jun 24 '15 at 21:56

2 Answers2

3

It looks like you want InstanceData.buffer to have the actual memory space which is allocated/deallocated/accessed by other things. The entity and vertices pointers then point into this space. But by trying to use std::vector, you are mixing up two completely incompatible approaches.

1) You can do this with the language and the standard library, which means no raw pointers, no "new", no "sizeof".

struct Point {float x; float y;} // usually this is int, not float
struct InstanceData {
    Entity entity;
    std::vector<Point> vertices;
}

This is the way I would recommend. If you need to output to a specific binary format for serialization, just handle that in the save method.

2) You can manage the memory internal to the class, using oldschool C, which means using N*sizeof(float) for the vertices. Since this will be extremely error prone for a new programmer (and still rough for vets), you must make all of this private to class InstanceData, and do not allow any code outside InstanceData to manage them. Use unit tests. Provide public getter functions. I've done stuff like this for data structures that go across the network, or when reading/writing files with a specified format (Tiff, pgp, z39.50). But just to store in memory using difficult data structures -- no way.

Some other questions you asked:

How do I allocate memory for std::vector?

You don't. The vector allocates its own memory, and manages it. You can tell it to resize() or reserve() space, or push_back, but it will handle it. Look at http://en.cppreference.com/w/cpp/container/vector

How do I allocate memory for a vector [sic] in a buffer like this?

You seem to be thinking of an array. You're way off with your pseudo code so far, so you really need to work your way up through a tutorial. You have to allocate with "new". I could post some starter code for this, if you really need, which I would edit into the answer here.

Also, you said something about vector increasing along another dimension. Vectors are one dimensional. You can make a vector of vectors, but let's not get into that.

edit addendum:

The basic idea with a megabuffer is that you allocate all the required space in the buffer, then you initialize the values, then you use it through the getters.

The data layout is "Header, Entity1, Entity2, ..., EntityN"

// I did not check this code in a compiler, sorry, need to get to work soon
MegaBuffer::MegaBuffer() {AllocateBuffer(0);}
MegaBuffer::~MegaBuffer() {ReleaseBuffer();}

MegaBuffer::AllocateBuffer(size_t size /*, whatever is needed for the header*/){
    if (nullptr!=buffer)
        ReleaseBuffer(); 

    size_t total_bytes = sizeof(Header) + count * sizeof(Entity)
    buffer = new unsigned char [total_bytes];
    header = buffer;

    // need to set up the header
    header->count = 0;
    header->allocated = size;

    // set up internal pointer
    entity = buffer + sizeof(Header);
}

MegaBuffer::ReleaseBuffer(){
    delete [] buffer;
}

Entity* MegaBuffer::operator[](int n) {return entity[n];}

The header is always a fixed size, and appears exactly once, and tells you how many entities you have. In your case there's no header because you are using member variables "usedInstances" and "allocatednstances" instead. So you do sort of have a header but it is not part of the allocated buffer. But you don't want to allocate 0 bytes, so just set usedInstances=0; allocatedInstances=0; buffer=nullptr;

I did not code for changing the size of the buffer, because the bitsquid ECS example covers that, but he doesn't show the first time initialization. Make sure you initialize n and allocated, and assign meaningful values for each entity before you use them.

You are not doing the bitsquid ECS the same as the link you posted. In that, he has several different objects of fixed size in parallel arrays. There is an entity, its mass, its position, etc. So entity[4] is an entity which has mass equal to "mass[4]" and its acceleration is "acceleration[4]". This uses pointer arithmetic to access array elements. (built in array, NOT std::Array, NOT std::vector)

The data layout is "Entity1, Entity2, ..., EntityN, mass1, mass2, ..., massN, position1, position2, ..., positionN, velocity1 ... " you get the idea.

If you read the article, you'll notice he says basically the same thing everyone else said about the standard library. You can use an std container to store each of these arrays, OR you can allocate one megabuffer and use pointers and "built in array" math to get to the exact memory location within that buffer for each item. In the classic faux-pas, he even says "This avoids any hidden overheads that might exist in the Array class and we only have a single allocation to keep track of." But you don't know if this is faster or slower than std::Array, and you're introducing a lot of bugs and extra development time dealing with raw pointers.

Kenny Ostrom
  • 5,639
  • 2
  • 21
  • 30
  • Did you take a look at that Bitsquid ECS link in the original post by chance? Even after reading that and understanding the necessity, are you still dead set on the first solution? Here's my implementation of what that link talks about: http://pastebin.com/htPeXUye @user4581301, this is for you too since you've been so kind. The pastebin link doesn't feature the MeshSystem I'm working on, which needs to vectors to handle the realtime need of mesh manipulation. It's pretty much the same as Transform, but with different a different data structure. – Stradigos Jun 24 '15 at 22:35
  • I'm hearing what you guys are saying, I really am, but Bitsquid isn't an indie engine and the developer isn't a novice. Autodesk just acquired the engine, so it has AAA interest. I think there's some really good ideas in it, but now you guys are making me doubt lol. By the way, this post seems to be a solution for using vectors with this method, but I'm trying to understand it more: http://bitsquid.blogspot.com/2010/09/custom-memory-allocation-in-c.html?showComment=1329474140968#c3748357253605756112 Just now learning about in-place constructors. – Stradigos Jun 24 '15 at 22:40
  • 1
    @Stradigos Important point you aren't taking from bitsquid. It's using blocks of simple and contiguous data. std::vector is neither. The approach is not revolutionary. I used to do that all the time programming for 6804s. Because it works. You need to know where it works and where it does not, but a lot of that is doing what you're doing:Experimenting. Knowing how to bend the system to do what you want is good, too. Look into overloading `new` to draw from your reserved memory pool if you need to create more complicated structures and custom allocators. – user4581301 Jun 24 '15 at 23:18
  • @user4581301 Thanks for all your great, non-judgmental advice today. I was super frustrated at the start. The approach is not revolutionary, I agree, but I feel is often misunderstood and met with real resistance from people who don't understand or appreciate with a departure from "proper OOP" can do. The right tool for the right job though. I'm still learning, and this is one tool I intend to master. – Stradigos Jun 25 '15 at 02:35
  • 1
    What 4581301 said, if you want to do things in a buffer, use a typed pointer to access the memory area like a built-in array. Do not use std::vector for that. But std::vector is much easier and safer, and with modern compilers should be fast (as long as you don't get a bunch of temporary objects involved by accident). I'll try to post the code in the morning. – Kenny Ostrom Jun 25 '15 at 04:53
  • KennyOstrom, and @user4581301. Got it working :-) Proof: http://screencast.com/t/3hO5fyy6KNz Intro is just showing off the classes, I check the values at the end. Tomorrow I need to read on whether or not I'm leaking anything by using the in place new operator. I'm almost certain I am and that I need to call a special deconstructor of some sort. – Stradigos Jun 25 '15 at 05:32
  • 1
    @Stradigos line 42 of the center window you shouldn't need those `memcpy`'s. `new(newData.entity+size)` should have just made and initialized a vector at newData.entity+size just the way you wanted. However, vector does not actually contain its data, just a pointer to it, so you have a memory leak if you put that instance away without manually destroying the vector. This sort of memory management is exactly what bitsquid is trying to avoid. Using a custom allocator with your vector can help you out here, but you're better off not using vector at all and instead using a simpler structure. – user4581301 Jun 25 '15 at 16:27
  • @user4581301 If I decide to allocate more space I'll still need to copy the old allocated data into the new space, no? And I've seen you guys mention I should be using something other than vector a few times, but I don't know what else to use if not a vector. The data structure is holding mesh data that could possibly change frame to frame, so it needs to be a variable size. I know that sort of memory management is what bitsquid was trying to avoid, but I don't see a way around it while still maintaining continuous memory like this. The end goal is reduced cache miss. – Stradigos Jun 25 '15 at 17:12
  • 1
    @Stradigos memcpy is really, really dumb. Given a vector, it copies the element count and the pointer to the elements. Not the contents of the pointer. This leaves you with two vectors pointing at the same array. Normally this is doom, because one of the vectors would delete the array on resize or destruction and the other vector would be left pointing to bad memory. Doesn't happen to you because you aren't doing anything with the old vector, including calling the destructor. It does leave a maintenance issue if another coder takes your work. – user4581301 Jun 25 '15 at 17:45
  • 1
    If you use std::vector you MUST DISREGARD EVERYTHING ELSE about allocating or copying anything inside the vector. Either you use std::vector member functions, or you do not use std::vector. If you follow the ECS allocation examples, you will use built-in arrays and pointer arithmetic. – Kenny Ostrom Jun 25 '15 at 17:58
  • @user4581301 Oh wow that's bad. I see your point. And point taken from you, Kenny, as well. Accepting this answer now that I've got a firmer understanding. Thanks again to you both. – Stradigos Jun 25 '15 at 18:28
  • 1
    @Stradigos two solutions: Roll-your-own dynamic array where you control the memory allocation/deallocation/copying or allocate worst-case sized mesh pool and do the same partitioning trick to dole bits of it out to instances as needed. Vector is built to handle different use cases. for future reference, vector implements the `=` operator with enough smarts to efficiently copy itself. If that's not enough, `std::copy` can handle the rest. Also look into `std::move` for cases that you don't care about the old vector any more. – user4581301 Jun 25 '15 at 19:16
1

I think I see what you are trying to do.

There are numerous issues. First. You are making a buffer of random data, telling C++ that a Vector sized piece of it is a Vector. But, at no time do you actually call the constructor to Vector which will initialize the pointers and constructs inside to viable values.

This has already been answered here: Call a constructor on a already allocated memory

The second issue is the line

instanceData.vertices[i] = vertices;

instanceData.vertices is a pointer to a Vector, so you actually need to write

(*(instanceData.vertices))[i]  

The third issue is that the contents of *(instanceData.vertices) are floats, and not Vector, so you should not be able to do the assignment there.

Community
  • 1
  • 1