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

Restore timeout in JNI event loop #8821

Closed

Conversation

kpschoedel
Copy link
Contributor

@kpschoedel kpschoedel commented Aug 5, 2021

Problem

The event loop in JNI IOThreadMain() used to wait no more than ten
seconds on each iteration. This timeout was temporarily removed in
PR #6561.

Change overview

Add an explicit timer.

Also removes another obsolete TODO.

Fixes #7756 add a timer for sleepTime.tv_sec = 10; sleepTime.tv_usec = 0;
Fixes #7757 Integrate timer platform details with WatchableEventManager.

Testing

Passes Android workflow.

#### Problem

The event loop in JNI `IOThreadMain()` used to wait no more than ten
seconds on each iteration. This timeout was temporarily removed in
PR project-chip#6561.

#### Change overview

Add an explicit timer.

Fixes project-chip#7756 _add a timer for sleepTime.tv_sec = 10; sleepTime.tv_usec = 0;_

#### Testing

Passes Android workflow.
@mspang
Copy link
Contributor

mspang commented Aug 5, 2021

Why is this needed?

Copy link
Contributor

@mspang mspang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like some more context, anything that depends on this is 1) going to have a 10 second latency associated with which isn't desirable and 2) is obviously not managing its wakeups properly.

@kpschoedel
Copy link
Contributor Author

Why is this needed?

It may well not be. Before the socket-event PR, the select() in that code had a 10 second timeout; I have no knowledge of the reason. I'll just as happily remove the TODO without adding a timer.

@mspang
Copy link
Contributor

mspang commented Aug 5, 2021

Why is this needed?

It may well not be. Before the socket-event PR, the select() in that code had a 10 second timeout; I have no knowledge of the reason. I'll just as happily remove the TODO without adding a timer.

Yes, I've seen it, and it raised my eyebrow. We don't want to wake up the stack every 10 seconds and drain the battery for no reason.

No sane code should need this.

@kpschoedel
Copy link
Contributor Author

No sane code should need this.

Given that it hasn't been present for a month, no insane code needs it either.

I'll just prune the obsolete TODOs in some future cleanup PR.

@kpschoedel kpschoedel closed this Aug 5, 2021
@kpschoedel kpschoedel deleted the x7725-system-event-6-7756 branch August 5, 2021 22:27
@github-actions
Copy link

github-actions bot commented Aug 5, 2021

Size increase report for "esp32-example-build" from 6b6c2ee

File Section File VM
chip-ipv6only-app.elf .flash.text -172 -172
chip-lock-app.elf .flash.text 64 64
chip-temperature-measurement-app.elf .flash.text 60 60
chip-all-clusters-app.elf .flash.text -80 -80
chip-shell.elf .flash.text -24 -24
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

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

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

sections,vmsize,filesize

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

sections,vmsize,filesize

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

sections,vmsize,filesize
.flash.text,64,64
[Unmapped],0,-64

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

sections,vmsize,filesize
.flash.text,60,60
[Unmapped],0,-60

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

sections,vmsize,filesize

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

sections,vmsize,filesize
[Unmapped],0,80
.flash.text,-80,-80

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

sections,vmsize,filesize
[Unmapped],0,24
.flash.text,-24,-24


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants