Code Review/Execution Speed tips

[adahn]

Hello all, I am completely new to these forums, and very new to C++. The first program I wrote (below) was a World of Darkness dice rolling program. I ran a few tests, 1 billion dice rolls took about 20 hours to complete. I am interested in learning techniques that could improve the speed of the program, just as an exercise in optimizing. Anyone mind taking a look? I've found some different changes I might enact like changing Dice = Dice + 1 to Dice++, and I am testing that now to see what kind of speed difference it makes, although I am skeptical that shortening this will have a significant or even noticeable difference.

#include "stdafx.h"
#include <iostream>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <conio.h>

using std::cout;
using std::cin;
using std::endl;


int main()
{
// Number of Dice to Roll
	int NumberOfDice; 

// Lower Bound Success requirement IE (SuccessNumber to 10 result in successes)
	int SuccessNumber; 

// Stored "random" integer 1 through 10.
	int RandomSeed;

// Stored amount of successes
	int Successes; 

// Lower bound reroll number, game mechanic for getting another roll
	int RerollNumber; 

// Part of the solution to repeating the program, I am guessing this is not the best..
        int done;


	done = 0;

	while (done == 0)
	{
		system("CLS"); // Non portable code, because I am a hack
		done = 0;
	Successes = 0;

	cout << "D10 Dice Roller \n"; // asks user for dice information
	cout << "Number of Dice: ";
	cin >> NumberOfDice;
	if (NumberOfDice >=1)
	{
	cout << "\n" << "Low Success Range (Usually 8): "; 
	cin >> SuccessNumber;
	}
	if (NumberOfDice >=1)
	cout << "\n" << "Low Reroll Range (Usually 10): ";
	cin >> RerollNumber;
	system("CLS");

	srand (time(NULL) ); // part of the random integer generator

	RandomSeed = rand() % 10 + 1; // calculates the initial chance die
	
	if (NumberOfDice <= 0)
	{
		cout << "Chance Die! You are so screwed!" << "\n" << "\n";
		cout << RandomSeed  << "\n" << "\n";
	
	if (RandomSeed == 1)
		{
			cout << "Critical Failure!";
			
		}
			
	if (RandomSeed == 10)
			{
				cout << "Success! Lucky Bastard. \n";
				
			}
	if (RandomSeed < 10 && RandomSeed != 1)
	{
			cout << "Failure!";
				
	}
	}

	while (NumberOfDice > 0) // while statement that rolls the dice
	{
	RandomSeed = rand() % 10 + 1;
	NumberOfDice = NumberOfDice - 1;
	if (RandomSeed >= SuccessNumber)
	{
		Successes = Successes + 1;
	}

cout << RandomSeed << ", ";
	if (RandomSeed == RerollNumber)
	{
	NumberOfDice = NumberOfDice + 1;
	}
	}
	
	cout << "\n";
	

	cout << "\n" << "Successes: " << Successes << "\n" << "\n";
	cout << "Roll again? (y/n) "; if (getch() != 'y') { done = 1; }
}
	return 0;
}

If you care to take a look and have a question about my undoubtedly sloppy stuff. I'll be sure to answer it. Thanks all, and I appreciate the site as a resource.

Edit: Oh and if someone inexperienced likes this simple wod dice roller, feel free to use any part of this.

[Cubbi]

You are correct in thinking that "changing Dice = Dice + 1 to Dice++" does not make a difference -- it is in fact exactly the same code.

Are you sure you're compiling for Release and not Debug (I'm assuming Visual Studio because of stdafx and conio)? Compiled with gcc 4.6.2 on Linux, your program took 2 minutes and 8 seconds to go through a billion dice rolls on my first-generation core i7.

[Gaminic]

How exactly did you time it? I imagine you didn't stick around for it to end, but I don't see any clocking code either. Since you have wait-for-input parts in the code, how did you test it?

[Cubbi]

@Gaminic

I commented out the getch() in the end and ran

 ~ $ time ./test > /dev/null
1000000000
8
10


real    2m7.896s
user    2m4.666s
sys     0m0.116s

[adahn]

"Are you sure you're compiling for Release [...]"

I'll try that. I am using a 2nd gen i5, I think. I will try compiling from release, yeah I am using Visual Studio.

"Took 2 minutes and 8 seconds"

WoW! That's crazy, I hope its your suggestion that made that difference. Haha.

"How exactly did you time it?"

I timed 1000 rolls, 10000 rolls, 100000 rolls (stopwatch), figured around ~80 microseconds per roll and multiplied that by 1 billion, did a little algebra and knew around about to get back to my computer the next day and check it. It was just under 20 hours.

The wait for input parts don't happen over and over, so I just set it to 1 billion and started it.


Edit: took 12 seconds for 100000 rolls. I am avoiding trying a billion because it'll be hard to tell the speed difference if it takes ~20 hours. (after building from release)

So it'd probably take 33.333333333333333333333333333333 hours? o.o; why?

[Gaminic]

Are you running this on a handheld calculator?

[adahn]

http://www.newegg.com/Product/Product.aspx?Item=N82E16819115072

That actually

[Gaminic]

...I think we can exclude your system as the reason for the slow execution time.

Did you disable the getch()? If it takes 2 seconds to execute but you wait 10 seconds to input something, it's going to say it took 12 seconds to complete.

[adahn]

I did, when I saw that he also did that, I tried building from release, when that didn't yield results, I commented out the looping function. It took almost an identical time.

[Gaminic]

I saw you're using Visual Studio. Any chance you're using a version that has a profiler? Might be interesting to see what the bottleneck is.

[adahn]

Good tip, it got me thinking. I think I figured it out; running it via console the speed was being choked by this bit of code:

cout << RandomSeed << ", ";

My guess: The console was only able to display so many lines at a time, so it was all held up by the displaying of the dice numbers. Just ran a billion quite quickly.

Edit: And while that solves the speed problem, it does set some sort of limit on display speed, which I now wonder if that can be overcome.