Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In search for RAM to save, why use ints instead of defines? #1849

Closed
TD-er opened this issue Aug 6, 2022 · 10 comments
Closed

In search for RAM to save, why use ints instead of defines? #1849

TD-er opened this issue Aug 6, 2022 · 10 comments
Assignees
Labels
more info Pending Confirmation Waiting for confirmation from user

Comments

@TD-er
Copy link

TD-er commented Aug 6, 2022

In my endless quest to save RAM in my project (ESPEasy) I came across this section of the code:

const uint16_t kNoRepeat = 0;
const uint16_t kSingleRepeat = 1;
const uint16_t kAirtonBits = 56;
const uint16_t kAirtonDefaultRepeat = kNoRepeat;
const uint16_t kAirwellBits = 34;
const uint16_t kAirwellMinRepeats = 2;
const uint16_t kAiwaRcT501Bits = 15;
const uint16_t kAiwaRcT501MinRepeats = kSingleRepeat;
const uint16_t kAlokaBits = 32;
const uint16_t kAmcorStateLength = 8;
const uint16_t kAmcorBits = kAmcorStateLength * 8;
const uint16_t kAmcorDefaultRepeat = kSingleRepeat;
const uint16_t kArgoStateLength = 12;
const uint16_t kArgoBits = kArgoStateLength * 8;
const uint16_t kArgoDefaultRepeat = kNoRepeat;
const uint16_t kArrisBits = 32;
const uint16_t kBosch144StateLength = 18;
const uint16_t kBosch144Bits = kBosch144StateLength * 8;
const uint16_t kCoolixBits = 24;
const uint16_t kCoolix48Bits = kCoolixBits * 2;
const uint16_t kCoolixDefaultRepeat = kSingleRepeat;
const uint16_t kCarrierAcBits = 32;
const uint16_t kCarrierAcMinRepeat = kNoRepeat;
const uint16_t kCarrierAc40Bits = 40;
const uint16_t kCarrierAc40MinRepeat = 2;
const uint16_t kCarrierAc64Bits = 64;
const uint16_t kCarrierAc64MinRepeat = kNoRepeat;
const uint16_t kCarrierAc128StateLength = 16;
const uint16_t kCarrierAc128Bits = kCarrierAc128StateLength * 8;
const uint16_t kCarrierAc128MinRepeat = kNoRepeat;
const uint16_t kCoronaAcStateLengthShort = 7;
const uint16_t kCoronaAcStateLength = kCoronaAcStateLengthShort * 3;
const uint16_t kCoronaAcBitsShort = kCoronaAcStateLengthShort * 8;
const uint16_t kCoronaAcBits = kCoronaAcStateLength * 8;
const uint16_t kDaikinStateLength = 35;
const uint16_t kDaikinBits = kDaikinStateLength * 8;
const uint16_t kDaikinStateLengthShort = kDaikinStateLength - 8;
const uint16_t kDaikinBitsShort = kDaikinStateLengthShort * 8;
const uint16_t kDaikinDefaultRepeat = kNoRepeat;
const uint16_t kDaikin2StateLength = 39;
const uint16_t kDaikin2Bits = kDaikin2StateLength * 8;
const uint16_t kDaikin2DefaultRepeat = kNoRepeat;
const uint16_t kDaikin64Bits = 64;
const uint16_t kDaikin64DefaultRepeat = kNoRepeat;
const uint16_t kDaikin160StateLength = 20;
const uint16_t kDaikin160Bits = kDaikin160StateLength * 8;
const uint16_t kDaikin160DefaultRepeat = kNoRepeat;
const uint16_t kDaikin128StateLength = 16;
const uint16_t kDaikin128Bits = kDaikin128StateLength * 8;
const uint16_t kDaikin128DefaultRepeat = kNoRepeat;
const uint16_t kDaikin152StateLength = 19;
const uint16_t kDaikin152Bits = kDaikin152StateLength * 8;
const uint16_t kDaikin152DefaultRepeat = kNoRepeat;
const uint16_t kDaikin176StateLength = 22;
const uint16_t kDaikin176Bits = kDaikin176StateLength * 8;
const uint16_t kDaikin176DefaultRepeat = kNoRepeat;
const uint16_t kDaikin200StateLength = 25;
const uint16_t kDaikin200Bits = kDaikin200StateLength * 8;
const uint16_t kDaikin200DefaultRepeat = kNoRepeat;
const uint16_t kDaikin216StateLength = 27;
const uint16_t kDaikin216Bits = kDaikin216StateLength * 8;
const uint16_t kDaikin216DefaultRepeat = kNoRepeat;
const uint16_t kDaikin312StateLength = 39;
const uint16_t kDaikin312Bits = kDaikin312StateLength * 8;
const uint16_t kDaikin312DefaultRepeat = kNoRepeat;
const uint16_t kDelonghiAcBits = 64;
const uint16_t kDelonghiAcDefaultRepeat = kNoRepeat;
const uint16_t kTechnibelAcBits = 56;
const uint16_t kTechnibelAcDefaultRepeat = kNoRepeat;
const uint16_t kDenonBits = 15;
const uint16_t kDenon48Bits = 48;
const uint16_t kDenonLegacyBits = 14;
const uint16_t kDishBits = 16;
const uint16_t kDishMinRepeat = 3;
const uint16_t kDoshishaBits = 40;
const uint16_t kEcoclimBits = 56;
const uint16_t kEcoclimShortBits = 15;
const uint16_t kEpsonBits = 32;
const uint16_t kEpsonMinRepeat = 2;
const uint16_t kElectraAcStateLength = 13;
const uint16_t kElectraAcBits = kElectraAcStateLength * 8;
const uint16_t kElectraAcMinRepeat = kNoRepeat;
const uint16_t kEliteScreensBits = 32;
const uint16_t kEliteScreensDefaultRepeat = kSingleRepeat;
const uint16_t kFujitsuAcMinRepeat = kNoRepeat;
const uint16_t kFujitsuAcStateLength = 16;
const uint16_t kFujitsuAcStateLengthShort = 7;
const uint16_t kFujitsuAcBits = kFujitsuAcStateLength * 8;
const uint16_t kFujitsuAcMinBits = (kFujitsuAcStateLengthShort - 1) * 8;
const uint16_t kGicableBits = 16;
const uint16_t kGicableMinRepeat = kSingleRepeat;
const uint16_t kGoodweatherBits = 48;
const uint16_t kGoodweatherMinRepeat = kNoRepeat;
const uint16_t kGreeStateLength = 8;
const uint16_t kGreeBits = kGreeStateLength * 8;
const uint16_t kGreeDefaultRepeat = kNoRepeat;
const uint16_t kHaierACStateLength = 9;
const uint16_t kHaierACBits = kHaierACStateLength * 8;
const uint16_t kHaierAcDefaultRepeat = kNoRepeat;
const uint16_t kHaierACYRW02StateLength = 14;
const uint16_t kHaierACYRW02Bits = kHaierACYRW02StateLength * 8;
const uint16_t kHaierAcYrw02DefaultRepeat = kNoRepeat;
const uint16_t kHaierAC160StateLength = 20;
const uint16_t kHaierAC160Bits = kHaierAC160StateLength * 8;
const uint16_t kHaierAc160DefaultRepeat = kNoRepeat;
const uint16_t kHaierAC176StateLength = 22;
const uint16_t kHaierAC176Bits = kHaierAC176StateLength * 8;
const uint16_t kHaierAc176DefaultRepeat = kNoRepeat;
const uint16_t kHitachiAcStateLength = 28;
const uint16_t kHitachiAcBits = kHitachiAcStateLength * 8;
const uint16_t kHitachiAcDefaultRepeat = kNoRepeat;
const uint16_t kHitachiAc1StateLength = 13;
const uint16_t kHitachiAc1Bits = kHitachiAc1StateLength * 8;
const uint16_t kHitachiAc2StateLength = 53;
const uint16_t kHitachiAc2Bits = kHitachiAc2StateLength * 8;
const uint16_t kHitachiAc3StateLength = 27;
const uint16_t kHitachiAc3Bits = kHitachiAc3StateLength * 8;
const uint16_t kHitachiAc3MinStateLength = 15;
const uint16_t kHitachiAc3MinBits = kHitachiAc3MinStateLength * 8;
const uint16_t kHitachiAc264StateLength = 33;
const uint16_t kHitachiAc264Bits = kHitachiAc264StateLength * 8;
const uint16_t kHitachiAc296StateLength = 37;
const uint16_t kHitachiAc296Bits = kHitachiAc296StateLength * 8;
const uint16_t kHitachiAc344StateLength = 43;
const uint16_t kHitachiAc344Bits = kHitachiAc344StateLength * 8;
const uint16_t kHitachiAc424StateLength = 53;
const uint16_t kHitachiAc424Bits = kHitachiAc424StateLength * 8;
const uint16_t kInaxBits = 24;
const uint16_t kInaxMinRepeat = kSingleRepeat;
const uint16_t kJvcBits = 16;
const uint16_t kKelonBits = 48;
const uint16_t kKelon168StateLength = 21;
const uint16_t kKelon168Bits = kKelon168StateLength * 8;
const uint16_t kKelvinatorStateLength = 16;
const uint16_t kKelvinatorBits = kKelvinatorStateLength * 8;
const uint16_t kKelvinatorDefaultRepeat = kNoRepeat;
const uint16_t kLasertagBits = 13;
const uint16_t kLasertagMinRepeat = kNoRepeat;
const uint16_t kLegoPfBits = 16;
const uint16_t kLegoPfMinRepeat = kNoRepeat;
const uint16_t kLgBits = 28;
const uint16_t kLg32Bits = 32;
const uint16_t kLgDefaultRepeat = kNoRepeat;
const uint16_t kLutronBits = 35;
const uint16_t kMagiquestBits = 56;
const uint16_t kMetzBits = 19;
const uint16_t kMetzMinRepeat = kNoRepeat;
const uint16_t kMideaBits = 48;
const uint16_t kMideaMinRepeat = kNoRepeat;
const uint16_t kMidea24Bits = 24;
const uint16_t kMidea24MinRepeat = kSingleRepeat;
const uint16_t kMirageStateLength = 15;
const uint16_t kMirageBits = kMirageStateLength * 8;
const uint16_t kMirageMinRepeat = kNoRepeat;
const uint16_t kMitsubishiBits = 16;
// TODO(anyone): Verify that the Mitsubishi repeat is really needed.
// Based on marcosamarinho's code.
const uint16_t kMitsubishiMinRepeat = kSingleRepeat;
const uint16_t kMitsubishiACStateLength = 18;
const uint16_t kMitsubishiACBits = kMitsubishiACStateLength * 8;
const uint16_t kMitsubishiACMinRepeat = kSingleRepeat;
const uint16_t kMitsubishi136StateLength = 17;
const uint16_t kMitsubishi136Bits = kMitsubishi136StateLength * 8;
const uint16_t kMitsubishi136MinRepeat = kNoRepeat;
const uint16_t kMitsubishi112StateLength = 14;
const uint16_t kMitsubishi112Bits = kMitsubishi112StateLength * 8;
const uint16_t kMitsubishi112MinRepeat = kNoRepeat;
const uint16_t kMitsubishiHeavy88StateLength = 11;
const uint16_t kMitsubishiHeavy88Bits = kMitsubishiHeavy88StateLength * 8;
const uint16_t kMitsubishiHeavy88MinRepeat = kNoRepeat;
const uint16_t kMitsubishiHeavy152StateLength = 19;
const uint16_t kMitsubishiHeavy152Bits = kMitsubishiHeavy152StateLength * 8;
const uint16_t kMitsubishiHeavy152MinRepeat = kNoRepeat;
const uint16_t kMultibracketsBits = 8;
const uint16_t kMultibracketsDefaultRepeat = kSingleRepeat;
const uint16_t kNikaiBits = 24;
const uint16_t kNECBits = 32;
const uint16_t kNeoclimaStateLength = 12;
const uint16_t kNeoclimaBits = kNeoclimaStateLength * 8;
const uint16_t kNeoclimaMinRepeat = kNoRepeat;
const uint16_t kPanasonicBits = 48;
const uint32_t kPanasonicManufacturer = 0x4004;
const uint16_t kPanasonicAcStateLength = 27;
const uint16_t kPanasonicAcStateShortLength = 16;
const uint16_t kPanasonicAcBits = kPanasonicAcStateLength * 8;
const uint16_t kPanasonicAcShortBits = kPanasonicAcStateShortLength * 8;
const uint16_t kPanasonicAcDefaultRepeat = kNoRepeat;
const uint16_t kPanasonicAc32Bits = 32;
const uint16_t kPioneerBits = 64;
const uint16_t kProntoMinLength = 6;
const uint16_t kRC5RawBits = 14;
const uint16_t kRC5Bits = kRC5RawBits - 2;
const uint16_t kRC5XBits = kRC5RawBits - 1;
const uint16_t kRC6Mode0Bits = 20; // Excludes the 'start' bit.
const uint16_t kRC6_36Bits = 36; // Excludes the 'start' bit.
const uint16_t kRCMMBits = 24;
const uint16_t kSamsungBits = 32;
const uint16_t kSamsung36Bits = 36;
const uint16_t kSamsungAcStateLength = 14;
const uint16_t kSamsungAcBits = kSamsungAcStateLength * 8;
const uint16_t kSamsungAcExtendedStateLength = 21;
const uint16_t kSamsungAcExtendedBits = kSamsungAcExtendedStateLength * 8;
const uint16_t kSamsungAcDefaultRepeat = kNoRepeat;
const uint16_t kSanyoAcStateLength = 9;
const uint16_t kSanyoAcBits = kSanyoAcStateLength * 8;
const uint16_t kSanyoAc88StateLength = 11;
const uint16_t kSanyoAc88Bits = kSanyoAc88StateLength * 8;
const uint16_t kSanyoAc88MinRepeat = 2;
const uint16_t kSanyoAc152StateLength = 19;
const uint16_t kSanyoAc152Bits = kSanyoAc152StateLength * 8;
const uint16_t kSanyoAc152MinRepeat = kNoRepeat;
const uint16_t kSanyoSA8650BBits = 12;
const uint16_t kSanyoLC7461AddressBits = 13;
const uint16_t kSanyoLC7461CommandBits = 8;
const uint16_t kSanyoLC7461Bits = (kSanyoLC7461AddressBits +
kSanyoLC7461CommandBits) * 2;
const uint8_t kSharpAddressBits = 5;
const uint8_t kSharpCommandBits = 8;
const uint16_t kSharpBits = kSharpAddressBits + kSharpCommandBits + 2; // 15
const uint16_t kSharpAcStateLength = 13;
const uint16_t kSharpAcBits = kSharpAcStateLength * 8; // 104
const uint16_t kSharpAcDefaultRepeat = kNoRepeat;
const uint8_t kSherwoodBits = kNECBits;
const uint16_t kSherwoodMinRepeat = kSingleRepeat;
const uint16_t kSony12Bits = 12;
const uint16_t kSony15Bits = 15;
const uint16_t kSony20Bits = 20;
const uint16_t kSonyMinBits = 12;
const uint16_t kSonyMinRepeat = 2;
const uint16_t kSymphonyBits = 12;
const uint16_t kSymphonyDefaultRepeat = 3;
const uint16_t kTcl96AcStateLength = 12;
const uint16_t kTcl96AcBits = kTcl96AcStateLength * 8;
const uint16_t kTcl96AcDefaultRepeat = kNoRepeat;
const uint16_t kTcl112AcStateLength = 14;
const uint16_t kTcl112AcBits = kTcl112AcStateLength * 8;
const uint16_t kTcl112AcDefaultRepeat = kNoRepeat;
const uint16_t kTecoBits = 35;
const uint16_t kTecoDefaultRepeat = kNoRepeat;
const uint16_t kTeknopointStateLength = 14;
const uint16_t kTeknopointBits = kTeknopointStateLength * 8;
const uint16_t kToshibaACStateLength = 9;
const uint16_t kToshibaACBits = kToshibaACStateLength * 8;
const uint16_t kToshibaACMinRepeat = kSingleRepeat;
const uint16_t kToshibaACStateLengthShort = kToshibaACStateLength - 2;
const uint16_t kToshibaACBitsShort = kToshibaACStateLengthShort * 8;
const uint16_t kToshibaACStateLengthLong = kToshibaACStateLength + 1;
const uint16_t kToshibaACBitsLong = kToshibaACStateLengthLong * 8;
const uint16_t kTotoBits = 24;
const uint16_t kTotoShortBits = kTotoBits;
const uint16_t kTotoLongBits = kTotoShortBits * 2;
const uint16_t kTotoDefaultRepeat = kSingleRepeat;
const uint16_t kTranscoldBits = 24;
const uint16_t kTranscoldDefaultRepeat = kNoRepeat;
const uint16_t kTrotecStateLength = 9;
const uint16_t kTrotecBits = kTrotecStateLength * 8;
const uint16_t kTrotecDefaultRepeat = kNoRepeat;
const uint16_t kTrumaBits = 56;
const uint16_t kWhirlpoolAcStateLength = 21;
const uint16_t kWhirlpoolAcBits = kWhirlpoolAcStateLength * 8;
const uint16_t kWhirlpoolAcDefaultRepeat = kNoRepeat;
const uint16_t kWhynterBits = 32;
const uint8_t kVestelAcBits = 56;
const uint16_t kXmpBits = 64;
const uint16_t kZepealBits = 16;
const uint16_t kZepealMinRepeat = 4;
const uint16_t kVoltasBits = 80;
const uint16_t kVoltasStateLength = 10;
const uint16_t kMilesTag2ShotBits = 14;
const uint16_t kMilesTag2MsgBits = 24;
const uint16_t kMilesMinRepeat = 0;
const uint16_t kBoseBits = 16;
const uint16_t kRhossStateLength = 12;
const uint16_t kRhossBits = kRhossStateLength * 8;
const uint16_t kRhossDefaultRepeat = 0;
const uint16_t kClimaButlerBits = 52;

These are all const uint16_t and I wonder why these need to be C++ objects instead of defines.
As objects they take RAM, where as defines they are just numbers in the code.

So I was wondering, is there a specific reason for having them as an object?
If not, would you appreciate a PR to convert these?
I have not tried it yet, but my guess is that it would save roughly 550 bytes of RAM and maybe also reduce the build size slightly as a bonus.

@NiKiZe
Copy link
Collaborator

NiKiZe commented Aug 6, 2022

You are under the impression that these takes RAM? Do you have anything that shows that is the case?

There is explanations on why DEFINE is not used and should be avoided. As well as previous issues on this same topic.

(I will add references to these in a bit)

@crankyoldgit
Copy link
Owner

crankyoldgit commented Aug 6, 2022

I believe we have tried this before.

Please see the rest of #1493 for other efforts we've tried.

So I was wondering, is there a specific reason for having them as an object?

Yes, there is. As defined contants vs. defines, you're able to do type checking on them. It is also part of the style guide we use. They are not really in an object per se. Well, not as I understand it. The compiler does the right thing. They are not included if they are not used, and the don't take up SRAM.

If not, would you appreciate a PR to convert these?

Feel free to. If it can be proven to save space, I'll certainly look into it.
But please see #1493 (comment) before you try/waste effort on it.

I have not tried it yet, but my guess is that it would save roughly 550 bytes of RAM and maybe also reduce the build size slightly as a bonus.

I suggest you try for yourself, but I've already tried similar and it didn't save anything.

But as I said, prove it works and I'll be all on it.

@crankyoldgit crankyoldgit self-assigned this Aug 6, 2022
@crankyoldgit crankyoldgit added more info Pending Confirmation Waiting for confirmation from user labels Aug 6, 2022
@NiKiZe
Copy link
Collaborator

NiKiZe commented Aug 6, 2022

So after that (much better explanation then mine) I will now propose we close this. But are totally ready to reopen, if numbers can be shown that this would make any difference.

@TD-er
Copy link
Author

TD-er commented Aug 6, 2022

Thanks for the links.
I will have a look at the previous attempts.
One of my users is already doing some quick preliminary tests to see if it indeed saves RAM.
I will get back to this.

@NiKiZe
Copy link
Collaborator

NiKiZe commented Aug 6, 2022

I'll add (to be clear on what we expect) if built with debug flags enabled it might have an impact (but we won't optimize that)

However in general, builds are not and these consts are optimized and you would not see any difference.
But if it does make a difference do consider it a bug in the compiler, or the build settings/parameters.
But of course it is great to have such cases documented.

@crankyoldgit
Copy link
Owner

