[IM] Merge paths when global dirty set is exhausted #17417

erjiaqing · 2022-04-15T05:03:07Z

Problem

Change overview

Implement the following functions and related unit tests

void MergeDirtyPathsUnderSameCluster();
void MergeDirtyPathsUnderSameEndpoint();

Actual algorithm implemented in this PR:

sweep the existing path to merge the attributes under the same cluster
If 1 failed to yield a new slot, merge the existing attributes under the same endpoint
If 2 failed to yield a new slot, replace the dirty set by wildcard endpoint.

Check if the new path can be merged into the existing paths after merging the existing paths.

Testing

Added new unit tests.

github-actions · 2022-04-19T05:12:30Z

PR #17417: Size comparison from 8793870 to 7524f31

Increases above 0.2%:

platform	target	config	section	`8793870`	`7524f31`	change	% change
linux	chip-tool-no-interactive-ipv6only	arm64	.bss	40865	41073	208	0.5
	thermostat-no-ble	arm64	.bss	62945	63137	192	0.3

Increases (23 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)

platform	target	config	section	`8793870`	`7524f31`	change	% change
cc13x2_26x2	all-clusters-app	LP_CC2652R7	(read only)	684759	685223	464	0.1
			.text	580864	581328	464	0.1
	lock-ftd	LP_CC2652R7	(read only)	677679	678287	608	0.1
			.rodata	98903	99015	112	0.1
			.text	578292	578788	496	0.1
	lock-mtd	LP_CC2652R7	(read only)	626423	627031	608	0.1
			.rodata	98783	98895	112	0.1
			.text	527148	527644	496	0.1
	pump-app	LP_CC2652R7	(read only)	649807	650415	608	0.1
			.rodata	75719	75831	112	0.1
			.text	573600	574096	496	0.1
	pump-controller-app	LP_CC2652R7	(read only)	643151	643759	608	0.1
			.rodata	79055	79167	112	0.1
			.text	563608	564104	496	0.1
cyw30739	light	cyw930739m2evb_01	(read/write)	619310	619782	472	0.1
			.app_xip_area	526068	526540	472	0.1
	lock	cyw930739m2evb_01	(read/write)	613562	614026	464	0.1
			.app_xip_area	521824	522288	464	0.1
	ota-requestor-no-progress-logging	cyw930739m2evb_01	(read/write)	566406	566870	464	0.1
			.app_xip_area	465056	465520	464	0.1
efr32	lighting-app	BRD4161A	(read only)	909172	909868	696	0.1
			.text	909164	909860	696	0.1
		BRD4161A+rpc	(read only)	943548	944244	696	0.1
			.text	943540	944236	696	0.1
	window-app	BRD4161A	(read only)	845540	846228	688	0.1
			.text	845532	846220	688	0.1
esp32	all-clusters-app	c3devkit	(read only)	980742	981336	594	0.1
			(read/write)	1397866	1397978	112	0.0
			.flash.rodata	201936	202048	112	0.1
			.flash.text	980742	981336	594	0.1
		m5stack	(read only)	`1036127`	`1036655`	528	0.1
			(read/write)	465608	465720	112	0.0
			.flash.rodata	231508	231620	112	0.0
			.flash.text	1030743	`1031271`	528	0.1
k32w	light	k32w061+release	(read/write)	684868	685332	464	0.1
			.text	599164	599628	464	0.1
	lock	k32w061+release	(read/write)	725716	726180	464	0.1
			.text	639444	639908	464	0.1
linux	chip-tool-no-interactive-ipv6only	arm64	(read only)	10430092	10432732	2640	0.0
			(read/write)	494193	494401	208	0.0
			.bss	40865	41073	208	0.5
			.rodata	516652	516764	112	0.0
			.text	8804420	8806948	2528	0.0
	thermostat-no-ble	arm64	(read only)	2359996	2362636	2640	0.1
			(read/write)	151137	151329	192	0.1
			.bss	62945	63137	192	0.3
			.rodata	145356	145468	112	0.1
			.text	1985648	1988176	2528	0.1
mbed	lock-app	CY8CPROTO_062_4343W+release	(read/write)	2410300	`2410924`	624	0.0
			.text	1372900	1373524	624	0.0
nrfconnect	all-clusters-app	nrf52840dk_nrf52840	(read/write)	1164691	`1165299`	608	0.1
			rodata	147572	147684	112	0.1
			text	801960	802456	496	0.1
p6	all-clusters-app	default	(read/write)	`2517224`	2517912	688	0.0
			.text	1475488	`1476176`	688	0.0
	light-app	default	(read/write)	`2417144`	2417840	696	0.0
			.text	1375408	`1376104`	696	0.1
	lock-app	default	(read/write)	2420608	2421304	696	0.0
			.text	1378872	1379568	696	0.1
telink	lighting-app	tlsr9518adk80d	(read/write)	802236	802944	708	0.1
			text	570790	571386	596	0.1

Decreases (2 builds for cc13x2_26x2)

platform	target	config	section	`8793870`	`7524f31`	change	% change
cc13x2_26x2	all-clusters-app	LP_CC2652R7	(read/write)	168424	167960	-464	-0.3
	lock-ftd	LP_CC2652R7	(read/write)	166448	165840	-608	-0.4

Full report (23 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)

platform	target	config	section	`8793870`	`7524f31`	change	% change
cc13x2_26x2	all-clusters-app	LP_CC2652R7	(read only)	684759	685223	464	0.1
			(read/write)	168424	167960	-464	-0.3
			.bss	76168	76168	0	0.0
			.data	3380	3380	0	0.0
			.rodata	103415	103415	0	0.0
			.text	580864	581328	464	0.1
	lock-ftd	LP_CC2652R7	(read only)	677679	678287	608	0.1
			(read/write)	166448	165840	-608	-0.4
			.bss	74168	74168	0	0.0
			.data	3212	3212	0	0.0
			.rodata	98903	99015	112	0.1
			.text	578292	578788	496	0.1
	lock-mtd	LP_CC2652R7	(read only)	626423	627031	608	0.1
			(read/write)	146956	146956	0	0.0
			.bss	69888	69888	0	0.0
			.data	3212	3212	0	0.0
			.rodata	98783	98895	112	0.1
			.text	527148	527644	496	0.1
	pump-app	LP_CC2652R7	(read only)	649807	650415	608	0.1
			(read/write)	152492	152492	0	0.0
			.bss	74624	74624	0	0.0
			.data	3244	3244	0	0.0
			.rodata	75719	75831	112	0.1
			.text	573600	574096	496	0.1
	pump-controller-app	LP_CC2652R7	(read only)	643151	643759	608	0.1
			(read/write)	152160	152160	0	0.0
			.bss	74328	74328	0	0.0
			.data	3208	3208	0	0.0
			.rodata	79055	79167	112	0.1
			.text	563608	564104	496	0.1
cyw30739	light	cyw930739m2evb_01	(read/write)	619310	619782	472	0.1
			.app_xip_area	526068	526540	472	0.1
			.bss	75908	75908	0	0.0
			.data	684	684	0	0.0
			.rodata	0	0	0	0.0
			.text	0	0	0	0.0
	lock	cyw930739m2evb_01	(read/write)	613562	614026	464	0.1
			.app_xip_area	521824	522288	464	0.1
			.bss	74436	74436	0	0.0
			.data	648	648	0	0.0
			.rodata	0	0	0	0.0
			.text	0	0	0	0.0
	ota-requestor-no-progress-logging	cyw930739m2evb_01	(read/write)	566406	566870	464	0.1
			.app_xip_area	465056	465520	464	0.1
			.bss	83752	83752	0	0.0
			.data	564	564	0	0.0
			.rodata	0	0	0	0.0
			.text	112	112	0	0.0
efr32	lighting-app	BRD4161A	(read only)	909172	909868	696	0.1
			(read/write)	133128	133128	0	0.0
			.bss	131088	131088	0	0.0
			.data	2040	2040	0	0.0
			.text	909164	909860	696	0.1
		BRD4161A+rpc	(read only)	943548	944244	696	0.1
			(read/write)	149812	149812	0	0.0
			.bss	147568	147568	0	0.0
			.data	2244	2244	0	0.0
			.text	943540	944236	696	0.1
	window-app	BRD4161A	(read only)	845540	846228	688	0.1
			(read/write)	131116	131116	0	0.0
			.bss	129168	129168	0	0.0
			.data	1948	1948	0	0.0
			.text	845532	846220	688	0.1
esp32	all-clusters-app	c3devkit	(read only)	980742	981336	594	0.1
			(read/write)	1397866	1397978	112	0.0
			.dram0.bss	62600	62600	0	0.0
			.dram0.data	14412	14412	0	0.0
			.flash.rodata	201936	202048	112	0.1
			.flash.text	980742	981336	594	0.1
			.iram0.text	62016	62016	0	0.0
		m5stack	(read only)	`1036127`	`1036655`	528	0.1
			(read/write)	465608	465720	112	0.0
			.dram0.bss	68112	68112	0	0.0
			.dram0.data	34152	34152	0	0.0
			.flash.rodata	231508	231620	112	0.0
			.flash.text	1030743	`1031271`	528	0.1
			.iram0.text	123107	123107	0	0.0
