Skip to content

Add an implementation-defined change threshold exceeded check #180

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 5 commits into from
Feb 2, 2023

Conversation

anssiko
Copy link
Member

@anssiko anssiko commented Feb 1, 2023

  • Define "contributing factors change threshold exceeded" steps
  • Call into these steps from the "change in data" steps
  • Add an informative note for implementation considerations

Fix #176


Preview | Diff

- Define "contributing factors change threshold exceeded" steps
- Call into these steps from the "change in data" steps
- Add an informative note for implementation considerations

Fix #176
index.html Outdated
</ol>
<aside class="note">
The [=contributing factors change threshold exceeded=] algorithm allows user agents to avoid
flip-flopping between two states in certain circumstances. This specification does not define
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe clarify "certian"

Copy link
Member Author

Choose a reason for hiding this comment

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

I expanded this further with an abstract example.

- Expand the note with an abstract example
- Clarify steps: Changes in [=contributing factors=] represented by |factors|
@anssiko
Copy link
Member Author

anssiko commented Feb 1, 2023

@kenchris thanks for the comments. I pushed an update, PTAL.

This was a bit hard to define in abstract without defining the concrete algorithm normatively, which I don't want to do. Thus I proposed that short two-step algorithm and an accompanying note that tries to explain what's the problem and what implementers should do to improve. I think the significance of the changes to contributing factors need to be considered collectively. That is, a significant change in one metric may not exceed the threshold for the entire system if other metrics remain within their own bounds.

Also, I wanted this to remain implementation-defined and thus I tried to avoid saying explicitly "if CPU utilization fluctuates around a boundary" to not steer implementers to think that is the metric to use.

Please take another look. Further suggestions welcome.

- Refactor "contributing factors change threshold exceeded"
  into "change in contributing factors is substantial"

- Require no argument passing to this abstract algorithm to
  clarify and streamline related prose, refer to the current
  pressure state instead
- Refer to this internal slot in prose
- Note: [[State]] represents the current pressure state
@anssiko
Copy link
Member Author

anssiko commented Feb 2, 2023

The PR has been updated with further refinements, PTAL. I think I'm soon satisfied with this.

Copy link
Contributor

@kenchris kenchris left a comment

Choose a reason for hiding this comment

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

Good job, this is very hard describing

- We want to match the underlying hardware platform's behavior
@@ -916,7 +943,8 @@ <h3>Supporting algorithms</h3>
Let |record:PressureRecord| be |observer|.{{PressureObserver/[[LastRecordMap]]}}[|source|].
Copy link
Contributor

Choose a reason for hiding this comment

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

we should really rename this to lastRecord, it just confused me.

@kenchris kenchris merged commit 2e5d949 into main Feb 2, 2023
@anssiko anssiko deleted the change-threshold-exceeded branch February 2, 2023 10:46
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.

Add an implementation-defined change threshold exceeded check
3 participants