-2
\$\begingroup\$

I tried this program in both Orwell DevC++ and Visual Studio 2013 (program works correctly). My astonishment is the performance of the same program in both IDEs. While the former took only 395 milliseconds, the latter took a whooping 34984 milliseconds. What's wrong with VS2013? Is it due to the compiler it uses? Or is it something else?

// Airport Simulation using vector
#include<iostream>
#include<chrono>
#include<random>
#include<vector>
using namespace std;

int time_unit=1;

int gen_rno(int lim)
{
    static default_random_engine dre (chrono::steady_clock::now().time_since_epoch().count());      // feeding a seed to dre so that everytime a different number is generated
    static uniform_int_distribution<int> uid {1,lim};
    return uid(dre);
}

struct flight
{
    int fno, fuel, ta, tl, td;  // ta : arrival time & d & l represent landing & departure
    static int f;
    flight()
    {
        fno=++f;
        fuel=gen_rno(25);   // generate random no
        ta=tl=td=time_unit;
    }
    void get_fno()
    {
        cout<<"Flight no: "<<fno<<" ";
    }

};
int flight :: f=0;

class runway
{
    vector<flight> lq, dq;      //lq : landing queue & dq: departure queue
    int count, crash, land, take_off;
    double tbal, tbld;      //tb : time interval between
    public:
        runway();
        void genq();    //generate queue
        void check_low_fuel();
        void landing();
        void departure();
        void fuel_reduction();
        void end_of_simulation();
};
runway :: runway()
{
        lq.reserve(10000), dq.reserve(1000);
        crash=land=take_off=tbal=tbld=0;
        int d=gen_rno(10), i;
        for (i=0;i<d;++i)
        dq.emplace_back();
}
void runway :: genq()
{
    int l=gen_rno(10);
    for (int i=0;i<l;++i)
    lq.emplace_back();
}
void runway :: check_low_fuel()
{
    int c=0;
    auto i=lq.begin();
    for (int j=0;j<lq.size();++j)
    {
        i=lq.begin()+j;
        if (i->fuel==0)
        {
            ++c;
            i->get_fno();
            cout<<"has crashed\n";
            lq.erase(i);
        }
        else if (i->fuel==1)
        {
            lq.emplace(lq.begin(),*i);
            lq.erase(i);
        }       
    }
    crash+=c;
    cout<<"No of crashes = "<<c<<"\n";
}
void runway :: fuel_reduction()
{
    for (int i=0;i<lq.size();++i)
    --lq.at(i).fuel;
}
void runway :: landing()
{
    count=0;
    for (int i=0;i<lq.size() && count<6;++i,++count)
    {
        lq.front().get_fno();
        lq.front().tl=time_unit;
        tbal+=lq.front().tl-lq.front().ta;
        cout<<"has landed\n";
        dq.emplace_back(lq.front());
        lq.erase(lq.begin());
        ++land;
    }
    cout<<"No of successful landings were = "<<count<<"\n";
}
void runway :: departure()
{
    if (count<6)
    {
        int x=count;
        for (int i=0;i<lq.size() && count<6;++i,++count)
        {
            dq.front().get_fno();
            dq.front().td=time_unit;
            tbld+=dq.front().td-dq.front().tl;
            cout<<"has taken off\n";
            dq.erase(dq.begin());
            ++take_off;
        }
        cout<<"No of successful departures were = "<<count-x<<'\n';
    }
}
void runway :: end_of_simulation()
{
    lq.shrink_to_fit(), dq.shrink_to_fit();
    double avg1=tbal/land, avg2=tbld/take_off;
    cout<<"\n\naverage time elapsed between arrival & landing is = "<<avg1<<"\naverage time elapsed between landing & take_off is = "<<avg2;
    cout<<"\nsuccessful landings = "<<land<<"\nsuccesful departures = "<<take_off<<"\ncrashes = "<<crash;
}