Thanks for the links. I will have a look at the previous attempts.
One of my users is already doing some quick preliminary tests to see if it indeed saves RAM.

I wish them well.
I'm fairly sure the values are treated as literals & stored in Flash/progmem space. So, if it does reduce, it will most likely be in flash/progmem space, not RAM (on an ESP8266).

If you look at those links and the comments from @mcspr . They point out how to use the gdb to look at the actual instructions built/compiled and it can determine where a variable or constant is located. i.e. Ram or Flash.

It's been a while since I delved into it. But if I recall correctly, I looked into it and the consts were all stored in Flash space.
e.g.
blah += kConstant; with gdb, when looking at the addresses in use in the compiled binary, kConstant was located in Flash/ProgMem, and most def. not a variable in RAM. Same as using the code: blah += 4; etc. The 4 is stored in Flash/Progmem.

But hey, it's been a while. I could be remembering wrong and/or things have changed etc.

@mcspr
Copy link
Contributor

mcspr commented Aug 7, 2022

+1 to 'gdb'. Here it is useful as an interactive 'objdump', 'nm', 'readelf', etc. all in the same box.

And on case-by-case basis (...preferably, not to go cross-eyed...). Compiler sees the whole code, then optimizer and then the linker get their pass at the code. inlineness, constexpr'ness also matters, or whether it is a symbol shared between files.

You may also run into the case where value is inserted as part of the machine code.
(see disassemble /m <func> and disassemble /r <func>. we just had a set of patches to the xtensa toolchain that sometimes do that)

Another one is C++ is not C, rules may vary between the two when dealing with 'const'ants and 'constexpr'essions.
(e.g. I love the very first example from this c23 blog post :)

@TD-er
Copy link
Author

TD-er commented Aug 7, 2022

Interesting blogpost!

@TD-er
Copy link
Author

TD-er commented Aug 7, 2022

OK, got feedback from this user with an already predicted result...
It doesn't reduce RAM usage and even causes some (small) increase in build size.

So this can be closed.

@crankyoldgit
Copy link
Owner

even causes some (small) increase in build size.

Okay, that was an unexpected result. Thanks for the confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more info Pending Confirmation Waiting for confirmation from user
Projects
None yet
Development

No branches or pull requests

4 participants