Fewer code lines, more space consumed. Why?

Projects Programming
1 

//void FlashLights() {
//  if (millis() > FlashTime) {
//    FlashTime = millis() + FlashInterval;
//    if (millis() < FlashExtTime) {
//      Flashing = true;
//      FlipFlop = !FlipFlop;
//      digitalWrite(SwPin[Ext], FlipFlop);
//      SwStatus[Ext]          = FlipFlop;
//      digitalWrite(SwPin[Sta], FlipFlop);
//      SwStatus[Sta]          = FlipFlop;
//      WiFiDrv::analogWrite(red, FlipFlop * 100);
//      WiFiDrv::analogWrite(blue, !FlipFlop * 100);
//    }
//    else {
//      Flashing = false;
//      WiFiDrv::analogWrite(blue, 0);
//      WiFiDrv::analogWrite(red, 0);
//    }
//  }
//}
void FlashLights() {
  if (millis() > FlashTime) {
    FlashTime = millis() + FlashInterval;
    if (millis() < FlashExtTime) {
      Flashing = true;
      FlipFlop = !FlipFlop;
      if (FlipFlop) {
        digitalWrite(SwPin[Ext], !SwBoolOn[Ext]);
        SwStatus[Ext]          = !SwBoolOn[Ext];
        digitalWrite(SwPin[Sta], !SwBoolOn[Sta]);
        SwStatus[Sta]          = !SwBoolOn[Sta];
        WiFiDrv::analogWrite(red, 0);
        WiFiDrv::analogWrite(blue, 200);
      }
      else {
        digitalWrite(SwPin[Ext], SwBoolOn[Ext]);
        SwStatus[Ext]          = SwBoolOn[Ext];
        digitalWrite(SwPin[Sta], SwBoolOn[Sta]);
        SwStatus[Sta]          = SwBoolOn[Sta];
        WiFiDrv::analogWrite(blue, 0);
        WiFiDrv::analogWrite(red, 200);
      }
    }
    else {
      Flashing = false;
      WiFiDrv::analogWrite(blue, 0);
      WiFiDrv::analogWrite(red, 0);
    }
  }
}

I was going thru a program to reduce the size, and thought, gee, I can make that one smaller, it flashes a light with a timer. Well the commented out portion actually resulted in a program about a hundred bytes larger.

How can that be? Is there some optimazation routine in the compiler causing this?

2

You've got avr-objdump - why not use it, and find out?

3

Additionally, how can you possibly expect anyone to provide a meaning full answer if you don't provide complete code, specify which Arduino board, or show us both the new and old code?

4

Never add milliseconds, only subtract.

If you need more memory, the best solution is just to pick up a board with more memory. Microoptimizations like this are pointless for hobby projects when boards with more memory are so cheap. If you're not at the limit yet, there's no point in optimizing code size, an unused byte of flash is a byte of flash wasted. If you really want to spend time on this, look at the generated code as suggested by Awol, but compilers are immensely complex, there's little point in trying to understand what's going on, unless there's a clear problem with the generated code, in which case you could report it to the GCC developers.

5

probably because I never heard of it. Would be fun to see.

6

I thought it might leave more for heap space.

Yeah, what's the best way to do the millisecond thing, I've been doing it this way for years but I admit I have seen it done differently.

7

I reasoned that since this what was making the difference the rest would be irrelevant.

8

The heap lives in RAM (dynamic memory), not flash/program storage.

Instead of checking millis() >= prevMillis + interval, use millis - prevMillis >= interval. The former fails when millis overflows, the latter is always correct.

2 Likes
9

when i compiled you code w/o WiFi the non-commented out code need 946 bytes, the commented code 934 and a 3rd version to invoked msec = millis() and used msec elsewhere needed 846

10

hi sevenoutpinball
it's a pity that nobody answers you, but leaves on other considerations, because I frequently asked myself this question, and I would have liked to have a beginning of answer. Forums are sometimes good, but participants should know how to read a simple question, and answer it with relevance. No, instead, they go off in all directions, when it's not reminding you of the rules because you took a wrong step.

Otherwise it is common that the memory size used by the compiler is not consistent with the changes you just added or subtracted. If a useful answer could come?

1 Like
11

Yeah. Right.

12