k32w	light	k32w061+release	(read/write)	684868	685332	464	0.1
			.bss	77912	77912	0	0.0
			.data	1992	1992	0	0.0
			.text	599164	599628	464	0.1
	lock	k32w061+release	(read/write)	725716	726180	464	0.1
			.bss	78520	78520	0	0.0
			.data	1952	1952	0	0.0
			.text	639444	639908	464	0.1
linux	chip-tool-no-interactive-ipv6only	arm64	(read only)	10430092	10432732	2640	0.0
			(read/write)	494193	494401	208	0.0
			.bss	40865	41073	208	0.5
			.data	1184	1184	0	0.0
			.data.rel.ro	390440	390440	0	0.0
			.dynamic	560	560	0	0.0
			.got	57904	57904	0	0.0
			.init	24	24	0	0.0
			.init_array	184	184	0	0.0
			.rodata	516652	516764	112	0.0
			.text	8804420	8806948	2528	0.0
	thermostat-no-ble	arm64	(read only)	2359996	2362636	2640	0.1
			(read/write)	151137	151329	192	0.1
			.bss	62945	63137	192	0.3
			.data	1440	1440	0	0.0
			.data.rel.ro	78984	78984	0	0.0
			.dynamic	560	560	0	0.0
			.got	4752	4752	0	0.0
			.init	24	24	0	0.0
			.init_array	368	368	0	0.0
			.rodata	145356	145468	112	0.1
			.text	1985648	1988176	2528	0.1
mbed	lock-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0	0.0
			(read/write)	2410300	`2410924`	624	0.0
			.bss	185228	185228	0	0.0
			.data	5840	5840	0	0.0
			.text	1372900	1373524	624	0.0
nrfconnect	all-clusters-app	nrf52840dk_nrf52840	(read/write)	1164691	`1165299`	608	0.1
			bss	136520	136520	0	0.0
			rodata	147572	147684	112	0.1
			text	801960	802456	496	0.1
p6	all-clusters-app	default	(read/write)	`2517224`	2517912	688	0.0
			.bss	118624	118624	0	0.0
			.data	2768	2768	0	0.0
			.text	1475488	`1476176`	688	0.0
	light-app	default	(read/write)	`2417144`	2417840	696	0.0
			.bss	112104	112104	0	0.0
			.data	2576	2576	0	0.0
			.text	1375408	`1376104`	696	0.1
	lock-app	default	(read/write)	2420608	2421304	696	0.0
			.bss	111880	111880	0	0.0
			.data	2536	2536	0	0.0
			.text	1378872	1379568	696	0.1
telink	lighting-app	tlsr9518adk80d	(read/write)	802236	802944	708	0.1
			bss	69952	69952	0	0.0
			noinit	40416	40416	0	0.0
			text	570790	571386	596	0.1

src/app/reporting/Engine.cpp

stale · 2022-04-27T20:11:55Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

mlepage-google · 2022-04-29T20:57:56Z

src/app/tests/TestReportingEngine.cpp

