Account for KVS collisions in NVM identifiers #9489

doru91 · 2021-09-05T07:24:28Z

Problem

Old implementation was not taking into account collisions.

Change overview

The new implementation keeps an unordered hash having as keys the matter keys and as values the key ids used as NVM identifiers. Used key ids are saved in a list which is updated at each Put/Delete operation.

Testing

manual testing

merge-mamba · 2021-09-06T11:23:49Z

Potential Conflict Warning

This pull request has changes that overlap with branches below. You might want to check in with the other
branch authors before you merge to avoid a conflict. 🐍

`restyled/pull-9748` (@kghost @restyled-commits)

src/lib/support/tests/BUILD.gn (modified)

`restyled/pull-10400` (@carol-apple @restyled-commits)

src/app/server/Server.cpp (modified)

Something wrong? Got a suggestion? Email me or leave feedback here.

src/platform/K32W/KeyValueStoreManagerImpl.cpp

andy31415

Please update the summary with:

if using stl with heap usage, why this is ok (is this temporary or ok for long term?)
how are the new global variables persisted across reboot? What makes the method work consistently?

msandstedt

* how are the new global variables persisted across reboot? What makes the method work consistently?

++ What @andreilitvin said.

Three things:

kvs and kvs_ids have to be recreated at boot, but aren't
it is impossible to recreate kvs at boot though because the string keys can't be recovered from the uint8 kv id hash
overwriting a matching key with PUT is no longer possible; put with equal key isn't a key collision; this should be legal

I believe a similar key collision problem may have been solved here: #7472. Suggest using that as a possible guide to a workable solution.

tcarmelveilleux · 2021-09-16T13:54:35Z

@msandstedt

I believe a similar key collision problem may have been solved here: #7472. Suggest using that as a possible guide to a workable solution.

I think the approach of #7472 is not going to work, because it does static string -> key ID mapping. That implies no dynamic keys. For instance, to have working storage for multiple fabrics, it would means storing ALL fabrics in 1 key with a single name, rather than being able to store one key per fabric, dynamically "generated" (e.g. fabric1, fabric2, etc). So yes, using #7472 is a very quick "short term" solution, but won't scale to needing to store a potentially dynamic amount of information, such as ACL entries, multiple fabric data and/or certs split by fabric, etc.

Overall, we are starting to arrive at a new choke point in storage examples for platforms that do not arbitrarily support key mapping.

msandstedt · 2021-09-16T16:13:13Z

@msandstedt

I believe a similar key collision problem may have been solved here: #7472. Suggest using that as a possible guide to a workable solution.

I think the approach of #7472 is not going to work, because it does static string -> key ID mapping. That implies no dynamic keys. For instance, to have working storage for multiple fabrics, it would means storing ALL fabrics in 1 key with a single name, rather than being able to store one key per fabric, dynamically "generated" (e.g. fabric1, fabric2, etc). So yes, using #7472 is a very quick "short term" solution, but won't scale to needing to store a potentially dynamic amount of information, such as ACL entries, multiple fabric data and/or certs split by fabric, etc.

Overall, we are starting to arrive at a new choke point in storage examples for platforms that do not arbitrarily support key mapping.

@doru91,

@tcarmelveilleux and I discussed offline and in fact #7472 does work (albeit only for keys <= 32 chars long). It's using two separate non-volatile spaces: one for keys and one for values. This is a variant of a table of contents.

The larger story is that we are dissatisfied with the sdk's current kvstore interface. However, for the purposes here of this PR (#9489), you just need to find some way to persist the table of contents. #7472 shows one possible approach.

andy31415 · 2021-10-27T20:07:27Z

@doru91 - this should be ready to merge after the comments from Michael

github-actions · 2021-11-04T07:33:00Z

PR #9489: Size comparison from ee7682d to 060b63b

Increases above 0.2%:

platform	target	config	section	`ee7682d`	`060b63b`	change	% change
k32w	lighting-app	k32w061+se05x+release	(read/write)	699648	705288	5640	0.8
			.text	614120	619700	5580	0.9
	lock-app	k32w061+debug	(read/write)	591956	597596	5640	1.0
			.text	515948	521528	5580	1.1
	shell	k32w061+debug	(read/write)	657904	662468	4564	0.7
			.text	571272	575776	4504	0.8

Increases (3 builds for k32w)

platform	target	config	section	`ee7682d`	`060b63b`	change	% change
k32w	lighting-app	k32w061+se05x+release	(read/write)	699648	705288	5640	0.8
			.bss	77824	77880	56	0.1
			.data	1904	1908	4	0.2
			.text	614120	619700	5580	0.9
	lock-app	k32w061+debug	(read/write)	591956	597596	5640	1.0
			.bss	68340	68396	56	0.1
			.data	1868	1872	4	0.2
			.text	515948	521528	5580	1.1
	shell	k32w061+debug	(read/write)	657904	662468	4564	0.7
			.bss	78992	79048	56	0.1
			.data	1840	1844	4	0.2
			.text	571272	575776	4504	0.8

Full report (8 builds for k32w, p6, qpg)

platform	target	config	section	`ee7682d`	`060b63b`	change	% change
k32w	lighting-app	k32w061+se05x+release	(read/write)	699648	705288	5640	0.8
			.bss	77824	77880	56	0.1
			.data	1904	1908	4	0.2
			.text	614120	619700	5580	0.9
	lock-app	k32w061+debug	(read/write)	591956	597596	5640	1.0
			.bss	68340	68396	56	0.1
			.data	1868	1872	4	0.2
			.text	515948	521528	5580	1.1
	shell	k32w061+debug	(read/write)	657904	662468	4564	0.7
			.bss	78992	79048	56	0.1
			.data	1840	1844	4	0.2
			.text	571272	575776	4504	0.8
p6	all-clusters-app	default	(read/write)	2301080	2301080	0	0.0
			.bss	107880	107880	0	0.0
			.data	2520	2520	0	0.0
			.heap	922944	922944	0	0.0
			.text	1259344	1259344	0	0.0
	lock-app	default	(read/write)	2213928	2213928	0	0.0
			.bss	95680	95680	0	0.0
			.data	2392	2392	0	0.0
			.heap	935272	935272	0	0.0
			.text	1172192	1172192	0	0.0
qpg	lighting-app	qpg6100+debug	(read only)	490932	490932	0	0.0
			(read/write)	114140	114140	0	0.0
			.bss	50968	50968	0	0.0
			.data	1004	1004	0	0.0
			.text	485612	485612	0	0.0
	lock-app	qpg6100+debug	(read only)	467280	467280	0	0.0
			(read/write)	114144	114144	0	0.0
			.bss	49912	49912	0	0.0
			.data	960	960	0	0.0
			.text	461960	461960	0	0.0
	persistent-storage-app	qpg6100+debug	(read only)	153212	153212	0	0.0
			(read/write)	114140	114140	0	0.0
			.bss	18920	18920	0	0.0
			.data	356	356	0	0.0
			.text	147892	147892	0	0.0

src/platform/nxp/k32w/k32w0/KeyValueStoreManagerImpl.cpp

msandstedt

Key collision will likely brick the system with abort().

github-actions · 2021-11-04T17:21:16Z

PR #9489: Size comparison from 3295ca0 to f5b84c8

Full report (2 builds for esp32)

platform	target	config	section	`3295ca0`	`f5b84c8`
esp32	all-clusters-app	c3devkit	(read only)	880814	880814
			(read/write)	1305592	1305592
			.dram0.bss	57528	57528
			.dram0.data	16464	16464
			.flash.rodata	198360	198360
			.flash.text	880814	880814
			.iram0.text	57554	57554
		m5stack	(read only)	912095	912095
			(read/write)	425380	425380
			.dram0.bss	60032	60032
			.dram0.data	32100	32100
			.flash.rodata	207084	207084
			.flash.text	912095	912095
			.iram0.text	125115	125115

github-actions · 2021-11-05T10:59:27Z

PR #9489: Size comparison from 3f67119 to 6724e07

Increases above 0.2%:

platform	target	config	section	`3f67119`	`6724e07`	change	% change
k32w	lighting-app	k32w061+se05x+release	(read/write)	699168	705532	6364	0.9
			.text	613680	619984	6304	1.0
	lock-app	k32w061+debug	(read/write)	591988	598336	6348	1.1
			.text	516000	522288	6288	1.2
	shell	k32w061+debug	(read/write)	657480	662768	5288	0.8
			.text	570888	576116	5228	0.9

Increases (3 builds for k32w)

platform	target	config	section	`3f67119`	`6724e07`	change	% change
k32w	lighting-app	k32w061+se05x+release	(read/write)	699168	705532	6364	0.9
			.bss	77784	77840	56	0.1
			.data	1904	1908	4	0.2
			.text	613680	619984	6304	1.0
	lock-app	k32w061+debug	(read/write)	591988	598336	6348	1.1
			.bss	68316	68372	56	0.1
			.data	1872	1876	4	0.2
			.text	516000	522288	6288	1.2
	shell	k32w061+debug	(read/write)	657480	662768	5288	0.8
			.bss	78952	79008	56	0.1
			.data	1840	1844	4	0.2
			.text	570888	576116	5228	0.9

Full report (31 builds for efr32, k32w, linux, mbed, nrfconnect, telink)

platform	target	config	section	`3f67119`	`6724e07`	change	% change
efr32	lighting-app	BRD4161A	(read only)	744852	744852	0	0.0
			(read/write)	115704	115704	0	0.0
			.bss	113932	113932	0	0.0
			.data	1772	1772	0	0.0
			.text	744844	744844	0	0.0
		BRD4161A+rpc	(read only)	732384	732384	0	0.0
			(read/write)	132324	132324	0	0.0
			.bss	130436	130436	0	0.0
			.data	1888	1888	0	0.0
			.text	732376	732376	0	0.0
	lock-app	BRD4161A	(read only)	724136	724136	0	0.0
			(read/write)	113524	113524	0	0.0
			.bss	111788	111788	0	0.0
			.data	1732	1732	0	0.0
			.text	724128	724128	0	0.0
	window-app	BRD4161A	(read only)	725048	725048	0	0.0
			(read/write)	113844	113844	0	0.0
			.bss	112108	112108	0	0.0
			.data	1736	1736	0	0.0
			.text	725040	725040	0	0.0
k32w	lighting-app	k32w061+se05x+release	(read/write)	699168	705532	6364	0.9
			.bss	77784	77840	56	0.1
			.data	1904	1908	4	0.2
			.text	613680	619984	6304	1.0
	lock-app	k32w061+debug	(read/write)	591988	598336	6348	1.1
			.bss	68316	68372	56	0.1
			.data	1872	1876	4	0.2
			.text	516000	522288	6288	1.2
	shell	k32w061+debug	(read/write)	657480	662768	5288	0.8
			.bss	78952	79008	56	0.1
			.data	1840	1844	4	0.2
			.text	570888	576116	5228	0.9
linux	all-clusters-app	debug	(read only)	`1703297`	`1703297`	0	0.0
			(read/write)	118656	118656	0	0.0
			.bss	50000	50000	0	0.0
			.data	1042	1042	0	0.0
			.data.rel.ro	62336	62336	0	0.0
			.dynamic	592	592	0	0.0
			.got	4088	4088	0	0.0
			.init	27	27	0	0.0
			.init_array	552	552	0	0.0
			.rodata	139157	139157	0	0.0
			.text	1430674	1430674	0	0.0
	bridge-app	debug+rpc	(read only)	`1291317`	`1291317`	0	0.0
			(read/write)	69232	69232	0	0.0
			.bss	34928	34928	0	0.0
			.data	1568	1568	0	0.0
			.data.rel.ro	27768	27768	0	0.0
			.dynamic	592	592	0	0.0
			.got	3952	3952	0	0.0
			.init	27	27	0	0.0
			.init_array	408	408	0	0.0
			.rodata	110948	110948	0	0.0
			.text	1084453	1084453	0	0.0
	chip-tool	debug	(read only)	4508853	4508853	0	0.0
			(read/write)	125864	125864	0	0.0
			.bss	18128	18128	0	0.0
			.data	2224	2224	0	0.0
			.data.rel.ro	100096	100096	0	0.0
			.dynamic	592	592	0	0.0
			.got	4368	4368	0	0.0
			.init	27	27	0	0.0
			.init_array	432	432	0	0.0
			.rodata	231824	231824	0	0.0
			.text	4008565	4008565	0	0.0
	lighting-app	debug+rpc	(read only)	`1552313`	`1552313`	0	0.0
			(read/write)	102200	102200	0	0.0
			.bss	40568	40568	0	0.0
			.data	1170	1170	0	0.0
			.data.rel.ro	55168	55168	0	0.0
			.dynamic	608	608	0	0.0
			.got	4112	4112	0	0.0
			.init	27	27	0	0.0
			.init_array	536	536	0	0.0
			.rodata	129585	129585	0	0.0
			.text	1289122	1289122	0	0.0
	ota-provider-app	debug	(read only)	`1250553`	`1250553`	0	0.0
			(read/write)	67400	67400	0	0.0
			.bss	36992	36992	0	0.0
			.data	752	752	0	0.0
			.data.rel.ro	24568	24568	0	0.0
			.dynamic	592	592	0	0.0
			.got	4016	4016	0	0.0
			.init	27	27	0	0.0
			.init_array	448	448	0	0.0
			.rodata	112168	112168	0	0.0
			.text	1042258	1042258	0	0.0
	ota-requestor-app	debug	(read only)	1328881	1328881	0	0.0
			(read/write)	76656	76656	0	0.0
			.bss	45120	45120	0	0.0
			.data	816	816	0	0.0
			.data.rel.ro	25576	25576	0	0.0
			.dynamic	592	592	0	0.0
			.got	3992	3992	0	0.0
			.init	27	27	0	0.0
			.init_array	520	520	0	0.0
			.rodata	124176	124176	0	0.0
			.text	1106626	1106626	0	0.0
	shell	debug	(read only)	787481	787481	0	0.0
			(read/write)	57608	57608	0	0.0
			.bss	16040	16040	0	0.0
			.data	242	242	0	0.0
			.data.rel.ro	36816	36816	0	0.0
			.dynamic	592	592	0	0.0
			.got	3528	3528	0	0.0
			.init	27	27	0	0.0
			.init_array	344	344	0	0.0
			.rodata	77903	77903	0	0.0
			.text	608066	608066	0	0.0
	tv-app	debug	(read only)	`1815953`	`1815953`	0	0.0
			(read/write)	289824	289824	0	0.0
			.bss	222640	222640	0	0.0
			.data	2704	2704	0	0.0
			.data.rel.ro	58824	58824	0	0.0
			.dynamic	592	592	0	0.0
			.got	4408	4408	0	0.0
			.init	27	27	0	0.0
			.init_array	616	616	0	0.0
			.rodata	155208	155208	0	0.0
			.text	1518434	1518434	0	0.0
mbed	all-clusters-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0	0.0
			(read/write)	2291408	2291408	0	0.0
			.bss	178892	178892	0	0.0
			.data	5224	5224	0	0.0
			.heap	852328	852328	0	0.0
			.text	1254008	1254008	0	0.0
	lighting-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0	0.0
			(read/write)	2271504	2271504	0	0.0
			.bss	171948	171948	0	0.0
			.data	5568	5568	0	0.0
			.heap	858928	858928	0	0.0
			.text	1234104	1234104	0	0.0
	lock-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0	0.0
			(read/write)	`2249224`	`2249224`	0	0.0
			.bss	170844	170844	0	0.0
			.data	5560	5560	0	0.0
			.heap	860040	860040	0	0.0
			.text	1211824	1211824	0	0.0
	pigweed-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0	0.0
			(read/write)	1139744	1139744	0	0.0
			.bss	11752	11752	0	0.0
			.data	4368	4368	0	0.0
			.heap	1020328	1020328	0	0.0
			.text	103128	103128	0	0.0
	shell	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0	0.0
			(read/write)	2048464	2048464	0	0.0
			.bss	156224	156224	0	0.0
			.data	4960	4960	0	0.0
			.heap	875264	875264	0	0.0
			.text	1011064	1011064	0	0.0
nrfconnect	lighting-app	nrf52840dk_nrf52840	(read/write)	861255	861255	0	0.0
			bss	111256	111256	0	0.0
			rodata	96872	96872	0	0.0
			text	577508	577508	0	0.0
		nrf52840dk_nrf52840+rpc	(read/write)	823583	823583	0	0.0
			bss	107608	107608	0	0.0
			rodata	88048	88048	0	0.0
			text	551656	551656	0	0.0
		nrf5340dk_nrf5340_cpuapp	(read/write)	786278	786278	0	0.0
			bss	112632	112632	0	0.0
			rodata	92128	92128	0	0.0
			text	506964	506964	0	0.0
	lock-app	nrf52840dk_nrf52840	(read/write)	837959	837959	0	0.0
			bss	110292	110292	0	0.0
			rodata	93240	93240	0	0.0
			text	558988	558988	0	0.0
		nrf5340dk_nrf5340_cpuapp	(read/write)	763258	763258	0	0.0
			bss	111700	111700	0	0.0
			rodata	88548	88548	0	0.0
			text	488536	488536	0	0.0
	pigweed-app	nrf52840dk_nrf52840	(read/write)	497323	497323	0	0.0
			bss	51824	51824	0	0.0
			rodata	45776	45776	0	0.0
			text	339436	339436	0	0.0
	pump-app	nrf52840dk_nrf52840	(read/write)	844055	844055	0	0.0
			bss	110428	110428	0	0.0
			rodata	94952	94952	0	0.0
			text	563152	563152	0	0.0
	pump-controller-app	nrf52840dk_nrf52840	(read/write)	837815	837815	0	0.0
			bss	110328	110328	0	0.0
			rodata	93240	93240	0	0.0
			text	558724	558724	0	0.0
	shell	nrf52840dk_nrf52840	(read/write)	775931	775931	0	0.0
			bss	109080	109080	0	0.0
			rodata	72496	72496	0	0.0
			text	519764	519764	0	0.0
		nrf5340dk_nrf5340_cpuapp	(read/write)	690954	690954	0	0.0
			bss	110060	110060	0	0.0
			rodata	67140	67140	0	0.0
			text	440356	440356	0	0.0
telink	lighting-app	tlsr9518adk80d	(read/write)	662910	662910	0	0.0
			bss	69072	69072	0	0.0
			noinit	33216	33216	0	0.0
			text	458002	458002	0	0.0

doru91 · 2021-11-05T12:00:18Z

@doru91 - this should be ready to merge after the comments from Michael

Solved Michael's comments, the ESP32 build fails independently of the changes from this PR.

src/platform/nxp/k32w/k32w0/KeyValueStoreManagerImpl.cpp

github-actions · 2021-11-11T08:53:30Z

PR #9489: Size comparison from d45cfaa to dec7719

Increases above 0.2%:

platform	target	config	section	`d45cfaa`	`dec7719`	change	% change
k32w	lighting-app	k32w061+se05x+release	(read/write)	699396	705760	6364	0.9
			.text	613676	619980	6304	1.0
	lock-app	k32w061+debug	(read/write)	592132	598496	6364	1.1
			.text	515908	522212	6304	1.2
	shell	k32w061+debug	(read/write)	657760	663064	5304	0.8
			.text	570776	576020	5244	0.9

Increases (3 builds for k32w)

platform	target	config	section	`d45cfaa`	`dec7719`	change	% change
k32w	lighting-app	k32w061+se05x+release	(read/write)	699396	705760	6364	0.9
			.bss	78012	78068	56	0.1
			.data	1908	1912	4	0.2
			.text	613676	619980	6304	1.0
	lock-app	k32w061+debug	(read/write)	592132	598496	6364	1.1
			.bss	68548	68604	56	0.1
			.data	1876	1880	4	0.2
			.text	515908	522212	6304	1.2
	shell	k32w061+debug	(read/write)	657760	663064	5304	0.8
			.bss	79340	79396	56	0.1
			.data	1844	1848	4	0.2
			.text	570776	576020	5244	0.9

Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)

platform	target	config	section	`d45cfaa`	`dec7719`	change	% change
efr32	lighting-app	BRD4161A	(read only)	742476	742476	0	0.0
			(read/write)	116280	116280	0	0.0
			.bss	114500	114500	0	0.0
			.data	1780	1780	0	0.0
			.text	742468	742468	0	0.0
		BRD4161A+rpc	(read only)	730040	730040	0	0.0
			(read/write)	132900	132900	0	0.0
			.bss	131004	131004	0	0.0
			.data	1896	1896	0	0.0
			.text	730032	730032	0	0.0
	lock-app	BRD4161A	(read only)	721764	721764	0	0.0
			(read/write)	114096	114096	0	0.0
			.bss	112356	112356	0	0.0
			.data	1740	1740	0	0.0
			.text	721756	721756	0	0.0
	window-app	BRD4161A	(read only)	722676	722676	0	0.0
			(read/write)	114420	114420	0	0.0
			.bss	112676	112676	0	0.0
			.data	1744	1744	0	0.0
			.text	722668	722668	0	0.0
esp32	all-clusters-app	c3devkit	(read only)	879114	879114	0	0.0
			(read/write)	1305968	1305968	0	0.0
			.dram0.bss	58440	58440	0	0.0
			.dram0.data	16456	16456	0	0.0
			.flash.rodata	197824	197824	0	0.0
			.flash.text	879114	879114	0	0.0
			.iram0.text	57526	57526	0	0.0
		m5stack	(read only)	910683	910683	0	0.0
			(read/write)	423308	423308	0	0.0
			.dram0.bss	60952	60952	0	0.0
			.dram0.data	32092	32092	0	0.0
			.flash.rodata	204100	204100	0	0.0
			.flash.text	910683	910683	0	0.0
			.iram0.text	125115	125115	0	0.0
k32w	lighting-app	k32w061+se05x+release	(read/write)	699396	705760	6364	0.9
			.bss	78012	78068	56	0.1
			.data	1908	1912	4	0.2
			.text	613676	619980	6304	1.0
	lock-app	k32w061+debug	(read/write)	592132	598496	6364	1.1
			.bss	68548	68604	56	0.1
			.data	1876	1880	4	0.2
			.text	515908	522212	6304	1.2
	shell	k32w061+debug	(read/write)	657760	663064	5304	0.8
			.bss	79340	79396	56	0.1
			.data	1844	1848	4	0.2
			.text	570776	576020	5244	0.9
linux	all-clusters-app	debug	(read only)	1698921	1698921	0	0.0
			(read/write)	126432	126432	0	0.0
			.bss	57904	57904	0	0.0
			.data	1010	1010	0	0.0
			.data.rel.ro	62256	62256	0	0.0
			.dynamic	592	592	0	0.0
			.got	4088	4088	0	0.0
			.init	27	27	0	0.0
			.init_array	552	552	0	0.0
			.rodata	139061	139061	0	0.0
			.text	1426370	1426370	0	0.0
	bridge-app	debug+rpc	(read only)	1298741	1298741	0	0.0
			(read/write)	77264	77264	0	0.0
			.bss	42800	42800	0	0.0
			.data	1568	1568	0	0.0
			.data.rel.ro	27928	27928	0	0.0
			.dynamic	592	592	0	0.0
			.got	3952	3952	0	0.0
			.init	27	27	0	0.0
			.init_array	408	408	0	0.0
			.rodata	110932	110932	0	0.0
			.text	1091365	1091365	0	0.0
	chip-tool	debug	(read only)	4813069	4813069	0	0.0
			(read/write)	136992	136992	0	0.0
			.bss	25960	25960	0	0.0
			.data	2256	2256	0	0.0
			.data.rel.ro	103336	103336	0	0.0
			.dynamic	592	592	0	0.0
			.got	4368	4368	0	0.0
			.init	27	27	0	0.0
			.init_array	432	432	0	0.0
			.rodata	248298	248298	0	0.0
			.text	4288437	4288437	0	0.0
	lighting-app	debug+rpc	(read only)	1558113	1558113	0	0.0
			(read/write)	110376	110376	0	0.0
			.bss	48560	48560	0	0.0
			.data	1202	1202	0	0.0
			.data.rel.ro	55328	55328	0	0.0
			.dynamic	608	608	0	0.0
			.got	4112	4112	0	0.0
			.init	27	27	0	0.0
			.init_array	528	528	0	0.0
			.rodata	128369	128369	0	0.0
			.text	1295634	1295634	0	0.0
	ota-provider-app	debug	(read only)	`1259977`	`1259977`	0	0.0
			(read/write)	75624	75624	0	0.0
			.bss	44992	44992	0	0.0
			.data	752	752	0	0.0
			.data.rel.ro	24776	24776	0	0.0
			.dynamic	592	592	0	0.0
			.got	4016	4016	0	0.0
			.init	27	27	0	0.0
			.init_array	448	448	0	0.0
			.rodata	112615	112615	0	0.0
			.text	1050562	1050562	0	0.0
	ota-requestor-app	debug	(read only)	1345049	1345049	0	0.0
			(read/write)	79392	79392	0	0.0
			.bss	47456	47456	0	0.0
			.data	816	816	0	0.0
			.data.rel.ro	26040	26040	0	0.0
			.dynamic	592	592	0	0.0
			.got	3992	3992	0	0.0
			.init	27	27	0	0.0
			.init_array	472	472	0	0.0
			.rodata	123728	123728	0	0.0
			.text	`1121970`	`1121970`	0	0.0
	shell	debug	(read only)	788441	788441	0	0.0
			(read/write)	65608	65608	0	0.0
			.bss	23944	23944	0	0.0
			.data	242	242	0	0.0
			.data.rel.ro	36928	36928	0	0.0
			.dynamic	592	592	0	0.0
			.got	3528	3528	0	0.0
			.init	27	27	0	0.0
			.init_array	344	344	0	0.0
			.rodata	77519	77519	0	0.0
			.text	609026	609026	0	0.0
	tv-app	debug	(read only)	1842905	1842905	0	0.0
			(read/write)	408216	408216	0	0.0
			.bss	340232	340232	0	0.0
			.data	2736	2736	0	0.0
			.data.rel.ro	59600	59600	0	0.0
			.dynamic	592	592	0	0.0
			.got	4408	4408	0	0.0
			.init	27	27	0	0.0
			.init_array	616	616	0	0.0
			.rodata	155757	155757	0	0.0
			.text	1542674	1542674	0	0.0
mbed	all-clusters-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0	0.0
			(read/write)	`2289248`	`2289248`	0	0.0
			.bss	179412	179412	0	0.0
			.data	5216	5216	0	0.0
			.heap	851816	851816	0	0.0
			.text	1251848	1251848	0	0.0
	lighting-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0	0.0
			(read/write)	`2270688`	`2270688`	0	0.0
			.bss	172508	172508	0	0.0
			.data	5576	5576	0	0.0
			.heap	858360	858360	0	0.0
			.text	1233288	1233288	0	0.0
	lock-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0	0.0
			(read/write)	2248344	2248344	0	0.0
			.bss	171404	171404	0	0.0
			.data	5568	5568	0	0.0
			.heap	859472	859472	0	0.0
			.text	1210944	1210944	0	0.0
	pigweed-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0	0.0
			(read/write)	1139744	1139744	0	0.0
			.bss	11752	11752	0	0.0
			.data	4368	4368	0	0.0
			.heap	1020328	1020328	0	0.0
			.text	103128	103128	0	0.0
	shell	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0	0.0
			(read/write)	2048296	2048296	0	0.0
			.bss	156464	156464	0	0.0
			.data	4968	4968	0	0.0
			.heap	875016	875016	0	0.0
			.text	1010896	1010896	0	0.0
nrfconnect	lighting-app	nrf52840dk_nrf52840	(read/write)	861907	861907	0	0.0
			bss	111476	111476	0	0.0
			rodata	96532	96532	0	0.0
			text	578272	578272	0	0.0
		nrf52840dk_nrf52840+rpc	(read/write)	824267	824267	0	0.0
			bss	107828	107828	0	0.0
			rodata	87708	87708	0	0.0
			text	552444	552444	0	0.0
		nrf5340dk_nrf5340_cpuapp	(read/write)	786946	786946	0	0.0
			bss	112848	112848	0	0.0
			rodata	91788	91788	0	0.0
			text	507740	507740	0	0.0
	lock-app	nrf52840dk_nrf52840	(read/write)	838615	838615	0	0.0
			bss	110508	110508	0	0.0
			rodata	92904	92904	0	0.0
			text	559756	559756	0	0.0
		nrf5340dk_nrf5340_cpuapp	(read/write)	763926	763926	0	0.0
			bss	111920	111920	0	0.0
			rodata	88208	88208	0	0.0
			text	489316	489316	0	0.0
	pigweed-app	nrf52840dk_nrf52840	(read/write)	497327	497327	0	0.0
			bss	51824	51824	0	0.0
			rodata	45780	45780	0	0.0
			text	339436	339436	0	0.0
	pump-app	nrf52840dk_nrf52840	(read/write)	844707	844707	0	0.0
			bss	110648	110648	0	0.0
			rodata	94612	94612	0	0.0
			text	563916	563916	0	0.0
	pump-controller-app	nrf52840dk_nrf52840	(read/write)	838467	838467	0	0.0
			bss	110544	110544	0	0.0
			rodata	92900	92900	0	0.0
			text	559492	559492	0	0.0
	shell	nrf52840dk_nrf52840	(read/write)	775955	775955	0	0.0
			bss	109288	109288	0	0.0
			rodata	72152	72152	0	0.0
			text	519904	519904	0	0.0
		nrf5340dk_nrf5340_cpuapp	(read/write)	690962	690962	0	0.0
			bss	110272	110272	0	0.0
			rodata	66796	66796	0	0.0
			text	440512	440512	0	0.0
p6	all-clusters-app	default	(read/write)	2297136	2297136	0	0.0
			.bss	112416	112416	0	0.0
			.data	2520	2520	0	0.0
			.heap	918408	918408	0	0.0
			.text	`1255400`	`1255400`	0	0.0
	lock-app	default	(read/write)	2211760	2211760	0	0.0
			.bss	101264	101264	0	0.0
			.data	2400	2400	0	0.0
			.heap	929680	929680	0	0.0
			.text	1170024	1170024	0	0.0
qpg	lighting-app	qpg6100+debug	(read only)	490672	490672	0	0.0
			(read/write)	114144	114144	0	0.0
			.bss	51168	51168	0	0.0
			.data	1008	1008	0	0.0
			.text	485352	485352	0	0.0
	lock-app	qpg6100+debug	(read only)	466900	466900	0	0.0
			(read/write)	114140	114140	0	0.0
			.bss	50112	50112	0	0.0
			.data	964	964	0	0.0
			.text	461580	461580	0	0.0
	persistent-storage-app	qpg6100+debug	(read only)	105408	105408	0	0.0
			(read/write)	114142	114142	0	0.0
			.bss	8978	8978	0	0.0
			.data	272	272	0	0.0
			.text	100088	100088	0	0.0
telink	lighting-app	tlsr9518adk80d	(read/write)	663418	663418	0	0.0
			bss	69288	69288	0	0.0
			noinit	33216	33216	0	0.0
			text	458654	458654	0	0.0

Old implementation was not taking into account collissions. The new implementation keeps an unorderded hash having as keys the matter string keys and as values the key ids used as PDM identifiers. Used key ids are saved in a list which is updated at each Put/Delete operation. Also, in order to be able to restore from flash the unordered hash, the matter string keys are saved in flash. Signed-off-by: Doru Gucea <[email protected]>

Signed-off-by: Doru Gucea <[email protected]>

Co-authored-by: Restyled.io <[email protected]> Signed-off-by: Doru Gucea <[email protected]>

github-actions · 2021-11-11T10:46:32Z

PR #9489: Size comparison from d45cfaa to 06014a7

Decreases (1 build for esp32)

platform	target	config	section	`d45cfaa`	`06014a7`	change	% change
esp32	all-clusters-app	m5stack	(read only)	910683	910619	-64	-0.0
			.flash.text	910683	910619	-64	-0.0

Full report (13 builds for esp32, linux, qpg)

platform	target	config	section	`d45cfaa`	`06014a7`	change	% change
esp32	all-clusters-app	c3devkit	(read only)	879114	879114	0	0.0
			(read/write)	1305968	1305968	0	0.0
			.dram0.bss	58440	58440	0	0.0
			.dram0.data	16456	16456	0	0.0
			.flash.rodata	197824	197824	0	0.0
			.flash.text	879114	879114	0	0.0
			.iram0.text	57526	57526	0	0.0
		m5stack	(read only)	910683	910619	-64	-0.0
			(read/write)	423308	423308	0	0.0
			.dram0.bss	60952	60952	0	0.0
			.dram0.data	32092	32092	0	0.0
			.flash.rodata	204100	204100	0	0.0
			.flash.text	910683	910619	-64	-0.0
			.iram0.text	125115	125115	0	0.0
linux	all-clusters-app	debug	(read only)	1698921	1698921	0	0.0
			(read/write)	126432	126432	0	0.0
			.bss	57904	57904	0	0.0
			.data	1010	1010	0	0.0
			.data.rel.ro	62256	62256	0	0.0
			.dynamic	592	592	0	0.0
			.got	4088	4088	0	0.0
			.init	27	27	0	0.0
			.init_array	552	552	0	0.0
			.rodata	139061	139061	0	0.0
			.text	1426370	1426370	0	0.0
	bridge-app	debug+rpc	(read only)	1298741	1298741	0	0.0
			(read/write)	77264	77264	0	0.0
			.bss	42800	42800	0	0.0
			.data	1568	1568	0	0.0
			.data.rel.ro	27928	27928	0	0.0
			.dynamic	592	592	0	0.0
			.got	3952	3952	0	0.0
			.init	27	27	0	0.0
			.init_array	408	408	0	0.0
			.rodata	110932	110932	0	0.0
			.text	1091365	1091365	0	0.0
	chip-tool	debug	(read only)	4813069	4813069	0	0.0
			(read/write)	136992	136992	0	0.0
			.bss	25960	25960	0	0.0
			.data	2256	2256	0	0.0
			.data.rel.ro	103336	103336	0	0.0
			.dynamic	592	592	0	0.0
			.got	4368	4368	0	0.0
			.init	27	27	0	0.0
			.init_array	432	432	0	0.0
			.rodata	248298	248298	0	0.0
			.text	4288437	4288437	0	0.0
	lighting-app	debug+rpc	(read only)	1558113	1558113	0	0.0
			(read/write)	110376	110376	0	0.0
			.bss	48560	48560	0	0.0
			.data	1202	1202	0	0.0
			.data.rel.ro	55328	55328	0	0.0
			.dynamic	608	608	0	0.0
			.got	4112	4112	0	0.0
			.init	27	27	0	0.0
			.init_array	528	528	0	0.0
			.rodata	128369	128369	0	0.0
			.text	1295634	1295634	0	0.0
	ota-provider-app	debug	(read only)	`1259977`	`1259977`	0	0.0
			(read/write)	75624	75624	0	0.0
			.bss	44992	44992	0	0.0
			.data	752	752	0	0.0
			.data.rel.ro	24776	24776	0	0.0
			.dynamic	592	592	0	0.0
			.got	4016	4016	0	0.0
			.init	27	27	0	0.0
			.init_array	448	448	0	0.0
			.rodata	112615	112615	0	0.0
			.text	1050562	1050562	0	0.0
	ota-requestor-app	debug	(read only)	1345049	1345049	0	0.0
			(read/write)	79392	79392	0	0.0
			.bss	47456	47456	0	0.0
			.data	816	816	0	0.0
			.data.rel.ro	26040	26040	0	0.0
			.dynamic	592	592	0	0.0
			.got	3992	3992	0	0.0
			.init	27	27	0	0.0
			.init_array	472	472	0	0.0
			.rodata	123728	123728	0	0.0
			.text	`1121970`	`1121970`	0	0.0
	shell	debug	(read only)	788441	788441	0	0.0
			(read/write)	65608	65608	0	0.0
			.bss	23944	23944	0	0.0
			.data	242	242	0	0.0
			.data.rel.ro	36928	36928	0	0.0
			.dynamic	592	592	0	0.0
			.got	3528	3528	0	0.0
			.init	27	27	0	0.0
			.init_array	344	344	0	0.0
			.rodata	77519	77519	0	0.0
			.text	609026	609026	0	0.0
	tv-app	debug	(read only)	1842905	1842905	0	0.0
			(read/write)	408216	408216	0	0.0
			.bss	340232	340232	0	0.0
			.data	2736	2736	0	0.0
			.data.rel.ro	59600	59600	0	0.0
			.dynamic	592	592	0	0.0
			.got	4408	4408	0	0.0
			.init	27	27	0	0.0
			.init_array	616	616	0	0.0
			.rodata	155757	155757	0	0.0
			.text	1542674	1542674	0	0.0
qpg	lighting-app	qpg6100+debug	(read only)	490672	490672	0	0.0
			(read/write)	114144	114144	0	0.0
			.bss	51168	51168	0	0.0
			.data	1008	1008	0	0.0
			.text	485352	485352	0	0.0
	lock-app	qpg6100+debug	(read only)	466900	466900	0	0.0
			(read/write)	114140	114140	0	0.0
			.bss	50112	50112	0	0.0
			.data	964	964	0	0.0
			.text	461580	461580	0	0.0
	persistent-storage-app	qpg6100+debug	(read only)	105408	105408	0	0.0
			(read/write)	114142	114142	0	0.0
			.bss	8978	8978	0	0.0
			.data	272	272	0	0.0
			.text	100088	100088	0	0.0

* Fix KVS implementation Old implementation was not taking into account collissions. The new implementation keeps an unorderded hash having as keys the matter string keys and as values the key ids used as PDM identifiers. Used key ids are saved in a list which is updated at each Put/Delete operation. Also, in order to be able to restore from flash the unordered hash, the matter string keys are saved in flash. Signed-off-by: Doru Gucea <[email protected]> * Fixes after review Signed-off-by: Doru Gucea <[email protected]> * Restyled by clang-format (project-chip#11433) Co-authored-by: Restyled.io <[email protected]> Signed-off-by: Doru Gucea <[email protected]> * Restyled by clang-format (project-chip#11464) Co-authored-by: Restyled.io <[email protected]> Signed-off-by: Doru Gucea <[email protected]> Co-authored-by: restyled-io[bot] <32688539+restyled-io[bot]@users.noreply.github.com> Co-authored-by: Restyled.io <[email protected]>

pullapprove bot requested review from chrisdecenzo, jelderton, jmartinez-silabs, LuDuda, mspang, saurabhst and woody-apple September 5, 2021 07:25

pullapprove bot added the review - pending label Sep 5, 2021

kghost reviewed Sep 7, 2021

View reviewed changes

src/platform/K32W/KeyValueStoreManagerImpl.cpp Outdated Show resolved Hide resolved

andy31415 changed the title ~~Fix KVS implementation~~ Account for KVS collisions in NVM identifiers Sep 7, 2021

boring-cyborg bot added k32w platform labels Sep 7, 2021

andy31415 requested changes Sep 7, 2021

View reviewed changes

woody-apple added the SDK Approved label Sep 7, 2021

woody-apple approved these changes Sep 14, 2021

View reviewed changes

jmartinez-silabs approved these changes Sep 16, 2021

View reviewed changes

pullapprove bot requested review from Damian-Nordic, harimau-qirex, hawk248, holbrookt, mlepage-google, mrjerryjohns, msandstedt, robszewczyk, tcarmelveilleux and turon September 16, 2021 01:07

msandstedt suggested changes Sep 16, 2021

View reviewed changes

doru91 force-pushed the fix/kvs branch from b0c2564 to 060b63b Compare November 4, 2021 07:11

restyled-io bot mentioned this pull request Nov 4, 2021

Restyle Account for KVS collisions in NVM identifiers #11411

Merged

msandstedt reviewed Nov 4, 2021

View reviewed changes

src/platform/nxp/k32w/k32w0/KeyValueStoreManagerImpl.cpp Outdated Show resolved Hide resolved

msandstedt suggested changes Nov 4, 2021

View reviewed changes

doru91 force-pushed the fix/kvs branch from 30f1ffe to f7584a0 Compare November 4, 2021 16:02

restyled-io bot mentioned this pull request Nov 4, 2021

Restyle Account for KVS collisions in NVM identifiers #11433

Merged

doru91 force-pushed the fix/kvs branch from df2cd12 to 3b8240f Compare November 4, 2021 16:09

restyled-io bot mentioned this pull request Nov 4, 2021

Restyle Account for KVS collisions in NVM identifiers #11434

Merged

msandstedt approved these changes Nov 4, 2021

View reviewed changes

doru91 force-pushed the fix/kvs branch from f5b84c8 to a49bd3e Compare November 5, 2021 10:41

restyled-io bot mentioned this pull request Nov 5, 2021

Restyle Account for KVS collisions in NVM identifiers #11464

Merged

msandstedt reviewed Nov 6, 2021

View reviewed changes

src/platform/nxp/k32w/k32w0/KeyValueStoreManagerImpl.cpp Outdated Show resolved Hide resolved

msandstedt reviewed Nov 6, 2021

View reviewed changes

src/platform/nxp/k32w/k32w0/KeyValueStoreManagerImpl.cpp Outdated Show resolved Hide resolved

doru91 force-pushed the fix/kvs branch from 6724e07 to dec7719 Compare November 11, 2021 08:38

doru91 and others added 4 commits November 11, 2021 02:26

Fixes after review

5e2e478

Signed-off-by: Doru Gucea <[email protected]>

Restyled by clang-format (#11433)

3e459e3

Co-authored-by: Restyled.io <[email protected]> Signed-off-by: Doru Gucea <[email protected]>

Restyled by clang-format (#11464)

06014a7

Co-authored-by: Restyled.io <[email protected]> Signed-off-by: Doru Gucea <[email protected]>

doru91 force-pushed the fix/kvs branch from dec7719 to 06014a7 Compare November 11, 2021 10:26

andy31415 merged commit fe2cb0e into master Nov 15, 2021

andy31415 deleted the fix/kvs branch November 15, 2021 18:01

todo bot mentioned this pull request Nov 15, 2021

adjust these values */ #11792

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Account for KVS collisions in NVM identifiers #9489

Account for KVS collisions in NVM identifiers #9489

doru91 commented Sep 5, 2021

merge-mamba bot commented Sep 6, 2021 •

edited

Loading

andy31415 left a comment

msandstedt left a comment

tcarmelveilleux commented Sep 16, 2021

msandstedt commented Sep 16, 2021

andy31415 commented Oct 27, 2021

github-actions bot commented Nov 4, 2021 •

edited

Loading

msandstedt left a comment

github-actions bot commented Nov 4, 2021

github-actions bot commented Nov 5, 2021 •

edited

Loading

doru91 commented Nov 5, 2021

github-actions bot commented Nov 11, 2021 •

edited

Loading

github-actions bot commented Nov 11, 2021 •

edited

Loading

Account for KVS collisions in NVM identifiers #9489

Account for KVS collisions in NVM identifiers #9489

Conversation

doru91 commented Sep 5, 2021

Problem

Change overview

Testing

merge-mamba bot commented Sep 6, 2021 • edited Loading

Potential Conflict Warning

restyled/pull-9748 (@kghost @restyled-commits)

restyled/pull-10400 (@carol-apple @restyled-commits)

andy31415 left a comment

Choose a reason for hiding this comment

msandstedt left a comment

Choose a reason for hiding this comment

tcarmelveilleux commented Sep 16, 2021

msandstedt commented Sep 16, 2021

andy31415 commented Oct 27, 2021

github-actions bot commented Nov 4, 2021 • edited Loading

msandstedt left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 4, 2021

github-actions bot commented Nov 5, 2021 • edited Loading

doru91 commented Nov 5, 2021

github-actions bot commented Nov 11, 2021 • edited Loading

github-actions bot commented Nov 11, 2021 • edited Loading

merge-mamba bot commented Sep 6, 2021 •

edited

Loading

`restyled/pull-9748` (@kghost @restyled-commits)

`restyled/pull-10400` (@carol-apple @restyled-commits)

github-actions bot commented Nov 4, 2021 •

edited

Loading

github-actions bot commented Nov 5, 2021 •

edited

Loading

github-actions bot commented Nov 11, 2021 •

edited

Loading

github-actions bot commented Nov 11, 2021 •

edited

Loading