if (millis() >= FlashTime) {
FlashTime = millis() + FlashInterval;

is less preferable than

if (millis() - FlashTime > FlashInterval) {
FlashTime = millis();

Do I have this correct?

But then again, we are making it do math and compare which takes longer than compare alone.

On memory, the flash stuff is only instructions and all variables are in the stuff called dynamic memory. Ok. I admit I hadn't thought about it that far. Makes sense tho.

Thanks.

1 Like
13

Sure, ANYONE can provide a simple answer to a simple question. But, anybody experienced enough to provide an accurate / useful answer knows that compiler optimizations are extremely sophisticated and highly depend on both the nature of the complete program and the target processor it's being compiled for. So, if you don't understand why that information is required, then you definitely don't fit into that category.

14

How did you get it down to 846? I didn't understand that part.

15

this is the code i ran (of course i had to fill in a bunch of stuff


enum { Ext, Sta };

byte          SwPin    [] = { 13, 12 };
bool          SwBoolOn [] = { 0 };
bool          SwStatus [] = { 0 };

bool          Flashing      = true;
bool          FlipFlop;

unsigned long FlashExtTime  = 30000;
unsigned long FlashInterval = 250;
unsigned long FlashTime;

unsigned long msec;

#if 1
// 876 bytes of program storage, 16 bytes ram
void FlashLights() {
      msec = millis ();
      if (msec > FlashTime) {
            FlashTime = msec + FlashInterval;

            digitalWrite(SwPin[Sta], FlipFlop);
            FlipFlop = ! FlipFlop;
            digitalWrite(SwPin[Ext], FlipFlop);

#ifdef DBG
            Serial.println (__func__);
#endif
      }
}
#elif 0
// 890 bytes of program storage, 16 bytes ram
void FlashLights() {
      msec = millis ();
      if (msec < FlashTime) {
            FlashTime = msec + FlashInterval;
            if (msec < FlashExtTime) {
                  Flashing = true;
                  FlipFlop = !FlipFlop;

                  digitalWrite(SwPin[Ext], FlipFlop);
                  SwStatus[Ext]          = FlipFlop;

                  digitalWrite(SwPin[Sta], FlipFlop);
                  SwStatus[Sta]          = FlipFlop;
            }
            else {
                  Flashing = false;
            }
      }
}

#elif 0
// 934 bytes of program storage
void FlashLights() {
      if (millis() > FlashTime) {
            FlashTime = millis() + FlashInterval;
            if (millis() < FlashExtTime) {
                  Flashing = true;
                  FlipFlop = !FlipFlop;
                  digitalWrite(SwPin[Ext], FlipFlop);
                  SwStatus[Ext]          = FlipFlop;
                  digitalWrite(SwPin[Sta], FlipFlop);
                  SwStatus[Sta]          = FlipFlop;
            }
            else {
                  Flashing = false;
            }
      }
}

#else
// 946 bytes of program storage
void FlashLights() {
    if (millis() > FlashTime) {
        FlashTime = millis() + FlashInterval;
        if (millis() < FlashExtTime) {
            Flashing = true;
            FlipFlop = !FlipFlop;
            if (FlipFlop) {
                digitalWrite(SwPin[Ext], !SwBoolOn[Ext]);
                SwStatus[Ext]          = !SwBoolOn[Ext];
                digitalWrite(SwPin[Sta], !SwBoolOn[Sta]);
                SwStatus[Sta]          = !SwBoolOn[Sta];
            }
            else {
                digitalWrite(SwPin[Ext], SwBoolOn[Ext]);
                SwStatus[Ext]          = SwBoolOn[Ext];
                digitalWrite(SwPin[Sta], SwBoolOn[Sta]);
                SwStatus[Sta]          = SwBoolOn[Sta];
            }
        }
        else {
            Flashing = false;
        }
    }
}
#endif

void loop (void)
{
    FlashLights ();
}

void setup (void)
{
#ifdef DBG
    Serial.begin (9600);
#endif
    for (unsigned n = 0; n < sizeof(SwPin); n++)
        pinMode (SwPin [n], OUTPUT);
}
16

I'd usually use FlashTime += FlashInterval instead of FlashTime = millis(), it depends on how you want to handle overruns.

Sure, but one is correct and the other is not.

And given that you seem to be working with WiFi, I wouldn't lose sleep over a single subtraction ...

17

I pondered using += but in the case where the FlashTime is already very old, you wouldn't get now plus an interval, you would get the past plus an interval that might still be in the past.

18

Which might be exactly what you want. It preserves the average number of flashes per unit time, which is not the case is you use FlashTime = millis(), and the latter will start to drift over time.

19

I pity the fool!

a7

1 Like
20

great point.