Decouple ObjectPool from SystemLayer #9329

yufengwangca · 2021-08-30T14:32:32Z

Problem

What is being fixed? Examples:

Currently, the ObjectPool is coupled with SystemLayer and only be used by SystemLayer and InetLayer components. The purpose of this coupling is to guarantee the ObjectPool APIs are thread safe by atomically checking if the object has been retained and setting the object to 'retained' on those embedded systems which do not support std::mutex.
__sync_bool_compare_and_swap(&this->mSystemLayer, nullptr, &aLayer))
Actually, we can leverage another existing field mRefCount to do the same check and set atomically. So mSystemLayer can be saved from each Object.
__sync_bool_compare_and_swap(&this->mRefCount, 0, 1)
We could decouple ObjectPool from SystemLayer to make ObjectPool more generic and to be used by higher layers (such as apps) which do not have access to SystemLayer.

Change overview

Decouple ObjectPool from SystemLayer

Testing

How was this tested? (at least one bullet point required)

Regression test is covered by unit test ./TestSystemObject

src/system/SystemObject.cpp

src/system/SystemObject.h

yunhanw-google

besides the above comments, the overall decoupling Looks great! thanks

github-actions · 2021-09-01T17:15:51Z

Size increase report for "gn_qpg-example-build" from 90c6e96

File	Section	File	VM
chip-qpg6100-lighting-example.out	.heap	0	144
chip-qpg6100-lighting-example.out	.text	8	8
chip-qpg6100-lighting-example.out	.bss	0	-144

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
.debug_abbrev,0,331
.heap,144,0
.strtab,0,79
.symtab,0,64
.debug_ranges,0,24
.debug_frame,0,12
.debug_aranges,0,8
.text,8,8
.shstrtab,0,1
[Unmapped],0,-8
.debug_str,0,-32
.debug_line,0,-34
.debug_loc,0,-71
.bss,-144,0
.debug_info,0,-622

github-actions · 2021-09-01T17:34:56Z

Size increase report for "esp32-example-build" from 90c6e96

File	Section	File	VM
chip-shell.elf	.flash.rodata	-56	-56
chip-shell.elf	.flash.text	-56	-56
chip-shell.elf	.dram0.bss	0	-320
chip-ipv6only-app.elf	.flash.text	172	172
chip-temperature-measurement-app.elf	.flash.text	-32	-32
chip-temperature-measurement-app.elf	.flash.rodata	-56	-56
chip-temperature-measurement-app.elf	.dram0.bss	0	-288
chip-all-clusters-app.elf	.flash.rodata	-56	-56
chip-all-clusters-app.elf	.flash.text	-64	-64
chip-all-clusters-app.elf	.dram0.bss	0	-288
chip-lock-app.elf	.flash.rodata	-56	-56
chip-lock-app.elf	.flash.text	-160	-160
chip-lock-app.elf	.dram0.bss	0	-288
chip-bridge-app.elf	.flash.text	-44	-44
chip-bridge-app.elf	.flash.rodata	-56	-56
chip-bridge-app.elf	.dram0.bss	0	-288

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_ranges,0,464
.debug_abbrev,0,193
.xt.prop._ZN4chip6System10ObjectPoolINS0_5TimerELj32EE9TryCreateEv,0,168
[Unmapped],0,112
.strtab,0,79
.debug_frame,0,24
.symtab,0,16
.debug_aranges,0,8
.shstrtab,0,-23
.debug_str,0,-55
.flash.rodata,-56,-56
.flash.text,-56,-56
.xt.prop._ZN4chip6System10ObjectPoolINS0_5TimerELj32EE9TryCreateERNS0_5LayerE,0,-168
.debug_line,0,-197
.dram0.bss,-320,0
.debug_loc,0,-376
.debug_info,0,-1897

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize
.debug_loc,0,-2
.debug_str,0,-12
.debug_info,0,-46

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize
[Unmapped],0,3924
.flash.text,172,172

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
.debug_ranges,0,432
.debug_abbrev,0,196
.xt.prop._ZN4chip6System10ObjectPoolINS0_5TimerELj32EE9TryCreateEv,0,144
[Unmapped],0,88
.strtab,0,79
.debug_frame,0,24
.symtab,0,16
.debug_aranges,0,8
.shstrtab,0,-23
.flash.text,-32,-32
.flash.rodata,-56,-56
.debug_str,0,-58
.xt.prop._ZN4chip6System10ObjectPoolINS0_5TimerELj32EE9TryCreateERNS0_5LayerE,0,-144
.debug_line,0,-206
.dram0.bss,-288,0
.debug_loc,0,-694
.debug_info,0,-2614

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_loc,0,598
.debug_abbrev,0,208
[Unmapped],0,120
.strtab,0,79
.symtab,0,16
.debug_aranges,0,8
.riscv.attributes,0,1
.shstrtab,0,1
.debug_frame,0,-4
.debug_str,0,-56
.flash.rodata,-56,-56
.debug_ranges,0,-64
.flash.text,-64,-64
.debug_line,0,-265
.dram0.bss,-288,0
.debug_info,0,-2886

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
.debug_ranges,0,464
[Unmapped],0,216
.debug_abbrev,0,193
.xt.prop._ZN4chip6System10ObjectPoolINS0_5TimerELj32EE9TryCreateEv,0,168
.strtab,0,79
.debug_frame,0,24
.symtab,0,16
.debug_aranges,0,8
.shstrtab,0,-23
.flash.rodata,-56,-56
.debug_str,0,-57
.flash.text,-160,-160
.xt.prop._ZN4chip6System10ObjectPoolINS0_5TimerELj32EE9TryCreateERNS0_5LayerE,0,-168
.debug_line,0,-175
.dram0.bss,-288,0
.debug_loc,0,-525
.debug_info,0,-2724

Comparing ./master_artifact/chip-bridge-app.elf and ./pull_artifact/chip-bridge-app.elf:

sections,vmsize,filesize
.debug_ranges,0,464
.debug_abbrev,0,193
.xt.prop._ZN4chip6System10ObjectPoolINS0_5TimerELj32EE9TryCreateEv,0,168
[Unmapped],0,100
.strtab,0,79
.debug_frame,0,24
.symtab,0,16
.debug_aranges,0,8
.shstrtab,0,-23
.flash.text,-44,-44
.debug_str,0,-55
.flash.rodata,-56,-56
.xt.prop._ZN4chip6System10ObjectPoolINS0_5TimerELj32EE9TryCreateERNS0_5LayerE,0,-168
.debug_line,0,-193
.dram0.bss,-288,0
.debug_loc,0,-459
.debug_info,0,-2718

github-actions · 2021-09-01T17:35:11Z

Size increase report for "nrfconnect-example-build" from 90c6e96

File	Section	File	VM
chip-lock.elf	[LOAD #3 [RW]]	0	28
chip-lock.elf	device_handles	4	4
chip-lock.elf	text	-36	-36
chip-lock.elf	bss	0	-156
chip-shell.elf	[LOAD #3 [RW]]	0	17
chip-shell.elf	device_handles	4	4
chip-shell.elf	text	-36	-36
chip-shell.elf	bss	0	-145

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.debug_loc,0,211
.debug_line,0,60
.debug_ranges,0,56
[LOAD #3 [RW]],28,0
.debug_abbrev,0,11
device_handles,4,4
.shstrtab,0,-1
.debug_str,0,-7
.debug_frame,0,-12
.strtab,0,-23
text,-36,-36
bss,-156,0
.debug_info,0,-359

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_line,0,57
.debug_ranges,0,56
.debug_loc,0,31
[LOAD #3 [RW]],17,0
.debug_abbrev,0,11
device_handles,4,4
.shstrtab,0,-1
.debug_str,0,-7
.debug_frame,0,-12
.strtab,0,-23
text,-36,-36
bss,-145,0
.debug_info,0,-160

boring-cyborg bot added inet system labels Aug 30, 2021

pullapprove bot requested review from chrisdecenzo, jelderton, jmartinez-silabs, LuDuda, mspang, sagar-apple, saurabhst, vivien-apple and woody-apple August 30, 2021 14:33

pullapprove bot added the review - pending label Aug 30, 2021

yufengwangca requested review from jmartinez-silabs, msandstedt and yunhanw-google and removed request for jmartinez-silabs August 30, 2021 14:35

andy31415 approved these changes Aug 30, 2021

View reviewed changes

src/system/SystemObject.cpp Outdated Show resolved Hide resolved

src/system/SystemObject.h Outdated Show resolved Hide resolved

src/system/SystemObject.h Show resolved Hide resolved

andy31415 added the SDK Approved label Aug 30, 2021

yunhanw-google approved these changes Aug 30, 2021

View reviewed changes

msandstedt approved these changes Aug 31, 2021

View reviewed changes

andy31415 added the technical review - DONE label Aug 31, 2021

woody-apple approved these changes Aug 31, 2021

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels Aug 31, 2021

kpschoedel mentioned this pull request Aug 31, 2021

Virtualize System::Layer interfaces #9366

Merged

jmartinez-silabs approved these changes Aug 31, 2021

View reviewed changes

yufengwangca added 2 commits September 1, 2021 09:05

Decouple ObjectPool from SystemLayer

3a4698d

Address review comments

b1db8f3

msandstedt merged commit 9dfa537 into project-chip:master Sep 2, 2021

yufengwangca deleted the pr/pool/system branch September 2, 2021 01:16

mkardous-silabs pushed a commit to mkardous-silabs/connectedhomeip that referenced this pull request Sep 24, 2021

Decouple ObjectPool from SystemLayer (project-chip#9329)

90b401e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Decouple ObjectPool from SystemLayer #9329

Decouple ObjectPool from SystemLayer #9329

yufengwangca commented Aug 30, 2021 •

edited

Loading

yunhanw-google left a comment •

edited

Loading

github-actions bot commented Sep 1, 2021

github-actions bot commented Sep 1, 2021

github-actions bot commented Sep 1, 2021

Decouple ObjectPool from SystemLayer #9329

Decouple ObjectPool from SystemLayer #9329

Conversation

yufengwangca commented Aug 30, 2021 • edited Loading

Problem

Change overview

Testing

yunhanw-google left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Sep 1, 2021

github-actions bot commented Sep 1, 2021

github-actions bot commented Sep 1, 2021

yufengwangca commented Aug 30, 2021 •

edited

Loading

yunhanw-google left a comment •

edited

Loading