int main()
{
    auto t1=chrono::steady_clock::now();
    ios::sync_with_stdio(false);
    runway r;
    cout<<"\t\tAIRPORT SIMULATION\n\n";
    for (time_unit=1;time_unit<=1000;++time_unit)
    {
        cout<<"Time Unit: "<<time_unit<<"\n";
        r.genq();
        r.check_low_fuel();
        r.landing();
        r.departure();
        r.fuel_reduction();
        cout<<'\n';
    }
    r.end_of_simulation();
    auto t2=chrono::steady_clock::now();
    cout<<"\n"<<chrono::duration_cast<chrono::milliseconds>(t2-t1).count();
    return 0;
}
\$\endgroup\$

closed as unclear what you're asking by IEatBagels, TheCoffeeCup, Mast, Hosch250, Mathieu Guindon Nov 10 '15 at 4:33

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Are you compiling under Release mode with Full Optimizations enabled in VS2013? That sounds like Debug performance on VS. \$\endgroup\$ – user2296177 Nov 5 '15 at 15:14
  • 4
    \$\begingroup\$ So... Are you interested in getting your code peer reviewed (i.e. what this site is about), or you're asking where the performance difference is coming from (i.e. not what this site is about)? \$\endgroup\$ – Mathieu Guindon Nov 5 '15 at 15:48
4
\$\begingroup\$

Stop doing this:

using namespace std;

Yes it is in every book (they are trying to save space in the book). But for any program longer than 10 lines or is going to be read by somebody else this is a bad habit. Get out of this habit before you use it in an inappropriate situation.

To see full details:

Why is “using namespace std;” considered bad practice?

Global variables (or more specifically global mutable state is bad).

int time_unit=1;

It makes testing the program really hard. Because a function can do different things based on state outside the function it akes it really hard to find bugs etc. You should pass all the state a function as parameters to the function (or for methods parameters and members).

Function names should be meaningful.

int gen_rno(int lim)

This is not meaningful. Code should be self documenting. It costs you nothing and it makes reading the code that uses the function easier to read.

It is relatively standard for user defined types have an initial upper case letter. While objects (which include functions) have an initial lower case letter. Since types are really important in C++ this allows you to quickly see types in expressions.

struct flight
{

One line per variable.

    int fno, fuel, ta, tl, td;

You are doing nobody any favors by putting them on one line. But also going back to the self documenting code its hard to tell what the variables mean. Also by using meaningful names you remove the need to add comments to describe what the variable means.

Constructors should initialize all members (I think you do). But you should prefer to initialize them in the initializer list. It does not matter so much for integers (POD in general) but for any type that is more complicated it will definitely make a differentce.

    flight()
    {
        fno=++f;
        fuel=gen_rno(25);   // generate random no
        ta=tl=td=time_unit;
    }

As a simple example of the difference.

    class X
    {
         Y   y;
         Z   z;
         X()
              // Calls y default constructor here
              // Calls z default constructor here
         {
              y = 56;       // Calls y assignment operator
              z = "Plop";   // Calls z assignment operator.
         }


         X()
           : y(56)      // Calls y constructor
           , z("Plop")  // Calls z constructor
         {}
    };

Is this a print function?

    void get_fno()
    {
        cout<<"Flight no: "<<fno<<" ";
    }

The standard way to print an object in C++ is to define the operator<<. If not at least pass the stream you want to print the information too.

    void printFlight(std::ostream& s = std::cout) {
        s << "Flight No: << flightNumber << " ";
    }
    friend std::ostream& operator<<(std::ostream& str, Flight const& flight) {
        flight.printFlight(str);
        return str;
    }

Again self documenting member names:

    vector<flight> lq, dq;      //lq : landing queue & dq: departure queue

I would have written:

    std::vector<Flight>       arrivalQueue;
    std::vector<Flight>       departureQueue;

Not standard to put space around the ::

runway :: runway()

Also use the initializer list.

This is not the best way to loop over a container:

    auto i=lq.begin();
    for (int j=0;j<lq.size();++j) {
    }

In C++14

    for(auto const& flight: lq) {
    }

In C++11

    for(auto const& iter = lq.begin(); lq != lq.end(); ++lq) {
    }

In C++03

    for(std::vector<flight>::const_iterator iter = lq.begin(); lq != lq.end(); ++lq) {
    }

    // or 

    std::for_each(lq.begin(), lq.end(), DoStuff());
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.