Skip to content

polledTimeout: add option to use CPU count instead of millis() #5870

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

Merged
merged 47 commits into from
Apr 5, 2019

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Mar 13, 2019

No description provided.

@d-a-v d-a-v requested a review from devyte March 13, 2019 00:58
Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

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

Is there some way to check for an overflow timeout when using CPU cycles?

Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

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

As a general question: what is the use case? I see two slight advantages and one big disadvantage of the enhancement:
Pros:

  • better precision for detection of expiration (1 cycle is a fraction of a ns, vs. 1ms for millis)
  • faster polling (the code is lighter, so a for() or while() loop will run faster and hence poll at a higher frequency

Con:

  • Rollover time is drastically reduced from something like several days to a minute or so for 80MHz, and half that for 160MHz.

Neither Pro nor Con:

  • CPU busy waiting will be more or less the same
  • Power usage will be more or less the same
  • Usage is the same

@d-a-v
Copy link
Collaborator Author

d-a-v commented Mar 15, 2019

Is there some way to check for an overflow timeout when using CPU cycles?

I'm afraid not.

rollover is 26 seconds @160Mhz. timeMax() is introduced, set to 13s. (or 26 @80Mhz)
comments added.

Misleading type names are removed (YieldOrSkip).
Unclear names *Cpu are renamed do *CycleMs so it now clearly needs milliseconds.

Con:

  • Rollover time is drastically reduced from something like several days to a minute or so for 80MHz, and half that for 160MHz.

Yes, comments are added to use it only when it is known to be checked often

*Neither Pro nor Con:

  • CPU busy waiting will be more or less the same
  • Power usage will be more or less the same

Yes, it is not intended for busy waiting but for periodic testing only. Maybe I should remove oneShotCycleMs ?

  • Usage is the same

This is a Pro :)

About performance, millis() uses 356 cpu cycles, while the other one only 3 or 4 cpu cycles.
I intend to extend ScheduleFunctions with periodic calls. Periodicity will also optionally be checked from inside yield() (only from cont stack of course). I could'nt resolve me to use phat millis() there, at every yield/loop. CycleMs does the same job about 100 times faster.

@d-a-v d-a-v requested a review from devyte March 18, 2019 09:11
Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

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

That off-by-one is the last thing, but I'm not sure how to fix it.

@d-a-v d-a-v requested a review from devyte March 21, 2019 14:05
Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

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

Done! Great job :)

@devyte devyte merged commit 9a2ed27 into esp8266:master Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants