For-loop problem

[Bv202]

Hi,

I have some strange problem I don't really understand. Maybe I'm missing something very stupid, but I can't figure out what I'm doing wrong

So I have this at the beginning of my code:
vector<car*> database;

As I use dynamic memory, I need to clear it at the end.
So I'm doing this (I believe this is correct, please tell me if I'm doing anything wrong here):

for (int i = 0; i < database.size(); i++)
	{
		delete database[i]; 
	}

Pretty easy. It compiles and it doesn't give any problems (ofcourse I can't see if the memory is really deleted - is there any way to check this?) but it gives a warning:

warning C4018: '<' : signed/unsigned mismatch
At this line:
for (int i = 0; i < database.size(); i++)

I don't get it. Am I missing something VERY stupid or why does the compiler gives a warning?

[R0mai]

It gives you this warning because vector::size() returns an unsigned integer, and your variable 'i' is a signed integer. Your code will run totally correctly, you won't have any leaks, but if you change int to unsigned, the warning will disappear. Unless database's size is VERY big (depending on your complier) you won't have any problems with this code either.

[Bv202]

Hi R0mai,

Okay, thank you very much for the explaination. I've made my int unsigned and it compiles fine now =)

[jsmith]

The right answer is

for( vector<car*>::size_type i = 0; i < database.size(); ++i )
    delete database[i];

[R0mai]

Yes jsmith that's the right answer, but please don't tell me you write this down every time :)
Although the most generic answer would be to use iterators, but I found iterators slower for vector than this operator[] approach.

[jsmith]

I do, though sometimes I cheat and use size_t instead, because I know that's what my implementation
of STL uses for size_type.

I don't know why or how you find that vector iterators are slower. vector iterators are pointers; they
should be as fast as indexing.

Anyway I tend to use the iterators because it is most generic.

[helios]

vector iterators are pointers; they
should be as fast as indexing.

That's what I used to think, until I implemented a UTF-8 to wchar_t conversion function and string iterators were ten times slower than directly accessing the string with raw pointers.

EDIT: After some testing, here's what I found: Iterators are much slower than pointers without optimization. With optimization, they're take 3.8 times the time for the same algorithm with pointers.
Here are the times for converting a 280 MiB UTF-8 file:
Pointers: 2515 ms.
Iterators: 9547 ms.

[Bv202]

Jsmith, what is wrong with mine and why is your correct?

[jsmith]

@helios:

#include <vector>

typedef std::vector<int>      vec_t;
typedef vec_t::const_iterator vec_citer_t;

int with_pointers( const int* first, const int* last ) {
    int sum = 0;
    for( ; first != last; ++first )
        sum += *first;
    return sum;
}

int with_iterators( vec_citer_t first, vec_citer_t last ) {
    int sum = 0;
    for( ; first != last; ++first )
        sum += *first;
    return sum;
}

int main() {
    vec_t v( 50, 1 );
    int sum_1 = with_pointers( &v[0], &v[v.size()] );
    int sum_2 = with_iterators( v.begin(), v.end() );
    return sum_1 + sum_2;
}

Compiled without optimization using GCC obviously iterators are slower since GCC doesn't even
inline functions at -O0.

Compiled at -O3, here is the assembler output of the two functions:


_Z13with_pointersPKiS0_:
.LFB498:
pushl %ebp
.LCFI0:
xorl %eax, %eax
movl %esp, %ebp
.LCFI1:
movl 8(%ebp), %edx
movl 12(%ebp), %ecx
cmpl %ecx, %edx
je .L4
.p2align 4,,7
.L5:
addl (%edx), %eax
addl $4, %edx
cmpl %edx, %ecx
jne .L5
.L4:
popl %ebp
ret

.globl _Z14with_iteratorsN9__gnu_cxx17__normal_iteratorIPKiSt6vectorIiSaIiEEEES6_
.type _Z14with_iteratorsN9__gnu_cxx17__normal_iteratorIPKiSt6vectorIiSaIiEEEES6_, @function
_Z14with_iteratorsN9__gnu_cxx17__normal_iteratorIPKiSt6vectorIiSaIiEEEES6_:
.LFB499:
pushl %ebp
.LCFI2:
xorl %eax, %eax
movl %esp, %ebp
.LCFI3:
movl 8(%ebp), %edx
movl 12(%ebp), %ecx
cmpl %ecx, %edx
je .L13
.p2align 4,,7
.L14:
addl (%edx), %eax
addl $4, %edx
cmpl %edx, %ecx
jne .L14
.L13:
popl %ebp
ret


... Identical.

[jsmith]

@Bv202:

R0mai is correct in the analysis of the problem:

vector<>::size() returns a vector<>::size_type, not an int. vector<>::size_type happens to be a typedef
for size_t, which happens to be a typedef for unsigned int.

Therefore, the comparison i < database.size() is comparing a [signed] int (i) against an unsigned int
(database.size()). This generates the warning.

[helios]

You're right, GCC inlines iterators -- furthermore, for a big enough n, iterators are actually faster -- but not all compilers do. If you know your compiler inlines iterators and you'll only be using that compiler, then you don't lose anything by using them. If you're writing cross-platform, however...

[jsmith]

good heavens, please tell me what compiler you are using that doesn't inline iterators so I know never to use it.

[helios]

VC++ 9.0

[jsmith]

ok, that won't be a problem then.

why does it not inline them? After all, begin() and end() for vector should be one liners and so should prefix operator++ for the iterator.

[helios]

Beats me.

_Myt __CLR_OR_THIS_CALL operator++(int)
	{	// postincrement
	_Myt _Tmp = *this;
	++*this;
	return (_Tmp);
	}

Maybe something to do with CLR?

[jsmith]

Well, to compare vector iterators to pointers you have to use pre-increment, not post-increment. Otherwise it isn't a fair test.