Thread: Re: [Dev-C++] Optimizing using the STL
Open Source C & C++ IDE for Windows
Brought to you by:
claplace
|
From: David M. <ci...@ya...> - 2003-07-26 03:03:58
|
Hmm,
Some interesting comments. (I know I need to increment the
iterator, I
forgot that when I sent this e-mail). So far the only
optimization that
would be applicable is the passing by refrence. This
program is
multi-threaded so even though I could use mutexes, etc. to
prevent the
string from changing between calls to write_to_player I
would rather not
have that headache. If you are wondering I plan to put the
map in a
Read-write mutex (the only time that map changes is when a
player logs in or
logs out).
Thanks for all your comments.
----- Original Message -----
From: "P.D. van Zelst" <pa...@my...>
To: <ci...@ya...>
Sent: Friday, July 25, 2003 7:18 AM
Subject: RE: [Dev-C++] Optimizing using the STL
> Hi,
>
> Well, incrementing the iterator inside your while loop
would be a
> performance improvement I suppose :P
> And to nitpick about the arguments a bit... it would be
more efficient
> to pass the string as a const string&.
> But other than that I wouldn't know how to optimize this
function any
> further. I know there's a template function "memfun",
which allows you
> to call member functions in objects, but I don't know if
it's possible
> to pass parameters to that.
>
> Regards,
>
>
> Paul
>
> -----Original Message-----
> From: dev...@li...
> [mailto:dev...@li...] On
Behalf Of David
> McKen
> Sent: Thursday, July 24, 2003 9:17 PM
> To: Dev-C Mailing List
> Subject: [Dev-C++] Optimizing using the STL
>
>
> Any suggestions on how I can optimized the following
> function?
>
> void pc_char::write_to_all_pc(string to_send, bool
> add_eoln)
> {
> map<string, pc_char *>::iterator i =
> current_players.begin();
>
> while (i != current_players.end()) {
> (*i).second->write_to_player(to_send, add_eoln);
> }
> }
>
> I was hoping to use for_each but kept coming across the
> problem of "How the hell am I going to pass the arguments
> to write_to_player?"
>
> Thanks in Advance
>
> =====
> Signed
> David Mcken
>
> Life Sucks
> Live with it
>
> __________________________________
> Do you Yahoo!?
> Yahoo! SiteBuilder - Free, easy-to-use web site design
software
> http://sitebuilder.yahoo.com
>
>
> -------------------------------------------------------
> This SF.Net email sponsored by: Free pre-built ASP.NET
sites including
> Data Reports, E-commerce, Portals, and Forums are
available now.
> Download today and enter to win an XBOX or Visual Studio
.NET.
>
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01
> /01
> _______________________________________________
> Dev-cpp-users mailing list
> Dev...@li...
> TO UNSUBSCRIBE:
http://www23.brinkster.com/noicys/devcpp/ub.htm
>
https://lists.sourceforge.net/lists/listinfo/dev-cpp-users
>
>
__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software
http://sitebuilder.yahoo.com
|
|
From: David M. <ci...@ya...> - 2003-07-27 22:27:55
|
Whew, long e-mail.
Some interesting ideas there. You are right I am not too
familar with the
STL. I have a question though. What is the difference
between passing a
refrence and passing a const refrence? Is that to ensure
that the original
can never be changed(as would happen of I passed by value)?
My other comment is that current_players is a member of
pc_char. It is
static and thus shared by all instances of pc_char. So I am
guessing the
function signature stays the same.
From the looks of it using for_each may not be worth it
especially since the
function is not finished. What has to be added to this
function is a check
to make sure the write_to_player is not called for the
object invoking
write_to_all_pc (We don't want the message being sent to
the person who is
sending it). Would something like this work? (or rather is
it reccomended?)
void pc_char::write_to_all_pc(const string& to_send, bool
add_eoln)
{
for(map<string, pc_char *>::iterator i =
current_players.begin();
i != current_players.end(); ++i)
if((*i).second != this)
(*i).second->write_to_player(to_send, add_eoln);
}
With respect to the books. I will look into getting my
hands on a copy.
Thanks ALOT!
David Mcken
----- Original Message -----
From: "Hossein Haeri" <ult...@em...>
To: <ci...@ya...>
Sent: Sunday, July 27, 2003 4:07 PM
Subject: Re: [Dev-C++] Optimizing using the STL
> David,
>
> > Some interesting comments. (I know I need to increment
the
> > iterator, I
> > forgot that when I sent this e-mail). So far the only
> > optimization that
> > would be applicable is the passing by refrence. This
> > program is
> > multi-threaded so even though I could use mutexes, etc.
to
> > prevent the
> > string from changing between calls to write_to_player I
> > would rather not
> > have that headache. If you are wondering I plan to put
the
> > map in a
> > Read-write mutex (the only time that map changes is
when a
> > player logs in or
> > logs out).
>
> Consider the following if you want to adhere to your own
handcraft
> loop:
>
> void pc_char::write_to_all_pc(const string& to_send, bool
add_eoln)
> {
> for(map<string, pc_char *>::iterator i =
current_players.begin();
> i != current_players.end(); ++i)
> (*i).second->write_to_player(to_send, add_eoln);
> }
>
>
> It is a truly mentioned tip that you must prefere using
references
> to constants instead of passing arguemnts by value. But,
I presume that
> you know that it "may" not have any effect to the
underlying code
> generated by the compiler, as in most of the modern C++
> implementations, std::string is implemented using
reference-counted
> smart pointers. But it is not wise, anyway, to rely on
this fact. Keep
> on using that cute refernce to const!
>
> Another point is that I have placed "i" to lie inside the
loop body to
> optimise the lifetime issues. It is not needed outside!
>
> And the last point is that I have chosen preincrementing
the loop index
> to the postincrementing it. I hope you know how much this
can save you!
>
> Furthermore, it seems to me that the "current_players"
map is not a
> data member of pc_char. If I'm right, it means that,
according to the
> Const-Correctness issues you must change the function
signature to the
> following:
>
> void pc_char::write_to_all_pc(/*The same parameters*/)
const;
>
> As this member function is not mutating the internal
status, so it must
> be qualified const.
>
> > > I was hoping to use for_each but kept coming across
the
> > > problem of "How the hell am I going to pass the
arguments
> > > to write_to_player?"
>
> It is not that hard! It is enough to write a unary
function object that
> serves the action you need for a given object of the
desired type
> (using a member function invokation).
>
> E.g., consider the following:
>
> class write_to_all_pc_fun
> : public std::unary_function<pc_char, void>
> {
> public:
> write_to_all_pc_fun(const string& to_send, bool
add_eoln)
> : ts(to_send), aeoln(add_eoln) {}
> void operator () (pc_char& p) {p.write_to_all_pc(ts,
aeoln);}
> private:
> string ts;
> bool aeoln;
> };
>
> Now you could use std::for_each like this:
>
> void pc_char::write_to_all_pc(const string& to_send, bool
add_eoln)
> {
> std::for_each(
> current_players.begin(),
> current_players.end(),
> write_to_all_pc_fun(to_send, add_eoln));
> }
>
> To be honest, this is not the neatest solution! In fact,
the best
> solution is to either combine the gadgets like
std::mem_fun1_ref and
> the binders, or use Boost stuff in Lambda Library
> (http://www.boost.org). But I haven't considered those
solutions, as
> even STL-experinced programmers may fail to digest the
code truly.
> Especially, because that way, you have to either write
your own
> handcrafted mem_fun2_ref (that's whithin the ability of
> STL-professionals that are heavily experts in Generic
Programming as
> well), or search whitin the Function Library of Boost to
find an
> appropriate tool. (It seems to me that you are not so
familiar with STL
> and Generic Programming. I hartappologise if that's not
the case! But
> if I'm right, consider the following Bibles of STL and
Generic
> Programming:
>
> Matthew H. Austern, Generic Programming and the STL:
Using and
> Extending the C++ Standard Template Library,
Addison-Wesley, 1999,
> ISBN: 0-201-30956-4.
>
> Scott Meyers, Effective STL: 50 Specific Ways to Improve
Your Use of
> the Standard Template Library, Addison-Wesley, 2001,
> ISBN: 0-201-74962-9.
>
> Don't think that the first one is old. It is still the
best manual for
> learning STL and especially Generic Programming. The
second one,
> indeed, assumes that you already have used STL for an
enough long time.
>
> I recommend those books to any C++ programmer, especially
my friends,
> including you! Furthermore, if you need better reviews of
the books,
> consider the following best C++ books reviewer:
http://www.accu.org)
>
> Regards,
> --Hossein
> --
>
__________________________________________________________
> Sign-up for your own FREE Personalized E-mail at Mail.com
> http://www.mail.com/?sr=signup
>
> CareerBuilder.com has over 400,000 jobs. Be smarter about
your job search
> http://corp.mail.com/careers
>
__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software
http://sitebuilder.yahoo.com
|
|
From: Hossein H. <ult...@em...> - 2003-07-31 16:12:47
|
Hi David,
> Some interesting ideas there. ...
Some? Just some? Oh my Goodness! Thanks for that! I could except any
description expect that one.
> ... You are right I am not too
> familar with the
> STL.
You will become! For now on, just don't panic when you see powerful
STL codes.
> ... What is the difference
> between passing a
> refrence and passing a const refrence? Is that to ensure
> that the original
> can never be changed(as would happen of I passed by value)?
Hmmm... Yep in the sense that the original beast wouldn't be changed,
but nop in the sense that there is big differnce between passing by
value and passing by reference: The former copies the argument at the
moment of being called, whilst the later doesn't copy, it, in turn,
works on the same object, by creating a reference out of the
argument.
> My other comment is that current_players is a member of
> pc_char. It is
> static and thus shared by all instances of pc_char. So I am
> guessing the
> function signature stays the same.
Well, if you mean the same as what it now is, yes, it does. But the
important point here is that this is the first time I see a class
having a map (in fact, a container) of pointers to itself. My
recommendation is to change this strange design, unless you have
very plausible reasons for adhering to it. It is very likely to
result in sophisticated problems that are all hard to debug. (Amongst
them, one of the most important and vexing ones is the "Circular
Dependancies" problem.)
> From the looks of it using for_each may not be worth it
> especially since the
> function is not finished.
I read the bellow changes you want to be made. From my viewpoint,
that doesn't difere much the dicision wheather to use or not to use
for_each. Assuming that you are going to do the changes, you still can adhere to for_each. Especially because, that way, the only
change is needed to be done is inside that Functor, and no where
else. (Read on ...)
> ...
> void pc_char::write_to_all_pc(const string& to_send, bool
> add_eoln)
> {
> for(map<string, pc_char *>::iterator i =
> current_players.begin();
> i != current_players.end(); ++i)
> if((*i).second != this)
> (*i).second->write_to_player(to_send, add_eoln);
> }
At the moment beeing, the only very subtle bug I can see in it is the
one in the following line:
> if((*i).second != this)
Just think about it for a moment. You are storing pc_char* objects
in that map, right? But you are, probabely, just working with a copy
of the addresses of the original objects, huh? So those nifty-looking
pointers, which are beeing stored in the map, and are objects by
themselves, are copies of the "this" objects, and not the same ones!
Hence, althouge they point to the same location in the memory, they
are not the same, by themselves. Therefore, it is very likely that
the test fails, even if the semantic is right. In fact, you should be
very lucky if the code works. The solution I suggest (if it is OK
under your algorithm), is to replace it with the following one.
if(*(i->second) != (*this))
This way, you are checking if they are pointing to the same location,
and not if they are the same pointers! (And that's presumably what
you need!)
> With respect to the books. I will look into getting my
> hands on a copy.
Do it, ASAP.
Regards,
--Hossein
--
__________________________________________________________
Sign-up for your own FREE Personalized E-mail at Mail.com
http://www.mail.com/?sr=signup
CareerBuilder.com has over 400,000 jobs. Be smarter about your job search
http://corp.mail.com/careers
|
|
From: David M. <ci...@ya...> - 2003-07-31 16:55:41
|
Thanks for pointing that out..
There is no particular reason for keeping the algorithm.
I just like using less code, this method seems to use more.
(You have the original function and a functor now).
I have another question considering files but I think I
better start another thread for that.
Signed
David Mcken
--- Hossein Haeri <ult...@em...> wrote:
> Hi David,
>
> > Some interesting ideas there. ...
>
> Some? Just some? Oh my Goodness! Thanks for that! I could
> except any
> description expect that one.
>
> > ... You are right I am not too
> > familar with the
> > STL.
>
> You will become! For now on, just don't panic when you
> see powerful
> STL codes.
>
> > ... What is the difference
> > between passing a
> > refrence and passing a const refrence? Is that to
> ensure
> > that the original
> > can never be changed(as would happen of I passed by
> value)?
>
> Hmmm... Yep in the sense that the original beast wouldn't
> be changed,
> but nop in the sense that there is big differnce between
> passing by
> value and passing by reference: The former copies the
> argument at the
> moment of being called, whilst the later doesn't copy,
> it, in turn,
> works on the same object, by creating a reference out of
> the
> argument.
>
> > My other comment is that current_players is a member of
> > pc_char. It is
> > static and thus shared by all instances of pc_char. So
> I am
> > guessing the
> > function signature stays the same.
>
> Well, if you mean the same as what it now is, yes, it
> does. But the
> important point here is that this is the first time I see
> a class
> having a map (in fact, a container) of pointers to
> itself. My
> recommendation is to change this strange design, unless
> you have
> very plausible reasons for adhering to it. It is very
> likely to
> result in sophisticated problems that are all hard to
> debug. (Amongst
> them, one of the most important and vexing ones is the
> "Circular
> Dependancies" problem.)
>
>
> > From the looks of it using for_each may not be worth it
> > especially since the
> > function is not finished.
>
> I read the bellow changes you want to be made. From my
> viewpoint,
> that doesn't difere much the dicision wheather to use or
> not to use
> for_each. Assuming that you are going to do the changes,
> you still can adhere to for_each. Especially because,
> that way, the only
> change is needed to be done is inside that Functor, and
> no where
> else. (Read on ...)
>
> > ...
> > void pc_char::write_to_all_pc(const string& to_send,
> bool
> > add_eoln)
> > {
> > for(map<string, pc_char *>::iterator i =
> > current_players.begin();
> > i != current_players.end(); ++i)
> > if((*i).second != this)
> > (*i).second->write_to_player(to_send, add_eoln);
> > }
>
> At the moment beeing, the only very subtle bug I can see
> in it is the
> one in the following line:
>
> > if((*i).second != this)
>
> Just think about it for a moment. You are storing
> pc_char* objects
> in that map, right? But you are, probabely, just working
> with a copy
> of the addresses of the original objects, huh? So those
> nifty-looking
> pointers, which are beeing stored in the map, and are
> objects by
> themselves, are copies of the "this" objects, and not the
> same ones!
> Hence, althouge they point to the same location in the
> memory, they
> are not the same, by themselves. Therefore, it is very
> likely that
> the test fails, even if the semantic is right. In fact,
> you should be
> very lucky if the code works. The solution I suggest (if
> it is OK
> under your algorithm), is to replace it with the
> following one.
>
> if(*(i->second) != (*this))
>
> This way, you are checking if they are pointing to the
> same location,
> and not if they are the same pointers! (And that's
> presumably what
> you need!)
>
> > With respect to the books. I will look into getting my
> > hands on a copy.
>
> Do it, ASAP.
>
> Regards,
> --Hossein
=====
Signed
David Mcken
Life Sucks
Live with it
__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software
http://sitebuilder.yahoo.com
|