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.

Sign up for GitHub

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

Jump to bottom

Use safer System::Clock types in transport and messaging #10913

Merged
merged 1 commit into from
Oct 27, 2021
Merged

Use safer System::Clock types in transport and messaging #10913

merged 1 commit into from
Oct 27, 2021

Conversation

kpschoedel
Copy link
Contributor

Problem

Code uses plain integers to represent time values and relies on
users to get the unit scale correct.

Part of #10062 Some operations on System::Clock types are not safe

Change overview

Convert src/transport and src/messaging to use the safer Clock types.

Testing

CI; no change to functionality intended.
Conversion includes src/messaging/tests/echo/echo_requester.cpp
and several src/transport/raw/tests/.

@kpschoedel
Use safer System::Clock types in transport and messaging
92826e8 
#### Problem

Code uses plain integers to represent time values and relies on
users to get the unit scale correct.

Part of project-chip#10062 _Some operations on System::Clock types are not safe_

#### Change overview

Convert `src/transport` and `src/messaging` to use the safer `Clock` types.

#### Testing

CI; no change to functionality intended.
Conversion includes `src/messaging/tests/echo/echo_requester.cpp`
and several `src/transport/raw/tests/`.
@kpschoedel
Copy link
Contributor Author

kpschoedel commented Oct 25, 2021 •
edited by github-actions bot
Loading

PR #10913: Size comparison from 37c3f7f to 92826e8

Increases (8 builds for linux)
platform target config section 37c3f7f 92826e8 change % change
linux all-clusters-app debug .text 1370002 1370738 736 0.1
1370002 1370738 736 0.1
1370002 1370738 736 0.1
bridge-app debug+rpc .text 1064181 1064917 736 0.1
1064181 1064917 736 0.1
1064181 1064917 736 0.1
chip-tool debug .text 3699141 3699877 736 0.0
3699141 3699877 736 0.0
3699141 3699877 736 0.0
lighting-app debug+rpc .text 1262130 1262866 736 0.1
1262130 1262866 736 0.1
1262130 1262866 736 0.1
ota-provider-app debug .text 1023634 1024338 704 0.1
1023634 1024338 704 0.1
1023634 1024338 704 0.1
ota-requestor-app debug .text 1142066 1142770 704 0.1
1142066 1142770 704 0.1
1142066 1142770 704 0.1
shell debug .text 599506 600034 528 0.1
599506 600034 528 0.1
599506 600034 528 0.1
tv-app debug .text 1454098 1454626 528 0.0
1454098 1454626 528 0.0
1454098 1454626 528 0.0
Decreases (12 builds for efr32, nrfconnect, p6, telink)
platform target config section 37c3f7f 92826e8 change % change
efr32 lighting-app BRD4161A .text 735784 735736 -48 -0.0
735784 735736 -48 -0.0
BRD4161A+rpc .text 723200 723152 -48 -0.0
723200 723152 -48 -0.0
lock-app BRD4161A .text 715000 714952 -48 -0.0
715000 714952 -48 -0.0
window-app BRD4161A .text 715828 715780 -48 -0.0
715828 715780 -48 -0.0
nrfconnect lighting-app nrf52840dk_nrf52840 text 577220 577216 -4 -0.0
nrf52840dk_nrf52840+rpc text 550412 550408 -4 -0.0
lock-app nrf52840dk_nrf52840 text 558708 558704 -4 -0.0
nrf5340dk_nrf5340_cpuapp text 488172 488168 -4 -0.0
pump-controller-app nrf52840dk_nrf52840 text 558500 558496 -4 -0.0
shell nrf52840dk_nrf52840 text 520316 520312 -4 -0.0
p6 lock-app default .text 1126008 1125960 -48 -0.0
1126008 1125960 -48 -0.0
1126008 1125960 -48 -0.0
1126008 1125960 -48 -0.0
1126008 1125960 -48 -0.0
1126008 1125960 -48 -0.0
telink lighting-app tlsr9518adk80d text 457680 457666 -14 -0.0
457680 457666 -14 -0.0
457680 457666 -14 -0.0
457680 457666 -14 -0.0
457680 457666 -14 -0.0
457680 457666 -14 -0.0
Full report (30 builds for efr32, k32w, linux, nrfconnect, p6, qpg, telink)
platform target config section 37c3f7f 92826e8 change % change
efr32 lighting-app BRD4161A .bss 113684 113684 0 0.0
113684 113684 0 0.0
.data 1752 1752 0 0.0
1752 1752 0 0.0
.text 735784 735736 -48 -0.0
735784 735736 -48 -0.0
BRD4161A+rpc .bss 130188 130188 0 0.0
130188 130188 0 0.0
.data 1852 1852 0 0.0
1852 1852 0 0.0
.text 723200 723152 -48 -0.0
723200 723152 -48 -0.0
lock-app BRD4161A .bss 111540 111540 0 0.0
111540 111540 0 0.0
.data 1712 1712 0 0.0
1712 1712 0 0.0
.text 715000 714952 -48 -0.0
715000 714952 -48 -0.0
window-app BRD4161A .bss 111852 111852 0 0.0
111852 111852 0 0.0
.data 1716 1716 0 0.0
1716 1716 0 0.0
.text 715828 715780 -48 -0.0
715828 715780 -48 -0.0
k32w lighting-app k32w061+se05x+release .bss 78712 78712 0 0.0
78712 78712 0 0.0
78712 78712 0 0.0
78712 78712 0 0.0
.data 1900 1900 0 0.0
1900 1900 0 0.0
1900 1900 0 0.0
1900 1900 0 0.0
.text 613792 613792 0 0.0
613792 613792 0 0.0
613792 613792 0 0.0
613792 613792 0 0.0
lock-app k32w061+debug .bss 69196 69196 0 0.0
69196 69196 0 0.0
69196 69196 0 0.0
69196 69196 0 0.0
.data 1864 1864 0 0.0
1864 1864 0 0.0
1864 1864 0 0.0
1864 1864 0 0.0
.text 515092 515092 0 0.0
515092 515092 0 0.0
515092 515092 0 0.0
515092 515092 0 0.0
shell k32w061+debug .bss 63256 63256 0 0.0
63256 63256 0 0.0
63256 63256 0 0.0
63256 63256 0 0.0
.data 672 672 0 0.0
672 672 0 0.0
672 672 0 0.0
672 672 0 0.0
.text 359556 359556 0 0.0
359556 359556 0 0.0
359556 359556 0 0.0
359556 359556 0 0.0
linux all-clusters-app debug .bss 51184 51184 0 0.0
51184 51184 0 0.0
51184 51184 0 0.0
.data 978 978 0 0.0
978 978 0 0.0
978 978 0 0.0
.data.rel.ro 60576 60576 0 0.0
60576 60576 0 0.0
60576 60576 0 0.0
.dynamic 592 592 0 0.0
592 592 0 0.0
592 592 0 0.0
.got 4088 4088 0 0.0
4088 4088 0 0.0
4088 4088 0 0.0
.init 27 27 0 0.0
27 27 0 0.0
27 27 0 0.0
.init_array 512 512 0 0.0
512 512 0 0.0
512 512 0 0.0
.rodata 135573 135573 0 0.0
135573 135573 0 0.0
135573 135573 0 0.0
.text 1370002 1370738 736 0.1
1370002 1370738 736 0.1
1370002 1370738 736 0.1
bridge-app debug+rpc .bss 51856 51856 0 0.0
51856 51856 0 0.0
51856 51856 0 0.0
.data 976 976 0 0.0
976 976 0 0.0
976 976 0 0.0
.data.rel.ro 27112 27112 0 0.0
27112 27112 0 0.0
27112 27112 0 0.0
.dynamic 592 592 0 0.0
592 592 0 0.0
592 592 0 0.0
.got 3952 3952 0 0.0
3952 3952 0 0.0
3952 3952 0 0.0
.init 27 27 0 0.0
27 27 0 0.0
27 27 0 0.0
.init_array 400 400 0 0.0
400 400 0 0.0
400 400 0 0.0
.rodata 109740 109740 0 0.0
109740 109740 0 0.0
109740 109740 0 0.0
.text 1064181 1064917 736 0.1
1064181 1064917 736 0.1
1064181 1064917 736 0.1
chip-tool debug .bss 17680 17680 0 0.0
17680 17680 0 0.0
17680 17680 0 0.0
.data 1584 1584 0 0.0
1584 1584 0 0.0
1584 1584 0 0.0
.data.rel.ro 95280 95280 0 0.0
95280 95280 0 0.0
95280 95280 0 0.0
.dynamic 592 592 0 0.0
592 592 0 0.0
592 592 0 0.0
.got 4368 4368 0 0.0
4368 4368 0 0.0
4368 4368 0 0.0
.init 27 27 0 0.0
27 27 0 0.0
27 27 0 0.0
.init_array 416 416 0 0.0
416 416 0 0.0
416 416 0 0.0
.rodata 208700 208700 0 0.0
208700 208700 0 0.0
208700 208700 0 0.0
.text 3699141 3699877 736 0.0
3699141 3699877 736 0.0
3699141 3699877 736 0.0
lighting-app debug+rpc .bss 41176 41176 0 0.0
41176 41176 0 0.0
41176 41176 0 0.0
.data 1106 1106 0 0.0
1106 1106 0 0.0
1106 1106 0 0.0
.data.rel.ro 53808 53808 0 0.0
53808 53808 0 0.0
53808 53808 0 0.0
.dynamic 608 608 0 0.0
608 608 0 0.0
608 608 0 0.0
.got 4112 4112 0 0.0
4112 4112 0 0.0
4112 4112 0 0.0
.init 27 27 0 0.0
27 27 0 0.0
27 27 0 0.0
.init_array 528 528 0 0.0
528 528 0 0.0
528 528 0 0.0
.rodata 126897 126897 0 0.0
126897 126897 0 0.0
126897 126897 0 0.0
.text 1262130 1262866 736 0.1
1262130 1262866 736 0.1
1262130 1262866 736 0.1
ota-provider-app debug .bss 37440 37440 0 0.0
37440 37440 0 0.0
37440 37440 0 0.0
.data 752 752 0 0.0
752 752 0 0.0
752 752 0 0.0
.data.rel.ro 24488 24488 0 0.0
24488 24488 0 0.0
24488 24488 0 0.0
.dynamic 592 592 0 0.0
592 592 0 0.0
592 592 0 0.0
.got 4016 4016 0 0.0
4016 4016 0 0.0
4016 4016 0 0.0
.init 27 27 0 0.0
27 27 0 0.0
27 27 0 0.0
.init_array 440 440 0 0.0
440 440 0 0.0
440 440 0 0.0
.rodata 110344 110344 0 0.0
110344 110344 0 0.0
110344 110344 0 0.0
.text 1023634 1024338 704 0.1
1023634 1024338 704 0.1
1023634 1024338 704 0.1
ota-requestor-app debug .bss 205696 205696 0 0.0
205696 205696 0 0.0
205696 205696 0 0.0
.data 752 752 0 0.0
752 752 0 0.0
752 752 0 0.0
.data.rel.ro 25832 25832 0 0.0
25832 25832 0 0.0
25832 25832 0 0.0
.dynamic 592 592 0 0.0
592 592 0 0.0
592 592 0 0.0
.got 4144 4144 0 0.0
4144 4144 0 0.0
4144 4144 0 0.0
.init 27 27 0 0.0
27 27 0 0.0
27 27 0 0.0
.init_array 512 512 0 0.0
512 512 0 0.0
512 512 0 0.0
.rodata 128424 128424 0 0.0
128424 128424 0 0.0
128424 128424 0 0.0
.text 1142066 1142770 704 0.1
1142066 1142770 704 0.1
1142066 1142770 704 0.1
shell debug .bss 16136 16136 0 0.0
16136 16136 0 0.0
16136 16136 0 0.0
.data 242 242 0 0.0
242 242 0 0.0
242 242 0 0.0
.data.rel.ro 36496 36496 0 0.0
36496 36496 0 0.0
36496 36496 0 0.0
.dynamic 592 592 0 0.0
592 592 0 0.0
592 592 0 0.0
.got 3528 3528 0 0.0
3528 3528 0 0.0
3528 3528 0 0.0
.init 27 27 0 0.0
27 27 0 0.0
27 27 0 0.0
.init_array 336 336 0 0.0
336 336 0 0.0
336 336 0 0.0
.rodata 76495 76495 0 0.0
76495 76495 0 0.0
76495 76495 0 0.0
.text 599506 600034 528 0.1
599506 600034 528 0.1
599506 600034 528 0.1
tv-app debug .bss 215536 215536 0 0.0
215536 215536 0 0.0
215536 215536 0 0.0
.data 2032 2032 0 0.0
2032 2032 0 0.0
2032 2032 0 0.0
.data.rel.ro 57424 57424 0 0.0
57424 57424 0 0.0
57424 57424 0 0.0
.dynamic 592 592 0 0.0
592 592 0 0.0
592 592 0 0.0
.got 4408 4408 0 0.0
4408 4408 0 0.0
4408 4408 0 0.0
.init 27 27 0 0.0
27 27 0 0.0
27 27 0 0.0
.init_array 608 608 0 0.0
608 608 0 0.0
608 608 0 0.0
.rodata 152072 152072 0 0.0
152072 152072 0 0.0
152072 152072 0 0.0
.text 1454098 1454626 528 0.0
1454098 1454626 528 0.0
1454098 1454626 528 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 bss 112136 112136 0 0.0
rodata 97100 97100 0 0.0
text 577220 577216 -4 -0.0
nrf52840dk_nrf52840+rpc bss 108376 108376 0 0.0
rodata 87876 87876 0 0.0
text 550412 550408 -4 -0.0
nrf5340dk_nrf5340_cpuapp bss 113508 113508 0 0.0
rodata 92340 92340 0 0.0
text 506688 506688 0 0.0
lock-app nrf52840dk_nrf52840 bss 111208 111208 0 0.0
rodata 93500 93500 0 0.0
text 558708 558704 -4 -0.0
nrf5340dk_nrf5340_cpuapp bss 112580 112580 0 0.0
rodata 88760 88760 0 0.0
text 488172 488168 -4 -0.0
pigweed-app nrf52840dk_nrf52840 bss 51824 51824 0 0.0
rodata 45776 45776 0 0.0
text 339456 339456 0 0.0
pump-app nrf52840dk_nrf52840 bss 111268 111268 0 0.0
rodata 94396 94396 0 0.0
text 561860 561860 0 0.0
pump-controller-app nrf52840dk_nrf52840 bss 111204 111204 0 0.0
rodata 93476 93476 0 0.0
text 558500 558496 -4 -0.0
shell nrf52840dk_nrf52840 bss 109072 109072 0 0.0
rodata 72536 72536 0 0.0
text 520316 520312 -4 -0.0
nrf5340dk_nrf5340_cpuapp bss 110056 110056 0 0.0
rodata 67180 67180 0 0.0
text 440924 440924 0 0.0
p6 lock-app default .bss 67176 67176 0 0.0
67176 67176 0 0.0
67176 67176 0 0.0
67176 67176 0 0.0
67176 67176 0 0.0
67176 67176 0 0.0
.data 2416 2416 0 0.0
2416 2416 0 0.0
2416 2416 0 0.0
2416 2416 0 0.0
2416 2416 0 0.0
2416 2416 0 0.0
.heap 963752 963752 0 0.0
963752 963752 0 0.0
963752 963752 0 0.0
963752 963752 0 0.0
963752 963752 0 0.0
963752 963752 0 0.0
.text 1126008 1125960 -48 -0.0
1126008 1125960 -48 -0.0
1126008 1125960 -48 -0.0
1126008 1125960 -48 -0.0
1126008 1125960 -48 -0.0
1126008 1125960 -48 -0.0
qpg lighting-app qpg6100+debug .bss 52416 52416 0 0.0
52416 52416 0 0.0
52416 52416 0 0.0
52416 52416 0 0.0
52416 52416 0 0.0
.data 1000 1000 0 0.0
1000 1000 0 0.0
1000 1000 0 0.0
1000 1000 0 0.0
1000 1000 0 0.0
.text 485052 485052 0 0.0
485052 485052 0 0.0
485052 485052 0 0.0
485052 485052 0 0.0
485052 485052 0 0.0
lock-app qpg6100+debug .bss 51360 51360 0 0.0
51360 51360 0 0.0
51360 51360 0 0.0
51360 51360 0 0.0
51360 51360 0 0.0
.data 956 956 0 0.0
956 956 0 0.0
956 956 0 0.0
956 956 0 0.0
956 956 0 0.0
.text 461208 461208 0 0.0
461208 461208 0 0.0
461208 461208 0 0.0
461208 461208 0 0.0
461208 461208 0 0.0
persistent-storage-app qpg6100+debug .bss 27752 27752 0 0.0
27752 27752 0 0.0
27752 27752 0 0.0
27752 27752 0 0.0
27752 27752 0 0.0
.data 372 372 0 0.0
372 372 0 0.0
372 372 0 0.0
372 372 0 0.0
372 372 0 0.0
.text 149900 149900 0 0.0
149900 149900 0 0.0
149900 149900 0 0.0
149900 149900 0 0.0
149900 149900 0 0.0
telink lighting-app tlsr9518adk80d bss 69940 69940 0 0.0
69940 69940 0 0.0
69940 69940 0 0.0
69940 69940 0 0.0
69940 69940 0 0.0
69940 69940 0 0.0
noinit 33216 33216 0 0.0
33216 33216 0 0.0
33216 33216 0 0.0
33216 33216 0 0.0
33216 33216 0 0.0
33216 33216 0 0.0
text 457680 457666 -14 -0.0
457680 457666 -14 -0.0
457680 457666 -14 -0.0
457680 457666 -14 -0.0
457680 457666 -14 -0.0
457680 457666 -14 -0.0
5 builds (for esp32, mbed)
platform target config section 37c3f7f 92826e8 change % change
esp32 all-clusters-app c3devkit .dram0.bss 59248 59248 0 0.0
.dram0.data 16464 16464 0 0.0
.flash.rodata 199088 199088 0 0.0
.flash.text 877286 877286 0 0.0
.iram0.text 57564 57564 0 0.0
m5stack .dram0.bss 61752 61752 0 0.0
.dram0.data 32084 32084 0 0.0
.flash.rodata 207848 207848 0 0.0
.flash.text 908127 908127 0 0.0
.iram0.text 125115 125115 0 0.0
mbed lighting-app CY8CPROTO_062_4343W+release .bss 171060 171060 0 0.0
.data 5464 5464 0 0.0
.heap 859920 859920 0 0.0
.text 1219256 1219256 0 0.0
lock-app CY8CPROTO_062_4343W+release .bss 169980 169980 0 0.0
.data 5432 5432 0 0.0
.heap 861032 861032 0 0.0
.text 1197216 1197216 0 0.0
pigweed-app CY8CPROTO_062_4343W+release .bss 11760 11760 0 0.0
.data 4360 4360 0 0.0
.heap 1020328 1020328 0 0.0
.text 103064 103064 0 0.0

@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 37c3f7f

File Section File VM
chip-lock.elf device_handles 4 4
chip-lock.elf text -4 -4
chip-shell.elf device_handles 4 4
chip-shell.elf text -4 -4
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_info,0,24467
.debug_str,0,1105
.debug_loc,0,631
.debug_line,0,342
.strtab,0,100
.symtab,0,80
.debug_ranges,0,64
device_handles,4,4
text,-4,-4
.debug_abbrev,0,-121

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

sections,vmsize,filesize
.debug_info,0,9057
.debug_str,0,1105
.debug_loc,0,707
.debug_line,0,344
.strtab,0,179
.symtab,0,80
.debug_ranges,0,64
.debug_abbrev,0,35
device_handles,4,4
.shstrtab,0,1
text,-4,-4

@pullapprove pullapprove bot requested a review from selissia October 26, 2021 15:14
@woody-apple woody-apple merged commit 9a0e4c2 into project-chip:master Oct 27, 2021
@kpschoedel kpschoedel deleted the x10062-clock-type-4a branch October 27, 2021 13:51
JasonLiuZhuoCheng pushed a commit to JasonLiuZhuoCheng/connectedhomeip that referenced this pull request Oct 28, 2021
@kpschoedel @JasonLiuZhuoCheng
Use safer System::Clock types in transport and messaging (project-chi…
5a74cce 
…p#10913)

#### Problem

Code uses plain integers to represent time values and relies on
users to get the unit scale correct.

Part of project-chip#10062 _Some operations on System::Clock types are not safe_

#### Change overview

Convert `src/transport` and `src/messaging` to use the safer `Clock` types.

#### Testing

CI; no change to functionality intended.
Conversion includes `src/messaging/tests/echo/echo_requester.cpp`
and several `src/transport/raw/tests/`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@woody-apple woody-apple woody-apple approved these changes

@msandstedt msandstedt msandstedt approved these changes

@andy31415 andy31415 andy31415 approved these changes

@anush-apple anush-apple Awaiting requested review from anush-apple

@balducci-apple balducci-apple Awaiting requested review from balducci-apple

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

@carol-apple carol-apple Awaiting requested review from carol-apple

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chulspro chulspro Awaiting requested review from chulspro

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@franck-apple franck-apple Awaiting requested review from franck-apple

@harimau-qirex harimau-qirex Awaiting requested review from harimau-qirex

@hawk248 hawk248 Awaiting requested review from hawk248

@holbrookt holbrookt Awaiting requested review from holbrookt

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@LuDuda LuDuda Awaiting requested review from LuDuda

@mlepage-google mlepage-google Awaiting requested review from mlepage-google

@mspang mspang Awaiting requested review from mspang

@pan-apple pan-apple Awaiting requested review from pan-apple

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@tecimovic tecimovic Awaiting requested review from tecimovic

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

@selissia selissia Awaiting requested review from selissia

Assignees
No one assigned
Labels
review - approved transport
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kpschoedel @andy31415 @msandstedt @woody-apple