| View previous topic :: View next topic |
| Author |
Message |
James Christie
Guest
|
 Posted: Wed Mar 23, 2005 8:37 pm Post subject: passing AnsiString to my class and program crawls :-( |
 |
|
Hi Guys,
I am trying to populate a TStringGrid with some data and then use that
data to draw a nice graph. So I parse the data I receive to populate the
cells of the TStringGrid, so the user can see what he's received, and then
I thought I may as well convert the values into doubles, which is what the
graph requires, and store that information in the TStringGrid->Objects.
I created this class to store the data in the TStringGrid->Objects
//--------------------------TMyData.h----------------------------------
class TMyData : public TObject {
public:
__fastcall TMyData(const AnsiString &data);
__fastcall TMyData(TDateTime data);
__property double Value = {read = FValue};
__property bool Valid = {read = FValid};
private:
double FValue;
bool FValid;
};
//-----------------------TMyData.cpp---------------------------------
static AnsiString numbers = "0123456789";
__fastcall TTexData::TTexData(const AnsiString &data)
{
try {
if (!numbers.Pos(data.AnsiLastChar())) data.SetLength(data.Length() - 1);
// Data may have character at end
if (data.Pos(":")) FValue = (TDateTime)data; // Data may be a date/time
else FValue = data.ToDouble(); // Numeric
FValid = true;
}
catch(...) {
FValue = NULL;
FValid = false;
}
}
//--------------------------------------------------------------------
and in my main program I have the following representitive snip of code
.......
RegEx->Expression = space; // Regular Expression Component
RegEx->StringToMatch = data;
RegEx->Split(*RegExList); // Splits data into a StringList
for (int i = 0; i < RegExList->Count; i++) {
DataGrid->Cells[DataGrid->Col][DataGrid->Row] = RegExList->Strings[i];
// Show raw value
DataGrid->Objects[DataGrid->Col][DataGrid->Row] = new
TMyData(RegExList->Strings[i]); // ***** SLOW
DataGrid->Col++;
}
......
Using RQ's profiler to get some numbers I find the average time to run
through my main loop of code parsing the data is 0.76 seconds!! If I remove
the creation of TMyData the average loop time reduces to just 0.002 seconds.
Any suggestions as to what is causing the slow down ??
cheers,
James
|
|
| Back to top |
|
 |
Remy Lebeau (TeamB)
Guest
|
| Posted: Wed Mar 23, 2005 9:06 pm Post subject: Re: passing AnsiString to my class and program crawls :-( |
|
|
"James Christie" <james (AT) texmate (DOT) co.nz> wrote
| Quote: | DataGrid->Objects[DataGrid->Col][DataGrid->Row] = new
TMyData(RegExList->Strings[i]); // ***** SLOW
|
You have a potential memory leak there. Make sure that you are freeing any
previous entry before creating a new one.
You cannot use the '++' operator with properties. You need to use the '='
and '+' operators instead:
DataGrid->Col = DataGrid->Col + 1;
| Quote: | Using RQ's profiler to get some numbers I find the average time
to run through my main loop of code parsing the data is 0.76 seconds!!
If I remove the creation of TMyData the average loop time reduces to
just 0.002 seconds. Any suggestions as to what is causing the slow down ??
|
Did you try profiling your actual constructor code to find the specific line
that is causing the slowdown?
Gambit
|
|
| Back to top |
|
|
Chris Uzdavinis (TeamB)
Guest
|
| Posted: Wed Mar 23, 2005 9:10 pm Post subject: Re: passing AnsiString to my class and program crawls :-( |
|
|
"James Christie" <james (AT) texmate (DOT) co.nz> writes:
| Quote: | Using RQ's profiler to get some numbers I find the average time to run
through my main loop of code parsing the data is 0.76 seconds!! If I remove
the creation of TMyData the average loop time reduces to just 0.002 seconds.
Any suggestions as to what is causing the slow down ??
|
What does the code in your TMyData(const AnsiString &data) constructor
actually do? You only showed what TTexData's constructor does, which
appears to b unrelated.
--
Chris (TeamB);
|
|
| Back to top |
|
|
James Christie
Guest
|
| Posted: Wed Mar 23, 2005 9:20 pm Post subject: Re: passing AnsiString to my class and program crawls :-( |
|
|
Hi Chris,
Sorry that was a typo.. I changed TTexData to TMyData for the post as
though it made it clearer that it was my data... I just missed it in one
place.
"Chris Uzdavinis (TeamB)" <chris (AT) uzdavinis (DOT) com> wrote
| Quote: | "James Christie" <james (AT) texmate (DOT) co.nz> writes:
Using RQ's profiler to get some numbers I find the average time to
run
through my main loop of code parsing the data is 0.76 seconds!! If I
remove
the creation of TMyData the average loop time reduces to just 0.002
seconds.
Any suggestions as to what is causing the slow down ??
What does the code in your TMyData(const AnsiString &data) constructor
actually do? You only showed what TTexData's constructor does, which
appears to b unrelated.
--
Chris (TeamB);
|
|
|
| Back to top |
|
|
James Christie
Guest
|
| Posted: Wed Mar 23, 2005 9:50 pm Post subject: Re: passing AnsiString to my class and program crawls :-( |
|
|
Hi Remy,
I might get back to you later, I've moved my profiler points and I'm now
unsure that's exactly where the problem is... thanks for the other points
though.
| Quote: | DataGrid->Objects[DataGrid->Col][DataGrid->Row] = new
TMyData(RegExList->Strings[i]); // ***** SLOW
You have a potential memory leak there. Make sure that you are freeing
any
previous entry before creating a new one.
|
Yep, freeing occurs elsewhere and the data in the cells isn't
overwritten ever.
| Quote: | DataGrid->Col++;
You cannot use the '++' operator with properties. You need to use the '='
and '+' operators instead:
DataGrid->Col = DataGrid->Col + 1;
|
It works OK like that, but I'll believe you and change it.
| Quote: | Using RQ's profiler to get some numbers I find the average time
to run through my main loop of code parsing the data is 0.76 seconds!!
If I remove the creation of TMyData the average loop time reduces to
just 0.002 seconds. Any suggestions as to what is causing the slow
down??
Did you try profiling your actual constructor code to find the specific
line that is causing the slowdown? |
I've tried that . I thought for a while that it was the calling the
constructor that was slowing it down as the profiler gave respectably short
time once inside the constructor.
James
|
|
| Back to top |
|
|
Chris Uzdavinis (TeamB)
Guest
|
| Posted: Wed Mar 23, 2005 10:33 pm Post subject: Re: passing AnsiString to my class and program crawls :-( |
|
|
"James Christie" <james (AT) texmate (DOT) co.nz> writes:
| Quote: | Hi Chris,
Sorry that was a typo.. I changed TTexData to TMyData for the post as
though it made it clearer that it was my data... I just missed it in one
place.
|
Doesn't profiling show where in the constructor the time is spent?
Well, if the bottleneck is in that constructor, then we can try to
optimize it. (But don't do this until you're certain that this is
really where the time is spent.)
I'd suggest you not catch exceptions in the constructor. If the
object fails to construct due to invalid input, the object probably
shouldn't exist in the first place. The way to indicate that is to
exit the constructor with an exception. This isn't just an
optimization, it's a usability and interface design question.
| Quote: | if (!numbers.Pos(data.AnsiLastChar()))
data.SetLength(data.Length() - 1);
|
This is a linear search, which is suboptimal (even though it's through
a short list.) Instead:
if (!isdigit(*data.AnsiLastChar()))
data.SetLength(data.Length() - 1);
| Quote: | // Data may have character at end
if (data.Pos(":"))
FValue = (TDateTime)data; // Data may be a date/time
|
Above is another linear scan through your string. I don't see how
this can be avoided, but it may be part of the bottleneck. I do not
know how fast or slow it is to construct a TDateTime, but that too
could be a source of your performance problem. I especially dislike
using a C-style cast, and on a non-pointer type it is generally a bad
idea. I'd prefer the code to explicitly create a temporary instead of
using a cast at all:
| Quote: | FValue = TDateTime(data); // Data may be a date/time
|
Notice it's just changing where the parenthesis are, but it has a
significantly cleaner meaning.
else FValue = data.ToDouble(); // Numeric
Again, this ToDouble function may have some overhead since it
validates and throws excpetions on bad input. Perhaps using atof()
would have faster results, though I don't know. It's worth a try,
though.
| Quote: | FValid = true;
}
catch(...) {
FValue = NULL;
FValid = false;
}
}
|
I also suggest getting rid of a "Valid" flag. If the object isn't
valid, its constructor should have failed with an exception. Then you
never need to check if an object is "valid", since by having an
instance, you know it's valid.
--
Chris (TeamB);
|
|
| Back to top |
|
|
James Christie
Guest
|
| Posted: Thu Mar 24, 2005 4:20 am Post subject: Re: passing AnsiString to my class and program crawls :-( |
|
|
OK now I remember what's happening.
I have two contructors for TMyData, one taking a refernece to an AnsiString,
the second a reference to a TDateTime object.
//---------------TMyData.h--------------------------------
class TMyData : public TObject {
public:
__fastcall TMyData (const AnsiString &data);
__fastcall TMyData (const TDateTime &data);
__property double Value = {read = FValue};
// __property bool Valid = {read = FValid};
private:
double FValue;
// bool FValid;
};
//----------------TMyData.cpp---------------------------------
static AnsiString numbers = "0123456789";
//---------------------------------------------------------------
__fastcall TMyData ::TMyData (const AnsiString &data) // Data usually
AnsiString
{
if (!isdigit(*data.AnsiLastChar())) data.SetLength(data.Length() - 1); //
Check Last Character
if (data.Pos(":")) FValue = (TDateTime)data; // Locale date time format
else FValue = data.ToDouble(); // Numeric
}
//---------------------------------------------------------------
__fastcall TMyData ::TMyData (const TDateTime &data) // Data ocassionally
DateTime
{
FValue = (TDateTime)data; // Locale date time format
}
//----------------------------------------------------------------
In my main program if I comment out the calls to the AnsiString
constructor the program flies, The constructors called using TDateTime have
no permorfmace impact.
If I comment out the inards of the AnsiString contructor
//---------------------------------------------------------------
__fastcall TMyData ::TMyData (const AnsiString &data) // Data usually
AnsiString
{
// if (!isdigit(*data.AnsiLastChar())) data.SetLength(data.Length() - 1);
// Check Last Character
// if (data.Pos(":")) FValue = (TDateTime)data; // Locale date time format
// else FValue = data.ToDouble(); // Numeric
}
//---------------------------------------------------------------
The program still crawls so the slow down is associated with the passing of
a const AnsiString reference to the constructor.
So is there a better way of passing a text string seems to be the
question.
cheers,
James
p.s it's Easter here in New Zealand so i wont be back here till next week.
|
|
| Back to top |
|
|
Chris Uzdavinis (TeamB)
Guest
|
| Posted: Thu Mar 24, 2005 6:13 pm Post subject: Re: passing AnsiString to my class and program crawls :-( |
|
|
"James Christie" <james (AT) texmate (DOT) co.nz> writes:
| Quote: | The program still crawls so the slow down is associated with the
passing of a const AnsiString reference to the constructor.
|
I have trouble believing that pasing anything by reference would be
the bottleneck. It must be something else.
--
Chris (TeamB);
|
|
| Back to top |
|
|
Alisdair Meredith [TeamB]
Guest
|
| Posted: Thu Mar 24, 2005 6:46 pm Post subject: Re: passing AnsiString to my class and program crawls :-( |
|
|
James Christie wrote:
| Quote: | //-----------------------TMyData.cpp---------------------------------
static AnsiString numbers = "0123456789";
__fastcall TTexData::TTexData(const AnsiString &data)
{
try {
if (!numbers.Pos(data.AnsiLastChar())) data.SetLength(data.Length()
- 1); // Data may have character at end
if (data.Pos(":")) FValue = (TDateTime)data; // Data may be a
date/time else FValue = data.ToDouble(); // Numeric
FValid = true;
}
catch(...) {
FValue = NULL;
FValid = false;
}
}
Using RQ's profiler to get some numbers I find the average time
to run through my main loop of code parsing the data is 0.76
seconds!! If I remove the creation of TMyData the average loop time
reduces to just 0.002 seconds. Any suggestions as to what is causing
the slow down ??
|
Have you tried profiling this constructor to see which lines are
responsible, rather than assuming it is the AnsiString itself?
There are slightly more efficient ways to determine if your last
character is a digit. Note: Since you pass your string by reference to
const, the compiler really should not let you truncate the string like
that! [I suspect you have a warning here, unless you have disabled it]
Parsing a string into a floating point number is not a trivial
operation, compared to simply passing a double. My guess is that the
data.ToDouble() line will probably show up when you profile the routine.
Note that calling new will probably involve locking a critical section,
as will calling SetLength() on the string. While these are relatively
cheap operations I would expect them to be more expensive than simply
passing a string by reference. I doubt they will show up if you
profile the routine though.
At the end of the day, until you profile this constructor to measure
the real impact of your code, you have very little idea where the true
cost lies.
And as a digression - I would make all your single-parameter
constructors explicit to prevent unexpected surprises. My preference
would also remove the __fastcall convention and the properties, but
those are more of a personal choice.
AlisdairM(TeamB)
|
|
| Back to top |
|
|
James Christie
Guest
|
| Posted: Mon Mar 28, 2005 10:22 pm Post subject: Re: passing AnsiString to my class and program crawls :-( |
|
|
Hi Chris,
I've found the cuplrit!
DataGrid->Objects[DataGrid->Col][DataGrid->Row] = new
TMyData(RegExList->Strings[i]); // ***** SLOW
It's the left hand side... My DataGrid->OnSelectCell event handler was
slowing everything down.
Thanks for your help,
James
"Chris Uzdavinis (TeamB)" <chris (AT) uzdavinis (DOT) com> wrote
| Quote: | "James Christie" <james (AT) texmate (DOT) co.nz> writes:
The program still crawls so the slow down is associated with the
passing of a const AnsiString reference to the constructor.
I have trouble believing that pasing anything by reference would be
the bottleneck. It must be something else.
--
Chris (TeamB);
|
|
|
| Back to top |
|
|
|
FAQ
Search
Memberlist
Usergroups
Register
Profile
Log in to check your private messages
Log in