+{
+    TestContext & ctx = *static_cast<TestContext *>(apContext);
+    CHIP_ERROR err    = CHIP_NO_ERROR;
+    err               = InteractionModelEngine::GetInstance()->Init(&ctx.GetExchangeManager());


I believe this function requires another argument. It doesn't build for me after syncing this commit.

Same here. Posted build errors below.

CodeChronos928 · 2022-04-29T22:06:45Z

I'm seeing build errors in src:tests after this change on x86-64:

../src/app/tests/TestReportingEngine.cpp: In static member function ‘static void chip::app::reporting::TestReportingEngine::TestMergeAttributePathWhenDirtySetPoolExhausted(nlTestSuite*, void*)’:
../src/app/tests/TestReportingEngine.cpp:241:68: error: no matching function for call to ‘chip::app::InteractionModelEngine::Init(chip::Messaging::ExchangeManager*)’
  241 |     err               = InteractionModelEngine::GetInstance()->Init(&ctx.GetExchangeManager());
      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../src/app/tests/TestReportingEngine.cpp:26:
../src/app/InteractionModelEngine.h:110:16: note: candidate: ‘CHIP_ERROR chip::app::InteractionModelEngine::Init(chip::Messaging::ExchangeManager*, chip::FabricTable*)’
  110 |     CHIP_ERROR Init(Messaging::ExchangeManager * apExchangeMgr, FabricTable * apFabricTable);
      |                ^~~~
../src/app/InteractionModelEngine.h:110:16: note:   candidate expects 2 arguments, 1 provided

This reverts commit 3005462.

bzbarsky-apple · 2022-04-29T22:50:29Z

Looks like there was also a merge problem with the lint for PR*16. See https://github.com/project-chip/connectedhomeip/runs/6235990998?check_suite_focus=true for a failure from that....

…chip#17417)" This reverts commit 3005462.

…#17931) This reverts commit 3005462.

* [IM] Merge paths when global dirty set is exhausted * Address comments * Fix build

* [IM] Merge paths when global dirty set is exhausted (#17417) * [IM] Merge paths when global dirty set is exhausted * Address comments * Fix build * Compile breakage fix * Apply suggestions from code review Co-authored-by: Boris Zbarsky <[email protected]> Co-authored-by: Song GUO <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]>

boring-cyborg bot added the app label Apr 15, 2022

erjiaqing force-pushed the im/issue16273 branch from 31a20d7 to 7524f31 Compare April 19, 2022 04:42

bzbarsky-apple reviewed Apr 19, 2022

View reviewed changes

src/app/reporting/Engine.cpp Show resolved Hide resolved

Fix build

2e23275

boring-cyborg bot added the lib label Apr 20, 2022

bzbarsky-apple mentioned this pull request Apr 25, 2022

[YAML] Update the tests to use ReadClient/WriteClient/CommandSender directly instead of Invoke #17628

Merged

stale bot added the stale Stale issue or PR label Apr 27, 2022

pullapprove bot requested review from arkq, emargolis and harimau-qirex and removed request for lzgrablic02 and austinh0 April 27, 2022 20:12

yunhanw-google approved these changes Apr 28, 2022

View reviewed changes

stale bot removed the stale Stale issue or PR label Apr 28, 2022

pullapprove bot added review - approved stale Stale issue or PR and removed review - pending stale Stale issue or PR labels Apr 28, 2022

bzbarsky-apple merged commit 3005462 into project-chip:master Apr 29, 2022

mlepage-google reviewed Apr 29, 2022

View reviewed changes

woody-apple added a commit that referenced this pull request Apr 29, 2022

Revert "[IM] Merge paths when global dirty set is exhausted (#17417)"

abe53f4

This reverts commit 3005462.

woody-apple mentioned this pull request Apr 29, 2022

Revert "[IM] Merge paths when global dirty set is exhausted" #17931

Merged

CodeChronos928 added a commit to CodeChronos928/connectedhomeip that referenced this pull request Apr 29, 2022

Revert "[IM] Merge paths when global dirty set is exhausted (project-…

1ed3524

…chip#17417)" This reverts commit 3005462.

mlepage-google pushed a commit that referenced this pull request Apr 29, 2022

Revert "[IM] Merge paths when global dirty set is exhausted (#17417)" (…

9dc2682

…#17931) This reverts commit 3005462.

mrjerryjohns pushed a commit to mrjerryjohns/connectedhomeip that referenced this pull request May 2, 2022

[IM] Merge paths when global dirty set is exhausted (project-chip#17417)

3f4a807

* [IM] Merge paths when global dirty set is exhausted * Address comments * Fix build

mrjerryjohns mentioned this pull request May 2, 2022

Un-revert #17417 #17974

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IM] Merge paths when global dirty set is exhausted #17417

[IM] Merge paths when global dirty set is exhausted #17417

erjiaqing commented Apr 15, 2022 •

edited

Loading

github-actions bot commented Apr 19, 2022 •

edited

Loading

stale bot commented Apr 27, 2022

mlepage-google Apr 29, 2022

CodeChronos928 Apr 29, 2022

CodeChronos928 commented Apr 29, 2022

bzbarsky-apple commented Apr 29, 2022

[IM] Merge paths when global dirty set is exhausted #17417

[IM] Merge paths when global dirty set is exhausted #17417

Conversation

erjiaqing commented Apr 15, 2022 • edited Loading

Problem

Change overview

Testing

github-actions bot commented Apr 19, 2022 • edited Loading

stale bot commented Apr 27, 2022

mlepage-google Apr 29, 2022

Choose a reason for hiding this comment

CodeChronos928 Apr 29, 2022

Choose a reason for hiding this comment

CodeChronos928 commented Apr 29, 2022

bzbarsky-apple commented Apr 29, 2022

erjiaqing commented Apr 15, 2022 •

edited

Loading

github-actions bot commented Apr 19, 2022 •

edited

Loading