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

Reduce binary size by avoiding aggressive inlining of term decoding #463

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pguyot
Copy link
Collaborator

@pguyot pguyot commented Jan 7, 2023

This leaves room for libs and programs

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

@pguyot pguyot marked this pull request as draft January 7, 2023 14:29
@pguyot pguyot marked this pull request as ready for review January 8, 2023 07:01
This leaves more room for libs and programs for smaller targets

Signed-off-by: Paul Guyot <[email protected]>
@bettio
Copy link
Collaborator

bettio commented May 1, 2023

DECODE_COMPACT_TERM is used pretty everywhere, so avoiding aggressive inlining might reduce performances quite a lot, so let's review this from 2 different point of views:

  • Module loading: not very frequent, we do this just once every time a new module is called. On ESP32 code size isn't a big constraint, but reducing size on a less frequent situation might be good anyway.
  • Execution: here performance are critical so we need to find a way to make inlining level tunable. On targets such as ESP32 I would keep aggressive inlining.

We have following options on the table:

  • macro
  • (always) inline function
  • regular function

Open points:

is the compiler able to peform same kind of optimizations on both the "macro" and the "(always) inline function" scenario?

If no, we might do this:

#define DECODE_COMPACT_TERM_IMPL(...)

#ifdef OPTIMIZE_SIZE

void decode_compact_term(...) {
DECODE_COMPACT_TERM_IMPL
}

#define DECODE_COMPACT_TERM(...)
decode_compact_term(...)

#else

#define DECODE_COMPACT_TERM(...)
DECODE_COMPACT_TERM_IMPL(...)

#endif

I'll investigate the real impact on the 2 scenarios for execution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants