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.

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

Move socket watch APIs into System::Layer #9086

Merged
merged 10 commits into from
Aug 23, 2021

Conversation

kpschoedel
Copy link
Contributor

Problem

Transitionally, System::Layer operations are provided by a
configuration-specific WatchableEventManager, but they are not yet
sufficiently aligned to convert to abstract and implementation classes.

Part of #7715 Virtualize System and Inet interfaces

Change overview

  • Move socket event watch APIs into System::Layer.
  • Remove WatchableSocket and its include-file hack that allowed
    implementation-specific state to be contained directly in EndPoints.
  • Revise the select() implementation to use an internal pool for watch
    state in place of the EndPoint-embedded WatchableSocket.
  • Revise the libevent sample implementation to use libevent timers
    directly instead of the System::Timer pool. The libevent
    implementation uses STL containers with heap-allocated state for the
    sake of being different from the select()-based implementation.
  • Added/moved documentation comments into headers.

Testing

CI; no change to functionality should is expected.

#### Problem

Transitionally, `System::Layer` operations are provided by a
configuration-specific `WatchableEventManager`, but they are not yet
sufficiently aligned to convert to abstract and implementation classes.

Part of project-chip#7715 Virtualize System and Inet interfaces

#### Change overview

- Move socket event watch APIs into `System::Layer`.
- Remove `WatchableSocket` and its include-file hack that allowed
  implementation-specific state to be contained directly in EndPoints.
- Revise the select() implementation to use an internal pool for watch
  state in place of the EndPoint-embedded WatchableSocket.
- Revise the libevent sample implementation to use libevent timers
  directly instead of the System::Timer pool. The libevent
  implementation uses STL containers with heap-allocated state for the
  sake of being different from the select()-based implementation.

#### Testing

CI; no change to functionality should is expected.
@boring-cyborg boring-cyborg bot added the darwin label Aug 18, 2021
@msandstedt msandstedt self-requested a review August 19, 2021 22:06
@kpschoedel kpschoedel force-pushed the x7715-system-layer-1 branch from 5f27728 to 94d8bb2 Compare August 19, 2021 23:03
@github-actions
Copy link

Size increase report for "gn_qpg-example-build" from 3acbf59

File Section File VM
chip-qpg6100-lighting-example.out .text 32 32
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_loc,0,227
.strtab,0,126
.debug_abbrev,0,74
.symtab,0,64
.debug_frame,0,32
.text,32,32
.debug_aranges,0,8
.debug_ranges,0,8
.shstrtab,0,2
.debug_str,0,-4
[Unmapped],0,-32
.debug_info,0,-374
.debug_line,0,-1127

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'


@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 3acbf59

