How can I improve this Function
-
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.
-
I am getting back into C++(be doing C# heavey for the past few years), Can anyone give me some help on how I can improve the function so no problems dont arise when it is run? I appreciate the help you guys. void* memcpy( void* dest, void* src, size_t size ) { byte* pTo = (byte*)dest; byte* pFrom = (byte*)src; assert( dest != NULL && src != NULL ); while( size-- > 0 ) *pTo++ = *pFrom++; return (dest); } Thanks.
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
-
I am getting back into C++(be doing C# heavey for the past few years), Can anyone give me some help on how I can improve the function so no problems dont arise when it is run? I appreciate the help you guys. void* memcpy( void* dest, void* src, size_t size ) { byte* pTo = (byte*)dest; byte* pFrom = (byte*)src; assert( dest != NULL && src != NULL ); while( size-- > 0 ) *pTo++ = *pFrom++; return (dest); } Thanks.
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.
-
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.
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.
image processing toolkits | batch image processing | blogging
-
I am getting back into C++(be doing C# heavey for the past few years), Can anyone give me some help on how I can improve the function so no problems dont arise when it is run? I appreciate the help you guys. void* memcpy( void* dest, void* src, size_t size ) { byte* pTo = (byte*)dest; byte* pFrom = (byte*)src; assert( dest != NULL && src != NULL ); while( size-- > 0 ) *pTo++ = *pFrom++; return (dest); } Thanks.
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.
-
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
-
I am getting back into C++(be doing C# heavey for the past few years), Can anyone give me some help on how I can improve the function so no problems dont arise when it is run? I appreciate the help you guys. void* memcpy( void* dest, void* src, size_t size ) { byte* pTo = (byte*)dest; byte* pFrom = (byte*)src; assert( dest != NULL && src != NULL ); while( size-- > 0 ) *pTo++ = *pFrom++; return (dest); } Thanks.
Many compilers recognise
memcpyas 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
-
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.