Remove unnecessary deps #329

gmarcosb · 2025-10-15T20:44:02Z

Fixes #314

github-actions · 2025-10-15T20:49:20Z

PR #329: Size comparison from b4bfd38 to 651abce

Increases above 0.2%:

platform	target	config	section	`b4bfd38`	`651abce`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2241040	4808	0.2

Full report (3 builds for linux)

platform	target	config	section	`b4bfd38`	`651abce`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2241040	4808	0.2
			RAM	42776	42776	0	0.0
	onoff-light	rs-matter	FLASH	2137800	2136704	-1096	-0.1
			RAM	42760	42760	0	0.0
	speaker	rs-matter	FLASH	2209256	2208208	-1048	-0.0
			RAM	5528	5528	0	0.0

ivmarkov

As per the comments.

rs-matter/src/dm/subscriptions.rs

rs-matter/src/transport/network/btp/context.rs

rs-matter/src/utils/storage/parsebuf.rs

ivmarkov · 2025-10-15T20:59:18Z

PR #329: Size comparison from b4bfd38 to 651abce

Increases above 0.2%:

platform target config section b4bfd38 651abce change % change
linux dimmable_light rs-matter FLASH 2236232 2241040 4808 0.2
Full report (3 builds for linux)

@gmarcosb Looks quite nice!

But I would not trust it until we have the dedicated guinea-pig executable. Which is compiled with - say - opt-level="z" or opt-level="s".

Standard Rust debug builds are opt-level=0 which is non-telling.

gmarcosb · 2025-10-15T21:01:25Z

PR #329: Size comparison from b4bfd38 to 651abce
Increases above 0.2%:
platform target config section b4bfd38 651abce change % change
linux dimmable_light rs-matter FLASH 2236232 2241040 4808 0.2
Full report (3 builds for linux)

@gmarcosb Looks quite nice!

But I would not trust it until we have the dedicated guinea-pig executable. Which is compiled with - say - opt-level="z" or opt-level="s".

Standard Rust debug builds are opt-level=0 which is non-telling.

Sweet, it worked! Hot damn on the first try, not bad!

So this is what I'm using for the builds, is it not enough?

cargo build --release --no-default-features --features os,rustcrypto,log

(this is in https://github.com/gmarcosb/rs-matter/blob/main/.github/workflows/size-check.yml)

ivmarkov · 2025-10-15T21:04:16Z

PR #329: Size comparison from b4bfd38 to 651abce
Increases above 0.2%:
platform target config section b4bfd38 651abce change % change
linux dimmable_light rs-matter FLASH 2236232 2241040 4808 0.2
Full report (3 builds for linux)

@gmarcosb Looks quite nice!
But I would not trust it until we have the dedicated guinea-pig executable. Which is compiled with - say - opt-level="z" or opt-level="s".
Standard Rust debug builds are opt-level=0 which is non-telling.

Sweet, it worked! Hot damn on the first try, not bad!

So this is what I'm using for the builds, is it not enough?
cargo build --release --no-default-features --features os,rustcrypto,log
(this is in https://github.com/gmarcosb/rs-matter/blob/main/.github/workflows/size-check.yml)

--release (by default) is opt-level=3, which means "optimize for speed, code-size go to hell" i.e. it does very aggressive inlining. On baremetal where code-size matters folks:

Use opt-level="z" or "s"
Use defmt (not log)
Sometimes take drastic measures like -Zbuild-std-features=panic_immediate_abort which cuts down .rodata even more (but you lose the panic messages).

We'll discuss those in more detail once we have the guinea pig executable. It should btw NOT target linux either, but a baremetal core-only target.

EDIT: And use other tricks like codegen-units=1 and then the new "fat" LTO which can eliminate more dead code.
This is a typical setup for a baremetal build which targets devices with a tiny flash.

github-actions · 2025-10-15T21:08:14Z

PR #329: Size comparison from b4bfd38 to 4e45cc3

Increases above 0.2%:

platform	target	config	section	`b4bfd38`	`4e45cc3`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2241040	4808	0.2

Full report (3 builds for linux)

platform	target	config	section	`b4bfd38`	`4e45cc3`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2241040	4808	0.2
			RAM	42776	42776	0	0.0
	onoff-light	rs-matter	FLASH	2137800	2136704	-1096	-0.1
			RAM	42760	42760	0	0.0
	speaker	rs-matter	FLASH	2209256	2208208	-1048	-0.0
			RAM	5528	5528	0	0.0

gmarcosb · 2025-10-15T21:13:34Z

PR #329: Size comparison from b4bfd38 to 651abce
Increases above 0.2%:
platform target config section b4bfd38 651abce change % change
linux dimmable_light rs-matter FLASH 2236232 2241040 4808 0.2
Full report (3 builds for linux)

@gmarcosb Looks quite nice!
But I would not trust it until we have the dedicated guinea-pig executable. Which is compiled with - say - opt-level="z" or opt-level="s".
Standard Rust debug builds are opt-level=0 which is non-telling.

Sweet, it worked! Hot damn on the first try, not bad!
So this is what I'm using for the builds, is it not enough?
cargo build --release --no-default-features --features os,rustcrypto,log
(this is in https://github.com/gmarcosb/rs-matter/blob/main/.github/workflows/size-check.yml)
--release (by default) is opt-level=3, which means "optimize for speed, code-size go to hell" i.e. it does very aggressive inlining. On baremetal where code-size matters folks:

Use opt-level="z" or "s"

Use defmt (not log)

Sometimes take drastic measures like -Zbuild-std-features=panic_immediate_abort which cuts down .rodata even more (but you lose the panic messages).

We'll discuss those in more detail once we have the guinea pig executable. It should btw NOT target linux either, but a baremetal core-only target.

EDIT: And use other tricks like codegen-units=1 and then the new "fat" LTO which can eliminate more dead code. This is a typical setup for a baremetal build which targets devices with a tiny flash.

Sounds like a plan 👍 Should just be a matter of updating the CLI in size-check.yml to whatever optimization we want

ivmarkov · 2025-10-15T21:24:28Z

Heh. The "RAM" of the speaker example is only 5K. That's because it is written in a way where everything is placed on-stack. One more reason to have the guinea-pig ELF, where we can place as much as possible stuff in .bss so that we get a more realistic memory estimation.

github-actions · 2025-10-15T21:30:16Z

PR #329: Size comparison from b4bfd38 to 7dc00f9

Increases above 0.2%:

platform	target	config	section	`b4bfd38`	`7dc00f9`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2241040	4808	0.2

Full report (3 builds for linux)

platform	target	config	section	`b4bfd38`	`7dc00f9`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2241040	4808	0.2
			RAM	42776	42776	0	0.0
	onoff-light	rs-matter	FLASH	2137800	2136704	-1096	-0.1
			RAM	42760	42760	0	0.0
	speaker	rs-matter	FLASH	2209256	2208208	-1048	-0.0
			RAM	5528	5528	0	0.0

github-actions · 2025-10-15T21:47:12Z

PR #329: Size comparison from b4bfd38 to ed742a8

Increases above 0.2%:

platform	target	config	section	`b4bfd38`	`ed742a8`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2241040	4808	0.2

Full report (3 builds for linux)

platform	target	config	section	`b4bfd38`	`ed742a8`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2241040	4808	0.2
			RAM	42776	42776	0	0.0
	onoff-light	rs-matter	FLASH	2137800	2136704	-1096	-0.1
			RAM	42760	42760	0	0.0
	speaker	rs-matter	FLASH	2209256	2208208	-1048	-0.0
			RAM	5528	5528	0	0.0

gmarcosb · 2025-10-15T22:18:15Z

Will be doing a bit of #258 to eliminate atomic

ivmarkov · 2025-10-15T22:24:48Z

Will be doing a bit of #258 to eliminate atomic

I suggest you reconsider.
#258 has enormous scope. You might end up in a never-ending PR if you start to tackle #258 just because of #329. For now, just putting two extra mutexes (or one mutex and one cell) is good enough to have #329 solved.

#258 is a whole other story. If you want to address it - be my guest, but doing it "just because" of #329 is not worth it.
#258 will require changes throughout the whole rs-matter codebase. All system clusters, Matter, transport, IM, DM, everything everything.

gmarcosb · 2025-10-16T16:18:11Z

Will be doing a bit of #258 to eliminate atomic

I suggest you reconsider. #258 has enormous scope. You might end up in a never-ending PR if you start to tackle #258 just because of #329. For now, just putting two extra mutexes (or one mutex and one cell) is good enough to have #329 solved.

#258 is a whole other story. If you want to address it - be my guest, but doing it "just because" of #329 is not worth it. #258 will require changes throughout the whole rs-matter codebase. All system clusters, Matter, transport, IM, DM, everything everything.

All done - it's not complete, as I said it's just in these specific classes where we're using atomic, so that we keep behavior (i.e. functionality usable across threads) even if it's not 100% complete (i.e. those Notification bits may need some work)

Leaves the state slightly better than it was before, and eliminates atomic usage (you were right, didn't make sense here)

It's a very small piece of #329, but acts as a good example & moves us forward

github-actions · 2025-10-16T16:49:34Z

PR #329: Size comparison from b4bfd38 to 4762a6b

Increases above 0.2%:

platform	target	config	section	`b4bfd38`	`4762a6b`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2243640	7408	0.3
	onoff-light	rs-matter	FLASH	2137800	2143256	5456	0.3
	speaker	rs-matter	RAM	5528	5552	24	0.4

Full report (3 builds for linux)

platform	target	config	section	`b4bfd38`	`4762a6b`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2243640	7408	0.3
			RAM	42776	42800	24	0.1
	onoff-light	rs-matter	FLASH	2137800	2143256	5456	0.3
			RAM	42760	42784	24	0.1
	speaker	rs-matter	FLASH	2209256	2209808	552	0.0
			RAM	5528	5552	24	0.4

github-actions · 2025-10-16T16:58:04Z

PR #329: Size comparison from b4bfd38 to 4ece152

Increases above 0.2%:

platform	target	config	section	`b4bfd38`	`4ece152`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2243624	7392	0.3
	onoff-light	rs-matter	FLASH	2137800	2143240	5440	0.3
	speaker	rs-matter	RAM	5528	5552	24	0.4

Full report (3 builds for linux)

platform	target	config	section	`b4bfd38`	`4ece152`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2243624	7392	0.3
			RAM	42776	42800	24	0.1
	onoff-light	rs-matter	FLASH	2137800	2143240	5440	0.3
			RAM	42760	42784	24	0.1
	speaker	rs-matter	FLASH	2209256	2209792	536	0.0
			RAM	5528	5552	24	0.4

ivmarkov

I have two very small nits you can disregard and one slightly more important - the "let's not default to CriticalSectionRawMutex" one.

Now, if this is the scope of change that you meant by "I'll do a part of #258 here" - yes, I'm completely fine with that.

The trouble in #258 is mutex-ifying (under a single mutex + RefCell) the state of the Matter struct.

rs-matter/src/dm/subscriptions.rs

rs-matter/src/transport/network/btp/context.rs

rs-matter/src/dm/subscriptions.rs

github-actions · 2025-10-16T17:50:32Z

PR #329: Size comparison from b4bfd38 to 87a8156

Increases above 0.2%:

platform	target	config	section	`b4bfd38`	`87a8156`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2241080	4848	0.2

Full report (3 builds for linux)

platform	target	config	section	`b4bfd38`	`87a8156`	change	% change
linux	dimmable_light	rs-matter	FLASH	2236232	2241080	4848	0.2
			RAM	42776	42776	0	0.0
	onoff-light	rs-matter	FLASH	2137800	2136504	-1296	-0.1
			RAM	42760	42760	0	0.0
	speaker	rs-matter	FLASH	2209256	2208208	-1048	-0.0
			RAM	5528	5528	0	0.0

gmarcosb requested a review from ivmarkov October 15, 2025 20:44

Remove unnecessary deps

0072b99

ivmarkov requested changes Oct 15, 2025

View reviewed changes

rs-matter/src/dm/subscriptions.rs Outdated Show resolved Hide resolved

rs-matter/src/transport/network/btp/context.rs Outdated Show resolved Hide resolved

rs-matter/src/utils/storage/parsebuf.rs Outdated Show resolved Hide resolved

Apply review suggestions

4e45cc3

gmarcosb force-pushed the dep-cleanup branch from 651abce to 4e45cc3 Compare October 15, 2025 20:59

Apply review suggestions

7dc00f9

gmarcosb force-pushed the dep-cleanup branch from f56c5c9 to 7dc00f9 Compare October 15, 2025 21:24

gmarcosb mentioned this pull request Oct 15, 2025

Add size reports #321

Merged

Remove unnecessary deps

ed742a8

Remove usage of atomic & use a Mutex instead

3a0f237

gmarcosb force-pushed the dep-cleanup branch from d955c0a to 3a0f237 Compare October 16, 2025 16:16

Apply clippy suggestions

60ed4b9

gmarcosb force-pushed the dep-cleanup branch from 6ca72b5 to 60ed4b9 Compare October 16, 2025 16:26

gmarcosb marked this pull request as ready for review October 16, 2025 16:28

Fix visibility for tests

4762a6b

gmarcosb requested a review from ivmarkov October 16, 2025 16:37

Post-increment

4ece152

ivmarkov approved these changes Oct 16, 2025

View reviewed changes

rs-matter/src/dm/subscriptions.rs Outdated Show resolved Hide resolved

rs-matter/src/dm/subscriptions.rs Show resolved Hide resolved

rs-matter/src/transport/network/btp/context.rs Outdated Show resolved Hide resolved

rs-matter/src/dm/subscriptions.rs Outdated Show resolved Hide resolved

Apply review suggestions

87a8156

gmarcosb merged commit 12141d0 into project-chip:main Oct 16, 2025
12 checks passed

gmarcosb linked an issue Oct 16, 2025 that may be closed by this pull request

Remove the dependency on portable-atomic #250

Closed

gmarcosb mentioned this pull request Oct 16, 2025

Remove the dependency on portable-atomic #250

Closed

Remove unnecessary deps #329

Remove unnecessary deps #329

Uh oh!

Conversation

gmarcosb commented Oct 15, 2025

Uh oh!

github-actions bot commented Oct 15, 2025

Uh oh!

ivmarkov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ivmarkov commented Oct 15, 2025

Uh oh!

gmarcosb commented Oct 15, 2025

Uh oh!

ivmarkov commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 15, 2025

Uh oh!

gmarcosb commented Oct 15, 2025

Uh oh!

ivmarkov commented Oct 15, 2025

Uh oh!

github-actions bot commented Oct 15, 2025

Uh oh!

github-actions bot commented Oct 15, 2025

Uh oh!

gmarcosb commented Oct 15, 2025

Uh oh!

ivmarkov commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gmarcosb commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 16, 2025

Uh oh!

github-actions bot commented Oct 16, 2025

Uh oh!

ivmarkov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Oct 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ivmarkov commented Oct 15, 2025 •

edited

Loading

ivmarkov commented Oct 15, 2025 •

edited

Loading

gmarcosb commented Oct 16, 2025 •

edited

Loading

ivmarkov left a comment •

edited

Loading