Use of CString::GetBuffer()

[Nilesh]

My code is something like this:

<code_a>
CString strQuery;
strQuery = "select ABC from ABCTab WHERE "

POSITION list_pos = pList->GetHeadPosition();
while(list_pos)
{
ELM * pELM = pList->GetNext(list_pos);
strTemp.Format(" (Index = %d ") OR ", pELM->GetIndex());
strQuery+= strTemp;
}
..
//code to remove final 'OR' in the query.
<end code>

<end code>
As you can see, the length of 'strQuery' keeps on increasing.
Basically, if I keep on adding string_1 into another string string_2
in a loop, then string_2 will have to relocate itself for larger
buffer everytime. So to avoid this, what I do is, somehow allocate the
size of the string_2 some maximum value initially, so that this
unnecessary reloaction is avoided. For doing this, i use:

<code_b>
CString strQuery("");
strQuery.GetBuffer(32000 /*some max value*/)
strQuery.ReleaseBuffer();

strQuery += "select ABC from ABCTab WHERE ";

while(list_pos)
{
ELM * pELM = pList->GetNext(list_pos);
strTemp.Format(" (Index = %d ") OR ", pELM->GetIndex());
strQuery+= strTemp;
}
..
//code to remove final 'OR' in the query.
<end code>

My question is, will the code snippet b will run faster than code
snippet a? Is code snippet b doing what I intend to do? (i.e. avoid
the relocation of CString every time some other string is added to it,
by previously allocating some max value for it's buffer)

Thanks a lot,
Nilesh Dhakras.

[Ashish]

Nilesh,
Second code is equivalent to the first one as it does NOT allocate more
memory to the cstring object. GetBuffer simply returns the char* equivalent
of the cstring and 32,000 is the maximum length of that char*. For
allocating extra buffers to your cstring construct it using the overloaded
constructor like:
CString x ("",32000);
HTH
Ashish

"Nilesh" <dhak...@hotmail.com> wrote in message
news:ae9adc7c.03070...@posting.google.com...

[José Lamas Ríos]

"Ashish" <a...@hotmail.com> wrote in message
news:%234ZJvRW...@TK2MSFTNGP10.phx.gbl...

> Nilesh,
> Second code is equivalent to the first one as it does NOT allocate more
> memory to the cstring object. GetBuffer simply returns the char*
equivalent
> of the cstring and 32,000 is the maximum length of that char*.

Hmm..., no. The parameter passed to GetBuffer() specifies a minimun
length (in TCHARs, BTW).

From CString::GetBuffer() documentation:

[quote]
LPTSTR GetBuffer( int nMinBufLength );
throw( CMemoryException );

Return Value

An LPTSTR pointer to the object’s (null-terminated) character buffer.

Parameters

nMinBufLength

The minimum size of the character buffer in characters. This value does
not include space for a null terminator.
[/quote]

You can even look at MFC's source code to confirm this.

So, Nilesh's code (<code_b>) is a valid way to reduce memory
reallocations.

--
jlr

[Ashish]

Ya whatever but still u need to use constructor to create more space..
Ashish
"José Lamas Ríos" <j...@artech.com.uy> wrote in message
news:eeNYgyXQ...@TK2MSFTNGP12.phx.gbl...

[Ashish]

I dont agree with u, cuz getbuffer does NOT allocate memory. It returns the
char* equivalent, which is temperory and should not be tampered with. You
can check the same in MSDN. So both code are equivalent.
Ashish

"José Lamas Ríos" <j...@artech.com.uy> wrote in message
news:eeNYgyXQ...@TK2MSFTNGP12.phx.gbl...

[Igor Tandetnik]

"Ashish" <a...@hotmail.com> wrote in message

news:%23xy0z9X...@TK2MSFTNGP12.phx.gbl...

> I dont agree with u, cuz getbuffer does NOT allocate memory. It
returns the
> char* equivalent, which is temperory and should not be tampered with.
You
> can check the same in MSDN. So both code are equivalent.

Huh? The whole point of CString::GetBuffer is to provide a buffer you
can manipulate directly. MSDN documentation is crystal clear:

[GetBuffer] Returns a pointer to the internal character buffer for the
CString object. The returned LPTSTR is not const and thus allows direct
modification of CString contents.

Do you maybe confuse CString::GetBuffer with std::string::c_str() ?
--
With best wishes,
Igor Tandetnik

"For every complex problem, there is a solution that is simple, neat,
and wrong." H.L. Mencken

[Ian Semmel]

You can have something like

CString strQuery = "select ABC from ABCTab WHERE ";

int n = strQuery.GetLength ();

LPTSTR ps = strQuery.GetBuffer (32000);

This allocates a buffer of the correct length and copies your null
terminated string to it

LPTSTR px = ps + n;

and in your loop you can build the string (checking for overflow etc) by

lstrcpy ( px, strTemp );
px += strTemp.GetLength ();

after you've finished

strQuery.ReleaseBuffer ();
(you will have to adjust to delete the final OR by putting '\0' 3 chars
back.

[Joseph M. Newcomer]

This is a pretty bad way of doing it. Instead, only put the OR in if needed!

CString OR = _T("");

{...loop...
CString t;
t.Format(_T("%s(Index == %d"), OR, pELM->GetIndex);
OR = _T(" OR ");
strQuery += t;
}

No need to go back and eliminate something that should not have been there in the first
place!
joe

Joseph M. Newcomer [MVP]
email: newc...@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm

[Joseph M. Newcomer]

WRONG! GetBuffer most DEFINITELY will allocate memory, if the size given is larger than
the current CString buffer. The only condition under which it will not allocate memory is
if the size given is already smaller than the buffer size (which may be larger than the
contents of the string, in characters). And as far as "tampering" with it, that is WHY
you call GetBuffer, to get a modifiable buffer!
joe

Joseph M. Newcomer [MVP]

[Joseph M. Newcomer]

I should also point out that your concerns about performance are probably irrelevant. The
total change in performance between the simple model and the overly complex one will be
masked entirely by the overhead if the query requires just ONE disk access! So this is
optimizing something which does not need to be optimized. For a tiny example like this,
even if you have a thousand entries in the list, you would probably not see a
statistically significant change in the performance. Also, you are presuming that a +=
always requires an allocation, which is not true; CString buffers are quantized (in the
release version) to 64, 128, 256 and 512 byte chunks, so a += does not necessarily do a
reallocation. This is a typical example of misplaced enthusiasm.

Read my essay "Optimization: your worst enemy" on my MVP Tips site.

Also, I posted an earlier response showing how to avoid having to remove the final OR.
That's just silly. However, I've been through three different news servers today, so I'm
not sure if it has made it out. I haven't seen it yet (the joys of changing ISPs)
joe

On 3 Jul 2003 04:30:18 -0700, dhak...@hotmail.com (Nilesh) wrote:

Joseph M. Newcomer [MVP]

[Alexander Grigoriev]

I'd advise to always cast CString to LPCTSTR, when passing to Format, or
other printf-like functins.

In this case, though, 'OR' is better declared as LPCTSTR.

"Joseph M. Newcomer" <newc...@flounder.com> wrote in message
news:4i29gv0jonhov45bd...@4ax.com...

[Nilesh]

Joseph M. Newcomer <newc...@flounder.com> wrote in message

> I should also point out that your concerns about performance are probably >irrelevant. The
> total change in performance between the simple model and the overly complex >one will be
> masked entirely by the overhead if the query requires just ONE disk access! >So this is
> optimizing something which does not need to be optimized. For a tiny example >like this,
> even if you have a thousand entries in the list, you would probably not see a
> statistically significant change in the performance.

For small no of entries, yes, there is not significant change. But i
tested it for 64000 entries in my list and I got 2.5X performance
improvement for this query formation.

>Also, you are presuming that a +=
> always requires an allocation, which is not true; CString buffers are >quantized (in the
> release version) to 64, 128, 256 and 512 byte chunks, so a += does not >necessarily do a
> reallocation.

Yes, I agree that not every += operation requires reallocation. In
fact, as u said, buffers are quantized to 64,128 and so on, as the no
of += operations increase, less reallocation needs to be done
(assuming that I am always adding a string of constant length)

>This is a typical example of misplaced enthusiasm.

The point is not that I can have optimization this way everywhere. The
point is, there are many places where such query formation needs to be
done and if we use such technique, we can have some improvement. I
dont think that there is anything bad in it.

Thanks a lot for your reply.
Nilesh Dhakras.

[Johan Rosengren]

Nilesh,

"Nilesh" <dhak...@hotmail.com> wrote in message
news:ae9adc7c.03070...@posting.google.com...

> Joseph M. Newcomer <newc...@flounder.com> wrote in message
> > I should also point out that your concerns about performance are
probably >irrelevant. The

<snip>

> For small no of entries, yes, there is not significant change. But i
> tested it for 64000 entries in my list and I got 2.5X performance
> improvement for this query formation.
>

The resulting query to the database will probably take *far* longer time
than the assembly of the question. From my experience, the small overall
gains from "microoptimizations" are nothing compared to a larger structural
analysis - something that can yield hefty gains in running time of an app.

Johan Rosengren
Abstrakt Mekanik AB

<snip>

[Joseph M. Newcomer]

Not necessary. Never has been necessary. Waste of characters.
joe

[Joseph M. Newcomer]

Right on! The 2.5X performance increase on forming a query that has 64000 OR-clauses is
probably masked by the cost of transmitting such a massive query to SQL server. I shudder
to think about how long it takes to execute the query. Especially on a database of
reasonable size (a few tens of gigabytes; otherwise, it wouldn't make any sense to have
64000 OR-clauses)

The difference between mere programming and engineering is understanding where the time or
space really goes, and where the real costs (usually development costs, debugging costs,
and maintenance costs) are. By creating a "more efficient" algorithm for solving an
uninteresting problem, you create code that is needlessly complex.

Example:

* If you get a hit on the L1 cache, it costs you 0 clock cycles.
* If you get an L1 miss and you get an L2 cache hit, it costs you 1 or 2 clock cycles,
depending on your chipset (full-speed or half-speed cache)
* If you get an L2 miss and have to go to memory, it costs you 20 clock cycles.
* If you take a page fault, it costs you 20,000,000 clock cycles.

Unless you are doing some sort of array processing where taking advantage of cache line
size and replacement algorithm can give you 10x-20x local speed performance improvement,
you are in more danger from a page fault; it will swamp by five orders of magnitude or so
the gains from optimizing a few thousand instructions out of the loop. And note that doing
cache-line-strategy optimizations generally only works on one CPU model, and one support
chipset, and has to be redone for each new CPU type and each new chipset.

Maybe it's the "old guys" like Johan and I who have learned that most of the
"optimizations" that young programmers do are completely misdirected efforts. We keep
trying to explain this, but nobody seems to listen. I attribute a lot of it to the
macho-programmer-mine-is-smaller-than-yours attitude; I have spent years breaking new
programmers of these thought processes. I attribute it to poor programming techniques
taught in schools.

Structure will kill you by orders of magnitude when your statement arrangement is
perfectly fine and "efficient". Spend your time optimizing structure, and get that right.
For example, if your SQL query needs 64000 OR-clauses, you will get orders of magnitude
performance by reorganizing the database, or using a different kind of query. Only after
you have the structure right is it worth worrying about individual subroutines.

2.5X of what absolute time, by the way? And in the running of the program, what percentage
of the total execution time is this? I once wasted 13 hours of CPU time (on a mainframe,
that tracked our CPU usage) finding a bug caused by a cute programmer zeroing two 16-bit
pointers by putting them in a union with a 32-bit DWORD, and storing 0 to the DWORD. In
the equivalent of the Win16-to-Win32 conversion, where pointers became 32 bits, this trick
zeroed only one of the pointers. (OK, it was 18 bit pointers and 36 bit pointers, it was
1977, on a PDP-10, in a language not many people have used). I confronted the programmer,
and he looked shocked at my criticism. "Why, I SAVED ONE INSTRUCTION!" he declaimed,
self-righteously. That was, on those machines, 1 microsecond. In the *initialization* of a
structure, of which we used about six in the program. He saved SIX MICROSECONDS PER
PROGRAM! I computed, given the normal runtime of this program, that we would have to run
it CONTINUOUSLY, as-soon-as-it-stopped-start-it-over-again, for ***14 years*** to recover
the CPU time I had spent recompiling, re-running, and debugging in that one day. So it
matters not one little bit that your algorithm is 2.5X faster until that saving is
expressed in exact milliseconds, and compared to the total running time of the program in
a real context (the cost of evaluating a query of 64000 OR-clauses to SQL server counts as
part of the total cost of the execution). If this is expressible in more than
single-digit-integer parts per million improvement, I will be very surprised. And if you
aren't doing 64K OR-clauses in the normal course of doing a query, what made you think
that measuring such a large example told you anything useful about the cost of an
algorithm? I have used O(n**3) algorithms because I knew that n rarely exceeded 2 (and
never exceeded 5). Why spend a lot of effort developing an n log n or linear algorithm
when the simple n-cubed algorithm (which I wrote in a couple hours) did the job so fast it
had no impact on the total running time?

You optimize what matters. But first, you have to decide what matters.
joe

Joseph M. Newcomer [MVP]

[Alexander Grigoriev]

C++ standard, chapter 5.2.2 clause 7 (passing arguments to a function with
ellipsis):

"If the argument has a non-POD class type (clause 9), the behavior is
undefined."

CString is non-POD (plain old data).

It just happens to work in expected way. Nothing (except for fear of
breaking every MFC program in the world) prohibits MS from changing this.

"Joseph M. Newcomer" <newc...@flounder.com> wrote in message

news:rr8agvk6d661bavbe...@4ax.com...

[Russ]

I'd like to point out something related to this that I discovered
today. In VC++ .net, ReleaseBuffer does not work the same way as in
version 6. If you call ReleaseBuffer with no parameter, it does a
strlen on the contents and attempts to set the CString size to the
measured length. However this can only be done if the string is
terminated. Normally CStrings are not terminated so problems can
arise. For instance:

CString s;

char* p = s.GetBuffer (32);

memcpy (p, buf, 32); // buf contains some character data

s.ReleaseBuffer ();

The ReleaseBuffer code attempts to do a strlen on the contents, but
since the data is not terminated it gets a length longer than the size
that was set with GetBuffer, and asserts.

The above WILL work with VC++ 6.0. I didn't check the code to see why.

The solution is to use the length when releasing:

s.ReleaseBuffer (32);

Hope this is helpful..

Regards, Russ

[Russ]

On Fri, 04 Jul 2003 02:49:58 -0400, Joseph M. Newcomer
<newc...@flounder.com> wrote:

>Not necessary. Never has been necessary. Waste of characters.
> joe

Joe. I have tons of code with CStrings cast to LPCSTR because at one
time it was necessary. Somewhere along the line some VC++ service
pack fixed the problem and it is no longer necessary. I remove the
casts whenever I am working on a section of code and notice them.

The cast was only needed if the CString was not the first CString in
the format list:

CString s1, s2;
str.Format ("test1: %s, test2: %s", s1, s2);

s2 needed casting, s1 did not.

Regards, Russ

[Doug Harrison [MVP]]

Russ wrote:

>I'd like to point out something related to this that I discovered
>today. In VC++ .net, ReleaseBuffer does not work the same way as in
>version 6. If you call ReleaseBuffer with no parameter, it does a
>strlen on the contents and attempts to set the CString size to the
>measured length.

So does VC6.

>However this can only be done if the string is
>terminated. Normally CStrings are not terminated so problems can
>arise. For instance:
>
>CString s;
>
>char* p = s.GetBuffer (32);
>
>memcpy (p, buf, 32); // buf contains some character data
>
>s.ReleaseBuffer ();
>
>The ReleaseBuffer code attempts to do a strlen on the contents, but
>since the data is not terminated it gets a length longer than the size
>that was set with GetBuffer, and asserts.
>
>The above WILL work with VC++ 6.0. I didn't check the code to see why.
>
>The solution is to use the length when releasing:
>
>s.ReleaseBuffer (32);

VC6 is documented to behave the same way. Here's the relevant VC6 source
code which verifies it behaves as documented (I added the default argument,
which does appear in the function declaration):

void CString::ReleaseBuffer(int nNewLength = -1)
{
CopyBeforeWrite(); // just in case GetBuffer was not called

if (nNewLength == -1)
nNewLength = lstrlen(m_pchData); // zero terminated

So what you show above "worked" merely by chance in VC6. Possibly more than
32 bytes were allocated to the string, and the 33rd byte happened to be a
zero. Possibly there was a buffer overrun. This illustrates an important
point. Code which appears to work may contain latent bugs. You always need
to "check the code", or at least the documentation, and preferably both. :)

--
Doug Harrison
Microsoft MVP - Visual C++

[Joseph M. Newcomer]

Microsoft carefully designed CString so that it was equivalent to LPCTSTR. Because of this
careful design, no cast is required.
joe

[Alexander Grigoriev]

I mean, the vlist-argiment call convention may be changed, for example, to
perform copy constructor and destructor (which is more appropriate, IMHO, I
was quite surprised that VC doesn't do it). Then the code without a cast
would add extra copy-ctor/dtor call pair.

"Joseph M. Newcomer" <newc...@flounder.com> wrote in message

news:kcobgvk69cucbm97b...@4ax.com...

[Russ]

My response at end:

No. I used that a lot and it never failed under any circumstance in
VC6.0. The first time I tried it in 7.0 it failed. So I looked to
see what the difference is. In both cases an extra byte is allocated
at the end of the CString. One more byte than was asked for in the
GetBuffer call, that is. The difference is that in version 6, that
extra byte is set to NULL. In version 7 it is left unchanged (has a
value of 0xcd in debug). So the string length test will always fail
unless the caller terminates the string before calling ReleaseBuffer
with no parameter (defaults to -1).

The version 6 code is in StrCore.cpp at line 514:

m_pchData[nNewLength] = '\0';

This step is missing from the version 7 code.

However, I just reviewed the version 6 documentation for GetBuffer and
Release buffer. It is obvious that I have been relying on
undocumented behaviour (which I did not realize). However it was not
a random thing as to whether it worked or not.

Regards, Russ

[Doug Harrison [MVP]]

Russ wrote:

>No. I used that a lot and it never failed under any circumstance in
>VC6.0. The first time I tried it in 7.0 it failed. So I looked to
>see what the difference is. In both cases an extra byte is allocated
>at the end of the CString. One more byte than was asked for in the
>GetBuffer call, that is. The difference is that in version 6, that
>extra byte is set to NULL. In version 7 it is left unchanged (has a
>value of 0xcd in debug). So the string length test will always fail
>unless the caller terminates the string before calling ReleaseBuffer
>with no parameter (defaults to -1).
>
>The version 6 code is in StrCore.cpp at line 514:
>
> m_pchData[nNewLength] = '\0';

That doesn't help, as nNewLength is determined from lstrlen when you call
ReleaseBuffer(-1).

>This step is missing from the version 7 code.

No, it just moved into the SetLength function. What you're saying does
appear to be true for VC6's AllocBuffer but not VC7's PrepareWrite, which
GetBuffer calls, and it was AllocBuffer that was saving you in VC6. But see
below.

>However, I just reviewed the version 6 documentation for GetBuffer and
>Release buffer. It is obvious that I have been relying on
>undocumented behaviour (which I did not realize). However it was not
>a random thing as to whether it worked or not.

What you were doing could still have failed in strange ways if you ever did
a GetBuffer(x) with x <= the current allocation (which is not at all hard to
imagine real code doing). In that case, VC6's GetBuffer does not set the
byte at offset x to zero. Thus, if you failed to zero-terminate,
ReleaseBuffer(-1) would tend to store the wrong length; the length would
tend to remain the same.

[Russ]

I understand that, but did not want to go into all the ramifications.
In actuality I was doing this in a routine I called
MakeCStringFromBuffer (UINT len), where len is the size of the buffer
which usually contained a string, right padded with spaces but no
terminator.

MakeCStringFromBuffer called CString::GetBuffer for the size of the
buffer, copied the buffer into it, and then looks backwards from the
end, for the first non space character. If the first non space
character was not the last char in the buffer, it put a null in the
following position, then called ReleaseBuffer with no parameter. That
worked fine, but the problem comes when there are no spaces at the end
of the string. In that case no null could be appended. But the
routine still worked and I did not noticed the discrepancy since the
terminate routine was written years before I decided to call it from
MakeCStringFromBuffer. But with version 6 code the safety catch of
nulling the end of the string when it was allocated allowed it to work
anyway. In version 7, the first time I called it with a string that
totally filled the buffer an exception was thrown for the reasons
outlined earlier.

There are several ways to properly fix my routine but in my original
post I only wanted to point out that calling release buffer without a
parameter had to be done with more care with version 7 code than with
version 6.

Regards, Russ

[David Crow [MCSD]]

Check again. CString::GetBuffer() indeed makes a call to the memory manager
via CString::AllocBuffer(). The smallest chunk that it allocated is 64
bytes, so even if 63 one-byte calls were made to CString::GetBuffer(),
CString::AllocBuffer() would only get called once. In other words, the
CString object is, at worst case, able to "grow" by 63 bytes at a time
without the need to ask for more memory.

"Ashish" <a...@hotmail.com> wrote in message

news:#xy0z9XQ...@TK2MSFTNGP12.phx.gbl...

[Joseph M. Newcomer]

Note the quantized allocation only happens in the non-debug (release) versions of the
code; the debug version will allocate exact sizes (shows up errors more quickly).

e.g., strcore.cpp, lines 93..115 are under control of #ifndef _DEBUG
joe
On Thu, 03 Jul 2003 19:22:45 GMT, "David Crow [MCSD]" <david.crow....@pbsnow.com>
wrote:

Joseph M. Newcomer [MVP]