C++ array resize

Started by
22 comments, last by Misery 12 years, 9 months ago

A couple of things:

First, I would talk out the usage of Boost with my employers. As far as I am concerned, Boost is an integral part of C++ and contains mostly things that should have been in the standard library in the first place (and some of them now are in C++0x). Unless you are working on a compiler with horrible template support, a very embedded platform with limited capabilities or suffer from some weird license issues (extremely weird, considering Boost's rather permissive license) there is not much reason not to use Boost.
In general using well-tested code (and Boost is that) will be much faster than writing it on your own, never mind all those teeny tiny bugs you have to weed out of your own code. Even if Boost contains a bug (not impossible but increasingly unlikely in the well-matured core libraries) I would expect the Boost mailing list could offer a fix for a reproducible bug within a day or two. At worst, you could hack a fix together yourself since the code is all there.

Second, I notice you have a class to grant access to the single bits of a byte. Have you made sure that class is not padded by the compiler to 32/64 bit boundaries? Have you checked in the standard that allocating an array of these classes will never introduce any padding? Because if you didn't, you are wasting memory by a factor of four or eight.

Last, you don't need to specify functions as inline if they are declared in the class body. Neither do you need to specify void for no-argument functions (that's C).


Bear in mind though that the inline keyword is just a suggestion to the compiler, most optimising compilers are smart enough to figure out that if they find a implementation of a function in a header you meant to inline it. Some functions found in cpp files will be inlined as well when the compiler knows it will provide better speed or according to Optimisation settings in your build environment.

That said however it is nice to still place inline in front of implementations in headers as it gives a heads up to the next programmer that that is what you actually meant to happen.

Worked on titles: CMR:DiRT2, DiRT 3, DiRT: Showdown, GRID 2, theHunter, theHunter: Primal, Mad Max, Watch Dogs: Legion

Advertisement
That is another can of worms which I did not want to open. Personally, the only place where I ever use inline nowadays is when it is required (for example because a function definition needs to be in a header). But ignoring modern optimizing compilers, things like unnecessary inlines or void parameter list always smell fishy and could very well raise an 'uh oh' when trying to land a job somewhere.
I checked bitset and it looks cool. Very cool, however it still has capacity of 16 bytes when empty. My class has 8.
And padding? I don't think it should happen here, because I have exactly 8 bytes. Char ptr and integer both have 4 bytes.

And padding? I don't think it should happen here, because I have exactly 8 bytes. Char ptr and integer both have 4 bytes.

I'm not worried about padding of TBitArray. Possible padding of TEightBits worries me and you cannot see that by doing a sizeof(TBitArray).
Your class doesn't implement the rule of three. Resize() doesn't work if you change the number of bits without changing the number of bytes (e.g. resize 10 to 11 bits). You are implementing "min" yourself for some reason. You've re-implemented the expression for converting #bits into #bytes in a number of places, often multiple times in the same function. Copy() isn't exception safe. There doesn't appear to be much checking for the case where the underlying storage is NULL.

Correctness before speed, always.

[quote name='Misery' timestamp='1309784853' post='4830939']
And padding? I don't think it should happen here, because I have exactly 8 bytes. Char ptr and integer both have 4 bytes.

I'm not worried about padding of TBitArray. Possible padding of TEightBits worries me and you cannot see that by doing a sizeof(TBitArray).
[/quote]

Ok. But TEightBits consists only of unsigned char. Why should it cause problem?

Your class doesn't implement the rule of three. Resize() doesn't work if you change the number of bits without changing the number of bytes (e.g. resize 10 to 11 bits). You are implementing "min" yourself for some reason. You've re-implemented the expression for converting #bits into #bytes in a number of places, often multiple times in the same function. Copy() isn't exception safe. There doesn't appear to be much checking for the case where the underlying storage is NULL.

Correctness before speed, always.


Well, as I said it is unfinished, or rather still at work so I need to do there a lot more.
Thanks for pointing out that mistake with non working resize. The idea was to change the size of array only when number of bytes needed changes and I didn't notice that.
About the NULL - I know :) But I just wanted to discuss the idea itself.
I think that I will think about correctnes, error handling etc when I'll try to make this a part of the larger project.

Thanks once more everybody for wasitng their time to help me :]

[quote name='BitMaster' timestamp='1309787710' post='4830962']
[quote name='Misery' timestamp='1309784853' post='4830939']
And padding? I don't think it should happen here, because I have exactly 8 bytes. Char ptr and integer both have 4 bytes.

I'm not worried about padding of TBitArray. Possible padding of TEightBits worries me and you cannot see that by doing a sizeof(TBitArray).
[/quote]

Ok. But TEightBits consists only of unsigned char. Why should it cause problem?
[/quote]

Because I do not know what the standard says about a class containing only a single char. Usually it would be smart to make sure that the individual members are aligned on 32/64 bit boundaries. That is why a class containing an uint32 and an uint8 will usually be 8 bytes big on 32 bit systems. If the compiler decides to pad you TEightBits class, you waste memory by a factor of four or eight. Even if your compiler does not do it, the question is if the standard promises that or if you can guarantee you will only ever use the code with that one compiler.

[quote name='Misery' timestamp='1309790051' post='4830985']
[quote name='BitMaster' timestamp='1309787710' post='4830962']
[quote name='Misery' timestamp='1309784853' post='4830939']
And padding? I don't think it should happen here, because I have exactly 8 bytes. Char ptr and integer both have 4 bytes.

I'm not worried about padding of TBitArray. Possible padding of TEightBits worries me and you cannot see that by doing a sizeof(TBitArray).
[/quote]

Ok. But TEightBits consists only of unsigned char. Why should it cause problem?
[/quote]

Because I do not know what the standard says about a class containing only a single char. Usually it would be smart to make sure that the individual members are aligned on 32/64 bit boundaries. That is why a class containing an uint32 and an uint8 will usually be 8 bytes big on 32 bit systems. If the compiler decides to pad you TEightBits class, you waste memory by a factor of four or eight. Even if your compiler does not do it, the question is if the standard promises that or if you can guarantee you will only ever use the code with that one compiler.
[/quote]

Ok. How do i test it?

Ok. How do i test it?

If it is enough to know no padding is added for *this* compiler with *these exact* settings, checking sizeof(TEightBits) should be enough. Personally, I would add a BOOST_STATIC_ASSERT for that. The check is done at compile time and automatically no matter what changes behind the scenes.

However, even if no padding is done I had the impression you have to be as speed- and time-efficient as possible. Accessing the three (or seven on x64) TEightBit instances which are not perfectly on machine word boundaries will be slower than it could be. Not horribly slower, but slower. If I had to do it with such a helper class, I would suggest

template <typename BaseType, unsigned BitCount = sizeof(BaseType) * 8>
class TMyBitHelper
{
// fill as appropriate
};

#ifdef WIN32
typedef TMyBitHelper<boost::uint32_t> MyBitHelper;
#else
typedef TMyBitHelper<boost::uint64_t> MyBitHelper;
#endif

Not tested, but the basic idea should be clear.

This topic is closed to new replies.

Advertisement