C++ Lobby Lister Class

[jrgp]

On devs wiki: http://devs.soldat.pl/wiki/index.php?title=Client-Lobby_Protocol_Parsers#By_jrgp
My personal server backup: http://jrgp.us/snipplets/lobby_client.cpp.txt

Compiles/runs on Linux, FreeBSD, and others. The exe generated by ming (using the comment at the top of the file) runs on Windows. I have not tried compiling the source directly on Windows but I assume it works since I'm using the winsock stuff.

It's [over] commented so dealing with it should be easy. Certain parts can be minified but I was too lazy. I was also too lazy to implement specific filtering.

Enjoy or use it for whatever.

[pubby8]

Nitpicks (since it is on wiki):
Put the "using namespace std" inside functions, not in global scope.
Don't use strlen inside for().
Use prefix ++ instead of postfix.

[FliesLikeABrick]

Can you explain why each of those things is the better way to do it?  Otherwise we have no way to know if what you're saying is just preference or factually better for some reason.

[pubby8]

Put the "using namespace std" inside functions, not in global scope.

If "using namespace" is in a header file in global scope, then it applies to anything that includes that header. This causes lots of naming problems and is bad practice.

Don't use strlen inside for().

This technically means you must call strlen every iteration of the loop, when you only need to call it once. Some compilers (GCC) have specific optimizations for strlen inside for loops, but it is still bad practice and you should not rely on compiler to optimize it. Store value in variable and then use variable in loop!

Use prefix ++ instead of postfix.

This is pretty minor, but prefix doesn't return the value while postfix does. Prefix should always be used in for loops, and it is considered best practice to do it if you don't need to return the value.

[jrgp]

Don't use strlen inside for().

This technically means you must call strlen every iteration of the loop, when you only need to call it once. Some compilers (GCC) have specific optimizations for strlen inside for loops, but it is still bad practice and you should not rely on compiler to optimize it. Store value in variable and then use variable in loop!

I agree with you, which is why it wasn't called each time. I think you read it wrong.

Code: (cpp) [Select]

for (i = 0, len = strlen(buff); i < len; i++)
Anything called/set before the first semicolon is just ran once.

I agree with the other two points you mentioned.

[FliesLikeABrick]

Yeah the strlen() in a loop was one that I agree is bad.  didn't know/care about the other two because I don't do anything particularly complex with classes/namespaces - and the other one just seemed very minor.


jrgp is right, that strlen() will only get called once

[DorkeyDear]

in regards to i++ and ++i;
that's pretty much the most i can contribute here.

[pubby8]

Don't use strlen inside for().

This technically means you must call strlen every iteration of the loop, when you only need to call it once. Some compilers (GCC) have specific optimizations for strlen inside for loops, but it is still bad practice and you should not rely on compiler to optimize it. Store value in variable and then use variable in loop!

I agree with you, which is why it wasn't called each time. I think you read it wrong.

Code: (cpp) [Select]

for (i = 0, len = strlen(buff); i < len; i++)
Anything called/set before the first semicolon is just ran once.

I agree with the other two points you mentioned.

Oh, hehe. Didn't see the comma!

[FliesLikeABrick]

I'd love it if you could provide me some references on the pre/post increment thing.  Thinking about it from a low-level machine point of view I'm not sure how they'd be implemented differently enough to make a difference at all, even in terms of just a few clock cycles.

http://physical-thought.blogspot.com/2008/11/pre-vs-post-increment-speed-test.html

I was about to do a test like that, but he did basically the same exact thing.

pubby, can you provide some evidence/links on the matter?

[pubby8]

It is mostly a problem if you overload classes with ++, as postfix may have to copy the value.

For normal types, it doesn't really matter, but I have habit of using prefix.

[Mr]

Instead of using an index to loop through a std::vector, use iterators instead, it's faster and results in smaller code:

Code: [Select]

// Show them
for (vector<server_entry>::iterator server = servers.begin(); server != servers.end(); server++) {
// (refer to any of the fields mentioned by struct near beginning of file)
cout << server->name << " " << server->ip << ":" << server->port << endl;
}

