|
joshp1217 wrote: how I can improve the function so no problems dont arise when it is run?
Correctly using the standard one!
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
|
|
|
|
|
I have to improve this code for a test, so any help you can Provide I would greatly appreciate.
|
|
|
|
|
IMHO it is good as it stands.
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
|
|
|
|
|
So there is no way I can optimize it or make it more efficient, because when I looked at it it seemed fine to me to but I have to make sure.
|
|
|
|
|
IMHO it seems OK, i.e. It is correct. But maybe you can find a more efficient way to copy memory (e.g. copying machine-words instead of bytes).
I think someone else will give a better hint than me about.
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
|
|
|
|
|
1. copy DWORDs instead of bytes.
2. make sure those DWORDs are aligned on 32-bit boundaries.
and if you're willing to learn a lot about the working of your microprocessor,
3. use MMX/SSE, paying careful attention to caching, pipelines, etc.
|
|
|
|
|
Other than "for a test," why do you think the code needs to be changed? How do you know it's inefficient? Do you have any metrics that support this? What if you left it as is?
That said, the only change I'd suggest, but is really not required for your case, is:
while (size-- > 0)
{
*(char *) pTo = *(char *) pFrom;
pTo = (char *) pTo + 1;
pFrom = (char *) pFrom + 1;
}
"A good athlete is the result of a good and worthy opponent." - David Crow
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
|
|
|
|
|
Use
--size
instead of
size--
Prefixed operations always run faster and will improve the function's performance some, but maybe not a lot in this specific case.
|
|
|
|
|
Michael Sadlon wrote: Prefixed operations always run faster and will improve the function's performance...
You might want to qualify that a bit with:
For non-intrinsic types, prefixed operations always run faster and will improve the function's performance...
"A good athlete is the result of a good and worthy opponent." - David Crow
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
|
|
|
|
|
Hint. You could try making it safe. Thread Safe? Exception Safe? Bad parameter safe? All depends on who set the test but it is a test so I'm not going to give you all the answers just a few hints.
What happens when dest==src?
What happens if size < 0
What happens if size > the allocated memory dest points to.
What happens if src isn't null but isn't valid either.
Or...
On the other hand you could try making it faster! Make it work in machine words as suggested in the other comments and once you've got it compiled disassmeble it again and hand optimise the assembler. Then rewrite the function as an inline __delcspec(naked) function containing nothing but inline assmebler __asm{...}. Even faster perhaps you could replace it altogether with the Compiler intrinsic memcpy where the compiler writers already did this for you.
You see it all depends on what the person who set the test means by 'improve'.
In my opinion the ultimate memcpy would have all the bells and safety whistles, thorough checking at every point when built for Debug but when built in Release it would all boil away to that ultimately optimised piece of inline assmebly. Pull that off and you'll not need to pass any test.;)
Nothing is exactly what it seems but everything with seems can be unpicked.
|
|
|
|
|
I see what you are saying,
You dont want the dest == src because then you are performing a action that is not needed because they are already the same correct?
Yes, That's one case of an optimisation improvement but it's questionable whether you want to pay the price of checking every time against it only saving time when the user is being daft. A tricky one.
A much worse case is when src and dst are nearly the same for example when dst == src+2 but size == 10;
the docs for this sort of function sometimes refer to this sort of situation and say the result will be 'undefined'. Just another way of saying it will all go Pete Tong if you ask me.
Also size is an unsigned integral so it will never be less than zero
True eonugh so try a test where you write size=(size_t)(-1) and then call memcpy!
if size is greater than the allocated memory dest points to then basically you will be increment out of bounds which will cause an error, but how can you check to make sure the sizes match up?
It will only cause an error in the sense of a reported error if some piece of code detects that it has happened otherwise it will happily go ahead and read and write memory it shouldn't and often return just as if it had worked. If you want to prevent this you might need to add extra parameters, Microsoft do in their Safe CRT functions, and use the API memory check calls like IsBadWritePtr to check whether you have access to the memory being read and written.
What is an example of a src that isnt valid?
Once example is where src = 0x00000002. This is never going to be a valid pointer but it will pass a NULL check. It may seem that this could never happen but if someone gets size and src swapped over, size_t will happily take a 32bit pointer value and src will happily auto cast a size_t. Ow!
My point is that there is an almost endless amount of checking that you could add to a function like this but whether that 'improves' it is a matter of opinion. I think it does providing that it has no runtime cost in the Release build and that it doesn't obscure the purpose or operation of the function in such a way as to make maintenance difficult.
Even such an apparently trivial function can be a major challenge if looked at the right way. Most of us knock out hundreds of such functions all the time without a second thought but as they say 'once you've started digging it's much harder to find the bottom of the hole'.
Matthew F.
|
|
|
|
|
Many compilers recognise memcpy as an intrinsic function and it will be substituted by an efficient machine code implementation by the compiler, especially in a release build. I'd leave it alone.
Steve
|
|
|
|
|
i have this rn = (double) (n/rCount)*(a/n)*(b/n)*(c/n)*(d/n)*(e/n); in my code
with debug i see that n = 3 rCount =6
a = 2 b = 1 c = 2 d = 2 e = 2
but rn = 0.000 
what i do wrong?
int a=0,b=0,c=0,d=0,e=0,n=0,y = 0;
for (int i = 0 ; i < (rCount) ; i++)
{
if ( m_Array[i][5]== "N" || m_Array[i][5] =="n")
{
n = n +1;
if ( m_Array[i][0] == m_A)
{
a = a+1;
}
if ( m_Array[i][1] == m_B)
{
b = b+1;
}
if (m_Array[i][2] == m_C)
{
c = c +1;
}
if (m_Array[i][3] == m_D)
{
d = d +1;
}
if (m_Array[i][4] ==m_E)
{
e = e +1;
}
}
}
if (a ==0 || b ==0 || c==0 || d==0 || e ==0)
{
rCount = rCount*100;
}
if ( rn > ry)
{
m_R = "NO";
}
else if (ry > rn)
{
m_R = "YES";
}
CString strf;
strf.Format(_T("%lf"),rn);
AfxMessageBox(strf);
FileText.close;
|
|
|
|
|
When you divide integers only, the result will be an integer also (and then it will be casted to a double, but it is already rounded to an integer value). If at least one of the operand is a double, then the result will be a double.
So, you can simply multiply one of your integer by 1.0 in your formula (this will convert it to a double).
|
|
|
|
|
|
i did this
rn = (double) (n/rCount)*(a/n)*(b/n)*(c/n)*(d/n)*(e/n*1.0000);
but it still return me 0.000 :/
|
|
|
|
|
you have to do it for every division involving 2 integers... and no need to use 4 zeros; 1. will suffice 
|
|
|
|
|
Actually you have to do the trick for each of the factors [^]
(I also made a little test ).
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
|
|
|
|
|
Immunity18 wrote: rn = (double) (n/rCount)*(a/n)*(b/n)*(c/n)*(d/n)*(e/n);
cast should happen (in each factor) before division is performed
(otherwise integer division will result zero)
try :
<br />
double rn = ((double)n/rCount)*((double)a/n)*((double)b/n)*((double)c/n)*((double)d/n)*((double)e/n); <br />
<br />
Note that there is a smarter formulation of your expression:
<br />
double rn =(double)( a * b * c * d * e )/(rCount * n * n * n * n);<br />
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
|
|
|
|
|
thanks  and hm yes the second one is smarter
|
|
|
|
|
Hi there,
i have a problem with excel automation. I am using excel 2007 and Visual Studio 2005 with a dialog based MFC app. My code runs fine under excel 2000 but now i can't export any value to excel. Here now a code snippet of what i am doing:
pXL.CreateInstance(L"Excel.Application.12");
pXL->Visible[0] = VARIANT_TRUE;
WorkbooksPtr pBooks = pXL->Workbooks;
pBook = pBooks->Add((long)xlWorksheet);
pSheet = pXL->ActiveSheet;
pSheet->Name = "testname";
pSheet->Range["A1"]->Value = "1234";
this works fine to the pSheet->Range line. Excel opens, a new sheet is opened and this sheet is renamed to testname. But if i would write 1234 in the A1 excel cell a unhandled exception occurs and when i debug this the IDE jumps to the excel.tli file and displays this line:
#pragma implementation_key(1001)
inline void Excel::Range::PutValue ( const _variant_t & RangeValueDataType, const _variant_t & _arg2 ) {
_com_dispatch_method(this, 0x6, DISPATCH_PROPERTYPUT, VT_EMPTY, NULL,
L"\x000c\x080c", &RangeValueDataType, &_arg2);
}
Now i don't now what to do, in the RangeValueDataType is the value i would write to the A1 cell and in _arg2 is a VT_ERROR value. I have searched the web and google groups, all people there using this pSheet->Range function and they don't have any problems with it. Anybody out there an idea whats going wrong there?
TIA and best regards,
Frank
|
|
|
|
|
LeMauvais1089 wrote: pXL.CreateInstance(L"Excel.Application.12");
Do you have both Excel 2000 and Excel 2007 on the same machine?
"A good athlete is the result of a good and worthy opponent." - David Crow
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
|
|
|
|
|
No, i have got a new laptop, there is only excel 2007 installed on it.
Thanks for the response and best regards,
Frank
|
|
|
|
|
So what happens if you use pXL.CreateInstance(L"Excel.Application") instead?
"A good athlete is the result of a good and worthy opponent." - David Crow
"To have a respect for ourselves guides our morals; to have deference for others governs our manners." - Laurence Sterne
|
|
|
|
|
exactly the same, excel starts and crash at Range->Value. Perhaps there is something wrong in my initialisation? Here is again a code snippet from my header file:
#import "progid:Excel.Sheet.12" auto_search auto_rename rename_search_namespace("Office12")
using namespace Excel;
i must admit that my knowledge of com is minimal. Perhaps you can see something on the code above
Thanks for your help,
Frank
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
