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

Reactive policies #74

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

bumbarad
Copy link

@bumbarad bumbarad commented Dec 7, 2021

Contains reactive policies. They take single argument, that is temperute level.
Bets_policy.hpp contains general policy framework, which reacts on temperature threshold overstepping and increases "level" variable. Higher level means more strict policy application. If level != 0, virtual function execute_policy is executed. Otherwise it acts like imx8_per_window policy.
Bets_skip.hpp contains skipping policy. Iterates over all tasks in each window and skips up to 5 of them (to keep one running), based on "level" variable.
Bets_soft_throttle.hpp reduces frequency of tasks in each window by 1 - 3 levels based on "level" variable.

…which turns on and off policies based on given temperature. Bets_skip performs skipping of a single task in each window while temperature is above threshold. Bets_soft_throttle reduces frequencies to closest lower frequency when threshold is overstepped. Bets_hard_throttle set frequencies to minimum when temperature is above threshold.
…tures over last 10 measurements, reduced temperature window.
@codecov-commenter
Copy link

Codecov Report

Merging #74 (073f056) into master (51086b7) will decrease coverage by 1.60%.
The diff coverage is 2.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #74      +/-   ##
==========================================
- Coverage   36.39%   34.79%   -1.61%     
==========================================
  Files          46       50       +4     
  Lines        2459     2578     +119     
  Branches     1117     1172      +55     
==========================================
+ Hits          895      897       +2     
- Misses       1094     1209     +115     
- Partials      470      472       +2     
Impacted Files Coverage Δ
src/power_policy/_power_policy.cpp 22.22% <0.00%> (-1.59%) ⬇️
src/power_policy/bets_hard_throttle.hpp 0.00% <0.00%> (ø)
src/power_policy/bets_policy.hpp 0.00% <0.00%> (ø)
src/power_policy/bets_skip.hpp 0.00% <0.00%> (ø)
src/power_policy/bets_soft_throttle.hpp 0.00% <0.00%> (ø)
src/window.hpp 100.00% <ø> (ø)
src/window.cpp 75.00% <50.00%> (-3.13%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51086b7...073f056. Read the comment docs.

Copy link
Member

@wentasah wentasah left a comment

Choose a reason for hiding this comment

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

Sending feedback as we agreed in today's call.

protected:
// currently, this is hardcoded for i.MX8, but should be quite simply extensible
// for other CPU cluster layouts
std::array<CpufreqPolicy *, 2> policies{ &pm.get_policy("policy0"), &pm.get_policy("policy4") };
Copy link
Member

Choose a reason for hiding this comment

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

Bylo by lepší to předělat, aby to nebylo platformě závislé. Podobně jako per_process.hpp byla upravena z imx8_per_process.hpp.

Comment on lines +24 to +25
float upper_threshold;
float lower_threshold;
Copy link
Member

Choose a reason for hiding this comment

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

V komentáři (nebo rovnou ve jménu proměnné) zmínit, že se jedna o teplotu.

Comment on lines +26 to +27
time_t timer;
const time_t limit = 10;
Copy link
Member

Choose a reason for hiding this comment

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

time_t je definován jako čas v sekundách. Myslím, že bychom někdy mohli chtít mít ten čas i kratší než sekundu, tak bych použil čas v milisekundách, jako se používá ve zbytku demosu.

Bylo by lepší použít C++ datové typy ze std::chrono, konkrétně asi time_point a duration.

float lower_threshold;
time_t timer;
const time_t limit = 10;
bool temperature_inertia_period = false;
Copy link
Member

Choose a reason for hiding this comment

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

Je divné, že něco, co se nazývá period je typu bool. V názvu proměnné (asi) chybí sloveso.

Bets_policy(const std::string &temperature_threshold)
{
upper_threshold = std::stof(temperature_threshold);
lower_threshold = upper_threshold - 1;
Copy link
Member

Choose a reason for hiding this comment

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

Bylo by lepší, kdyby to byl 2. (nepovinný) parametr.

Comment on lines +74 to +75
load(win);
sort_candidates();
Copy link
Member

Choose a reason for hiding this comment

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

Myslím, že load a sort_candidate není potřeba volat pokaždé, ale stačilo by to vykonat jednou na začátku. Myslím, že metoda validate je ideální místo, kde by se to mělo udělat.

this->level = 5;
}
int skip_counter = 0;
for (int j = 0; j < win.slices.size(); j++){
Copy link
Member

Choose a reason for hiding this comment

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

Když iterujete přes container, je přehlednější používat tzv. range-based for: for (auto &slice : win.slices) slice.to_be_skipped = false;.

}
load(win);
sort_candidates();
// Iterate trough candidates, until up to the level amounth of them is skipped, but at least 1 is kept.
Copy link
Member

Choose a reason for hiding this comment

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

Proč je potřeba 1 nechávat? Když by se to hodně přehřívalo, je možné klidně přeskočit všechny.

Comment on lines +28 to +40
void sort_candidates(){
std::sort(candidates.begin(), candidates.end(), cmp);
}

void safe_pushback(std::string key){
for (auto &p : candidates){
if (p.first == key){
return;
}
}
candidates.push_back(std::pair<std::string, int>(key, 0));
}

Copy link
Member

Choose a reason for hiding this comment

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

Pokud se použijí datové sktuktury navržené výše, tohle bude zbytečné.

Comment on lines +52 to +59
int is_present(std::string key){
for (int i = 0; i < present.size(); i++){
if (present[i] == key){
return i;
}
}
return -1;
}
Copy link
Member

Choose a reason for hiding this comment

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

Pokud se použijí datové sktuktury navržené výše, is_present bude zbytečné.

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