Optimizing for loop in c++

Question

1.00/5 (1 vote)

See more:

Hi Experts,

i have two for loop

C++

   for(y = Height; y--)
      {
        for(x = Width; x--)
         {
           im->setData(x, y, Getdata(x,y));
         }

      }

//definition for get and set data is as below

  T Getdata(unsigned int x, unsigned int y) const
   {
      return _Data[x * y];
   }
	
   void SetData(unsigned int x, unsigned int y, T data)
   {
      _Data[x + _width * y] = data;
   }

i want to optimize it further ,can somebody help me further...

What I have tried:

i read some where for(int j=Num;j--) is more efficient then for(int j=0;j

Posted 7-Feb-16 23:56pm

tasumisra

Add a Solution

Comments

Kornfeld Eliyahu Peter 8-Feb-16 6:09am

What kind of optimization you are looking for?
(there is no real difference between incremental and decremental loops in performance, but sometime the order is important)

tasumisra 8-Feb-16 6:34am

hi Kornfeld,
i am looking to optimise it for speed,basically i want to create some kind of hash table where it trying to call GetData each time...
any help would be appriciated

Thanks,
Vikas

Stefan_Lang 8-Feb-16 8:22am

This is not related to optimization, therefore I do not post this as solution, but I see some problems with your code:

0. The for loop syntax is incorrect, it requires three terms, not two.

1. GetData() looks like a member function and accesses the variable _Data, which looks like a member variable. But it is called like a global function. is there something missing?

2. in GetData() you use a different formula to determine an array index than in SetData(). It doesn't appear to make sense, since Getdata(1,8), Getdata(2,4), Getdata(4,2), and Getdata(8,1) would all return the same element, _Data[8] ! I suggest that in Getdata() you should use the same index expression as in Setdata().

3. There appear to be various alternate spellings of variables and member names (e. g. SetData vs. Setdata, _width vs. Width), but it isn't clear if these are meant to be different symbols or not.

Most of these issues wouldn't exist, if you at least tried to compile your code before posting it. it doesn't need to actually run, but these trivial errors make it unnecessarily hard for us to understand what you want to achieve.

Sergey Alexandrovich Kryukov 8-Feb-16 10:27am

Exactly. To optimize something, one needs something which works. The question simply makes no sense.
—SA

tasumisra 8-Feb-16 11:03am

Hello Stefan,
this is pseudo code and i was only expecting indicative solution\design..

but as you pointed out few things i ll try to look into this.

Thanks

tasumisra 8-Feb-16 11:53am

this is how implementation looks like

template<typename t="">
class Image
{
public:

void create(unsigned int width, unsigned int height)
{
_imageData = boost::shared_ptr<t>(new T[width * height]);
_width = width;
_height = height;
}

T getPixel(unsigned int x, unsigned int y) const
{
return _imageData[x * y];
}

void setPixel(unsigned int x, unsigned int y, T pixel)
{
_imageData[x + _width * y] = pixel;
}

// Return a sub-region of pixels
boost::shared_ptr<Image<t> > extractRegion(const Rectangle<t>& region) const
{
boost::shared_ptr<Image<t> > im = new Image<t>();

im->create(region.Width, region.Height);

for(unsigned int y = 0; y < region.Height; y++)
{
for(unsigned int x = 0; x < region.Width; x++)
{
im->setPixel(x, y, getPixel(region.X + x, region.Y + y));
}

}

return im;
}

private:

boost::shared_ptr<t> _imageData;

unsigned int _width;
unsigned int _height;

}

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Jochen Arndt · Answer 1 · 2016-02-08T00:48:00

Solution 3

There are two possible general optimisations:

Replace the function calls with the necessary code,
Pre-calculate intermediate results.

The resulting code may look like this:

C++

int z = Width * (Height - 1);
for (y = Height - 1; y >= 0; y--, z-= Width)
{
    for (x = Width - 1; x >= 0; x--)
    {
        dest[x + z] = src[x * y];
    }
}

Posted 8-Feb-16 0:48am

Jochen Arndt

Comments

nv3 8-Feb-16 15:23pm

My 5.

Aescleal · Answer 2 · 2016-02-08T00:22:00

Solution 2

If I were you I'd get the code working and consistent before messing about with optimising anything. Your Set and Gets don't access the same objects for example. Once you do that (and if _Data is an array like thingy) how about using std::copy to actually do the copy? If that's not fast enough and T is a POD look at using std::memcpy.

But fix your code first otherwise it'll be fast and wrong.

Posted 8-Feb-16 0:22am

Aescleal

Comments

tasumisra 8-Feb-16 6:32am

Basically this is a pseudo code where emphasis is on eliminating a for loop by creating some kind of lookup....

Aescleal 8-Feb-16 6:39am

In that case don't mess about with explicit loops, use std::copy or memcpy if that's not fast enough.

tasumisra 8-Feb-16 6:42am

Thanks Aescleal,I will try this option.