File Section File VM
chip-shell.elf [LOAD #3 [RW]] 0 29
chip-shell.elf device_handles 4 4
chip-shell.elf text -20 -20
chip-shell.elf bss 0 -29
chip-lock.elf device_handles 4 4
chip-lock.elf text -20 -20
chip-lock.elf bss 0 -32
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_loc,0,1390
.strtab,0,152
[LOAD #3 [RW]],29,0
.debug_aranges,0,8
.debug_ranges,0,8
device_handles,4,4
.symtab,0,-16
text,-20,-20
bss,-29,0
.debug_frame,0,-48
.debug_abbrev,0,-1115
.debug_str,0,-1746
.debug_line,0,-2466
.debug_info,0,-23711

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

sections,vmsize,filesize
.debug_loc,0,1566
.strtab,0,152
.debug_aranges,0,8
.debug_ranges,0,8
device_handles,4,4
.symtab,0,-16
text,-20,-20
bss,-32,0
.debug_frame,0,-48
.debug_abbrev,0,-1216
.debug_str,0,-1746
.debug_line,0,-7242
.debug_info,0,-40570


@github-actions
Copy link

Size increase report for "esp32-example-build" from 3acbf59

File Section File VM
chip-lock-app.elf .flash.text 52 52
chip-all-clusters-app.elf .flash.text 32 32
chip-bridge-app.elf .flash.text 4 4
chip-temperature-measurement-app.elf .flash.text 52 52
chip-shell.elf .flash.text 88 88
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_abbrev,0,175
.strtab,0,126
.flash.text,52,52
.symtab,0,32
.debug_frame,0,24
.debug_aranges,0,8
.debug_ranges,0,8
.shstrtab,0,-2
[Unmapped],0,-52
.debug_info,0,-403
.debug_loc,0,-404
.debug_line,0,-876

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

sections,vmsize,filesize
.debug_str,0,-7
.debug_info,0,-8
.debug_line,0,-17

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

sections,vmsize,filesize
.debug_loc,0,620
.debug_abbrev,0,177
.strtab,0,126
.flash.text,32,32
.symtab,0,32
.debug_frame,0,16
.debug_aranges,0,8
.debug_ranges,0,8
.riscv.attributes,0,1
.debug_str,0,-2
.shstrtab,0,-2
[Unmapped],0,-32
.debug_info,0,-213
.debug_line,0,-843

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

sections,vmsize,filesize
.debug_abbrev,0,220
.strtab,0,126
.symtab,0,32
.debug_frame,0,24
.debug_aranges,0,8
.debug_ranges,0,8
.flash.text,4,4
.debug_str,0,1
.shstrtab,0,-2
[Unmapped],0,-4
.debug_loc,0,-297
.debug_info,0,-443
.debug_line,0,-853

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

sections,vmsize,filesize
.debug_str,0,-7
.debug_info,0,-8
.debug_line,0,-17

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

sections,vmsize,filesize
.debug_abbrev,0,212
.strtab,0,126
.debug_loc,0,67
.flash.text,52,52
.symtab,0,32
.debug_frame,0,24
.debug_aranges,0,8
.debug_ranges,0,8
.shstrtab,0,2
.debug_str,0,-3
[Unmapped],0,-52
.debug_info,0,-455
.debug_line,0,-853

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

sections,vmsize,filesize
.debug_abbrev,0,171
.strtab,0,126
.flash.text,88,88
.symtab,0,32
.debug_frame,0,24
.debug_aranges,0,8
.debug_ranges,0,8
.shstrtab,0,2
.debug_str,0,1
[Unmapped],0,-88
.debug_loc,0,-228
.debug_info,0,-343
.debug_line,0,-445

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

sections,vmsize,filesize
.debug_str,0,15
.debug_info,0,-16
.debug_line,0,-51


@woody-apple
Copy link
Contributor

@saurabhst ?

src/system/LwIPEventSupport.h Show resolved Hide resolved
@andy31415 andy31415 merged commit 82cf1b1 into project-chip:master Aug 23, 2021
@kpschoedel kpschoedel deleted the x7715-system-layer-1 branch August 23, 2021 14:25
sharadb-amazon pushed a commit to sharadb-amazon/connectedhomeip that referenced this pull request Aug 23, 2021
* Move socket watch APIs into System::Layer

#### Problem

Transitionally, `System::Layer` operations are provided by a
configuration-specific `WatchableEventManager`, but they are not yet
sufficiently aligned to convert to abstract and implementation classes.

Part of project-chip#7715 Virtualize System and Inet interfaces

#### Change overview

- Move socket event watch APIs into `System::Layer`.
- Remove `WatchableSocket` and its include-file hack that allowed
  implementation-specific state to be contained directly in EndPoints.
- Revise the select() implementation to use an internal pool for watch
  state in place of the EndPoint-embedded WatchableSocket.
- Revise the libevent sample implementation to use libevent timers
  directly instead of the System::Timer pool. The libevent
  implementation uses STL containers with heap-allocated state for the
  sake of being different from the select()-based implementation.

#### Testing

CI; no change to functionality should is expected.

* restyle

* fix GetCommandExitStatus race

* Enforce use of API via System::Layer

* also include dispatch case

* add LwIP case

* restyle
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this pull request Sep 7, 2021
#### Problem

82cf1b1 (PR project-chip#9086) could clobber a returned error code with a
different value `GetCommandExitStatus()`, which could lead to `chip-tool`
exiting successfully when it shouldn't. (This doesn't actually happen
at present because nothing sets `err` after `RunCommand()`, but could
be triggered by otherwise innocuous code changes.)

#### Change overview

Check for an existing error in preference to `GetCommandExitStatus()`.

#### Testing

Manual run of `chip-tool` with edits.
woody-apple pushed a commit that referenced this pull request Sep 7, 2021
* Don't clobber chip-tool error

#### Problem

82cf1b1 (PR #9086) could clobber a returned error code with a
different value `GetCommandExitStatus()`, which could lead to `chip-tool`
exiting successfully when it shouldn't. (This doesn't actually happen
at present because nothing sets `err` after `RunCommand()`, but could
be triggered by otherwise innocuous code changes.)

#### Change overview

Check for an existing error in preference to `GetCommandExitStatus()`.

#### Testing

Manual run of `chip-tool` with edits.

* stop task before getting status
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this pull request Sep 9, 2021
* Don't clobber chip-tool error

#### Problem

82cf1b1 (PR project-chip#9086) could clobber a returned error code with a
different value `GetCommandExitStatus()`, which could lead to `chip-tool`
exiting successfully when it shouldn't. (This doesn't actually happen
at present because nothing sets `err` after `RunCommand()`, but could
be triggered by otherwise innocuous code changes.)

#### Change overview

Check for an existing error in preference to `GetCommandExitStatus()`.

#### Testing

Manual run of `chip-tool` with edits.

* stop task before getting status
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants