Thread: Problem with progress code

Code:

while(1) {
      
      c = fgetc(fo);

      if(c == EOF){

        break;
      }
      else{
        c = c + 'c' + 'd' - 't';
        fputc(c,fw);
        cnt++;
        if(cnt % d == 0) {
          prog++;
        }
        printf(" %d%%\r", prog);
        fflush(stdout);
      }
    }
    printf(" %d%%\n", prog);
    printf("DONE!\n");

When the lines of code that are in red are removed, the program works very quickly, but when it's kept, it works extremely slowly.

This is part of a program that encrypts characters from a file and then outputs it into another file. I created this progress meter to show how long it takes when encrypting. When the file size is small, < 5kb, there is no problem. But when it's 3MB, then it takes upto 5 minutes when the progress meter is there, when I remove it, it takes only 5 seconds.

I don't see what the problem with the code is.

use print only when the new percent value is different from the printed earlier one
printf with fflush takes a long time - so make it only when really necesary

How do I do that ^ ?

I removed fflush, but that makes no difference.

Does your code "know" when progress has changed, by any chance?

--
Mats

I think it has to do with

Code:

if(cnt % d == 0) {
...

you're essentially calculating the cnt variable to zero with ==. Try just using =
hope that helps.

Originally Posted by MikeyIckey

I think it has to do with

Code:

if(cnt % d == 0) {
...

you're essentially calculating the cnt variable to zero with ==. Try just using =
hope that helps.

No, = is an assignment. I'm not trying to assign it, I'm trying to find a remainder with modulus.

Originally Posted by matsp

Does your code "know" when progress has changed, by any chance?

--
Mats

Yea, when cnt % d == 0

Code:

while(1) {
      
	c = fgetc(fo);

	if (c == EOF){
		break;
	}
	// As a matter of practice, you don't need an else here.
	else {
		c = c + 'c' + 'd' - 't';
		fputc(c,fw);
		cnt++;
		if (cnt % d == 0) {
			prog++;   // indicator changed... therefore...
			printf(" %d%%\r", prog);    // update indicator only here
			fflush(stdout);                 
		}
	}
}
printf(" %d%%\n", prog);
printf("DONE!\n");

You could also create another variable to hold this result since it never changes, and there's no need to recalculate it for EVERY STINKING CHARACTER.

Code:

int delta = 'c' + 'd' - 't';

Originally Posted by JFonseka

Yea, when cnt % d == 0

Right, so my point being that you then only need to print the updated progress when it changes, as Todd posted below - I wanted you to figure that part out yourself.

--
Mats

I can't believe I didn't see that mistake...

Thanks

Originally Posted by Todd Burch

You could also create another variable to hold this result since it never changes, and there's no need to recalculate it for EVERY STINKING CHARACTER.

Code:

int delta = 'c' + 'd' - 't';

Although the compiler can calculate the sum/difference of a few constants in that simple way. And even if it didn't, it would still disappear in the noise in a profile, compared to fgetc()/fputc() - I'm sure the processor can add/subtract in something like 3 clock-cycles (on a reasonably modern processor - 486 or better), whilst fgetc() is bound to take a few hundred cycles.

--
Mats

true.

Originally Posted by JFonseka

No, = is an assignment. I'm not trying to assign it, I'm trying to find a remainder with modulus.

sorry I was really exhausted when I posted that.