Thanks for the class, using it already.

EDIT: The destructor of the class is never called, add a "delete lc;" after receiving the server list.
The result of the following is always the same:

Code: [Select]

// Request
char req[100];
sprintf(req, "e%c0%c0%c0%c0%c0%c0%c0%c0%c0%c0%c0%c-1%c0%c0\n", 169, 169, 169, 169, 169, 169, 169, 169, 169, 169, 169, 169, 169, 169);
So why not shorten it to:

Code: [Select]

#define LB_SEP "\169"
const char req[] = "e"LB_SEP"0"LB_SEP"0"LB_SEP"0"LB_SEP"0"LB_SEP"0"LB_SEP"0"LB_SEP"0"LB_SEP"0"LB_SEP"0"LB_SEP"0"LB_SEP"0"LB_SEP"-1"LB_SEP"0"LB_SEP"0\n";


[pubby8]

Iterators aren't faster, they have more overhead.

Also, you should put a reserve before you start using push_back, so you don't end up with lots of reallocs.

[Clawbug]

Well, to throw something in from my part, as I suggested to jrgp on IRC, the standard C libraries should be prefixed with c and should not have .h postfix in them, meaning that e.g. stdio.h becomes cstdio etc.

[FliesLikeABrick]

Yeah, for a vector/array/anything else which uses sequential memory - [] should always be faster than an iterator because you're directly addressing the memory offset.

[ramirez]

Iterators aren't faster, they have more overhead.

Yeah, for a vector/array/anything else which uses sequential memory - [] should always be faster than an iterator because you're directly addressing the memory offset.

This is a common misconception, however it is not true. The correct answer is that which is faster is implementation specific and hence depends on the compiler and what settings you use with it. The standard does in no way specify in which way a compiler should implement the iterator for a given container, the implementation details are completely up to the compiler, you can't say "iterators have more overhead", because different compilers can implement iterators differently. The ONLY thing that is defined is that a certain type of an iterator must do certain things, eg. a forward_iterator must be able to be incremented (these are called "iterator concepts" for the record).

Now that we've gotten technicalities out of the way, let's get back to the real world. What most people don't seem to know is that most compilers actually implement vector iterators as pointers. Yes, that's right. Pointers. I've seen as crude implementations as the iterator being defined as a pointer, but most modern compilers like GCC and MSVC do a bit more; they have a wrapper classes around those pointers. Namely in GCC uses __gnu_cxx::__normal_iterator, and MSVC10 uses std::_Vector_iterator. But make no mistake, these classes are nothing more than very simple wrappers around the underlying pointer types, eg. for vector<int>, they simply wrap int*. Now you might think that "since they're classes, there's obviously extra overhead using them." Well not quite true. These classes are very simple classes with all of their functions defined as inline. When you have your compiler optimizations on, your compiler will inline the code in the place of the function call, which essentially results in same as using a pointer directly.

Neither is obviously faster than the other. It depends completely on the compiler's implementation, its optimizer, whether running a debug build without optimizations or release build with optimizations, et cetera. To show you what I mean, I made a simple test case:

Code: [Select]

#include <vector>
#include <boost/progress.hpp>

int main()
{
        const int entries = 100000000;

        std::vector<int> v;
        v.reserve(entries);
        for (int i = 0; i < entries; ++i) {
                v.push_back(i);
        }

        // benchmark counter
        {
                boost::progress_timer timer;
                for (int i = 0; i < entries; ++i) {
                        v[i] += 10; // do something so loop doesn't get optimized away
                }
        }

        // benchmark iterator
        {
                boost::progress_timer timer;
                for (std::vector<int>::iterator i = v.begin(); i != v.end(); ++i) {
                        *i += 10; // do something so loop doesn't get optimized away
                }
        }
}
Here's the results with GCC:

$ g++ -O3 benchmark.cpp && ./a.out
0.12 s
0.08 s
$ g++ -O0 benchmark.cpp && ./a.out
0.48 s
1.92 s

As you can see, iterators are faster with full optimization (which you would normally use when you deploy your application), but operator[] is faster without optimizing, which is simply explained by the fact that the iterator code isn't even inlined in that case, not to speak of other optimizations the compiler might make. These results will more than likely vary from compiler to compiler, and there's simply no basis to think that either iterators or counter is faster than the other. I also tested on MSVC, and the results are pretty much the same with _SECURE_SCL set to 0. By default MSVC uses a feature called "checked iterators" which adds extra security, if you want to favor speed, you can do so by turning off checked iterators by turning _SECURE_SCL to 0. Basically this is similar to the operator[] vs .at() in std::vector, except it's a global setting.

So the speed should NOT be the thing you base your decision on. I would use iterators instead of counter and operator[] for following reasons:
- Iterators are made for this very purpose, and the compiler manufacturers will try their best to optimize it for this purpose.
- If you want to deal with container's data in a forward-like manner, then iterators are the most self-documenting way to do just that (this is just my opinion of course).
- Assuming that the underlying implementation uses pointer iterators, to access the iterating element only a de-reference of a pointer is required, with array access it requires a pointer addition arithmetic operation before de-referencing.
- With other containers that don't support random access, you'd have to use iterators, and using them in this situation as well makes your code more consistent.
- In certain situations when you use .begin() and .end() it's harder to write code that accesses the container at wrong position.

If the speed is your biggest issue, then the only thing you can really do is benchmark the code on the platforms you target, and make your decision based on that. That being said, 99% of people should not care about the little speed differences there are between these two implementations.

[FliesLikeABrick]

Interesting, I thought that since STL vectors are implemented how they are that what I said about operator[] is true (regardless of whether iterators are the same or faster or slower)

My understanding of how STL vectors are implemented is that they use an array internally and when it is outgrown there is a new array that is created which is double the size (roughly speaking, the real implementation is more nuanced). 

[ramirez]

Interesting, I thought that since STL vectors are implemented how they are that what I said about operator[] is true (regardless of whether iterators are the same or faster or slower)

My understanding of how STL vectors are implemented is that they use an array internally and when it is outgrown there is a new array that is created which is double the size (roughly speaking, the real implementation is more nuanced). 

Yeah, that is how a vector is usually implemented, but however as usual the standard doesn't force the compiler to do it this exact way. The one thing that the standard specifies is that "vector must be a contiguous area of memory".

[pubby8]

Hm, very interesting how iterators can produce faster code, however I wanted to point out that the most likely increase in speed you saw in your example was because of cache usage. If you switch the tests around, you will find that iterators are slightly slower if the test code was put before the non-iterator code.

I do agree that vector iterators should be used, but I think most people overuse vectors, and iterators overall.

[Clawbug]

Hm, very interesting how iterators can produce faster code, however I wanted to point out that the most likely increase in speed you saw in your example was because of cache usage. If you switch the tests around, you will find that iterators are slightly slower if the test code was put before the non-iterator code.

I do agree that vector iterators should be used, but I think most people overuse vectors, and iterators overall.

How can people overuse vectors or iterators in C++? STL containers are more or less meant to be used instead of old-fashioned plain array data structures, and iterators are meant to be used for iterating through them. It's kind of like std::string vs. C-string thing; in C++ using good ol' C strings is more or less considered bad practice(error prone, not always clear, POINTERS yuck!) because you've got std::string. Of course old C strings are perfectly valid, but I'd guess that it's more because of better backward compatibility with C and it's legacy rather than as an practical language feature.

I myself use STL containers as much as possible, and try to use high level features of C++ whenever possible instead of doing things in the "good old C way", even though I am much bigger fan of C than C++. After all, they're two distinct languages.

[ramirez]

Hm, very interesting how iterators can produce faster code, however I wanted to point out that the most likely increase in speed you saw in your example was because of cache usage. If you switch the tests around, you will find that iterators are slightly slower if the test code was put before the non-iterator code.

The results stay the same even after swapping them with GCC 4.45, that is not the reason. As I explained they boil down to simple pointers, THAT is the reason why they're faster.