arturocepeda / Cflat

Embeddable lightweight scripting language with C++ syntax
49 stars 8 forks source link

Performance/Memory Usage/Flexibility Improvement - mInstances using std::deque #4

Closed stephenberry closed 6 months ago

stephenberry commented 2 years ago

mInstances in InstancesHolder currently uses a std::vector, so the maximum size is fixed so that a reallocation due to growth won't invalidate pointers. But, this induces arbitrary limits on the user and adds configuration options like kMaxLocalInstances. It also forces large allocations when they aren't always needed if a larger size is required anywhere in the program.

A simple fix is to use std::deque rather than std::vector as the mInstances container. In this way a full allocation is not needed when the first element is populated. It also removes some of the required capacity checking code. When std::deque grows it does not invalidate its current objects, so you don't have to worry about growth.

There will be a little performance loss when iterating through the deque as opposed to a vector, but I think saving memory and adding flexibility and less user confusion and arbitrary limits are all worth it. The less the user has to care about configuration the better, especially as these are arbitrary limits not conformant with C++11.

stephenberry commented 2 years ago

Can close once #1 is merged

arturocepeda commented 2 years ago

Yeah, agreed, I have never been too happy with the kMax solution, and the only reason why I haven't used a different container such as std::deque was the desire of keeping the entries in a contiguous memory space, to have fast iterations. But I agree with you: in this case, the advantages of changing the container type compensate for the little performance loss when iterating.

arturocepeda commented 6 months ago

The current revision uses a std::deque to hold instances, as suggested.