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

Force inline lightweight methods to improve flash size #1990

Closed
jputcu opened this issue Nov 3, 2023 · 3 comments
Closed

Force inline lightweight methods to improve flash size #1990

jputcu opened this issue Nov 3, 2023 · 3 comments

Comments

@jputcu
Copy link

jputcu commented Nov 3, 2023

While going through the assembly, I noticed there were 2 "identical" toJson functions of about 100 bytes:

00001776 0000005e t ArduinoJson::V6212MA::Converter<char const*, void>::toJson(char const*, ArduinoJson::V6212MA::JsonVariant) [clone .isra.108]
00005a6e 00000060 t ArduinoJson::V6212MA::Converter<char const*, void>::toJson(char const*, ArduinoJson::V6212MA::JsonVariant) [clone .isra.65]

Looking more in detail, the only difference was:

call	 0x1754 <ArduinoJson::V6212MA::detail::ZeroTerminatedRamString::size() const>
vs
rcall	.-110    	; 0x1754 <ArduinoJson::V6212MA::detail::ZeroTerminatedRamString::size() const>

If you look into what the size() call actually does:

size_t size() const {
    return str_ ? ::strlen(str_) : 0;
  }

So, if I force that inline:

[[gnu::always_inline]] size_t size() const {
    return str_ ? ::strlen(str_) : 0;
}

My duplicates become much smaller (almost half), in total my code shrinks 150 bytes:

00001612 00000038 t ArduinoJson::V6212MA::Converter<char const*, void>::toJson(char const*, ArduinoJson::V6212MA::JsonVariant) [clone .isra.108]
00005960 00000038 t ArduinoJson::V6212MA::Converter<char const*, void>::toJson(char const*, ArduinoJson::V6212MA::JsonVariant) [clone .isra.65]

Now my duplicates are really duplicate, so perhaps it has something to do with where they are compiles?

@jputcu
Copy link
Author

jputcu commented Nov 3, 2023

This is compiled using avr-gcc 7.3 (now current Arduino or Microchip) -Os.

The compiler generates the following code for the size command:

00001754 <ArduinoJson::V6212MA::detail::ZeroTerminatedRamString::size() const>:
    1754:	movw	r30, r24
    1756:	ld	r26, Z
    1758:	ldd	r27, Z+1	; 0x01
    175a:	sbiw	r26, 0x00	; 0
    175c:	breq	.+18     	; 0x1770 <ArduinoJson::V6212MA::detail::ZeroTerminatedRamString::size() const+0x1c>
    175e:	movw	r30, r26
    1760:	ld	r0, Z+
    1762:	and	r0, r0
    1764:	brne	.-6      	; 0x1760 <ArduinoJson::V6212MA::detail::ZeroTerminatedRamString::size() const+0xc>
    1766:	sbiw	r30, 0x01	; 1
    1768:	movw	r24, r30
    176a:	sub	r24, r26
    176c:	sbc	r25, r27
    176e:	ret
    1770:	ldi	r25, 0x00	; 0
    1772:	ldi	r24, 0x00	; 0
    1774:	ret

Which it finds too big to inline.

@bblanchon
Copy link
Owner

Here are my measurements with ArduinoJson's examples:

Example Before After
JsonParserExample 6762 6758
JsonGeneratorExample 5574 5578
MsgPackParser 7088 7080
StringExample 10694 10698
ProgmemExample 6480 6476

It doesn't save much, but these programs are really small.
Still, I think it's an improvement.
Are you having second thoughts?

@jputcu
Copy link
Author

jputcu commented Nov 4, 2023

My program is around 75k on an Atmega128: going from 74900 to 74750.
It is an improvement, to bad the compiler didn't inline it (maybe with -O2 or -O3). Forcing it inline soon becomes a macro, like in so many other libraries, making it less C++. So I leave it up to you to decide, perhaps it is just something to keep in mind.

The biggest improvement will be getting rid of the duplicate toJson. All methods using it are called from the same source file, some take A, other take B. So perhaps some code organisation.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants