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

Add not-in-use computing prefs #4871

Merged
merged 23 commits into from
Sep 21, 2022
Merged

Add not-in-use computing prefs #4871

merged 23 commits into from
Sep 21, 2022

Conversation

davidpanderson
Copy link
Contributor

Fixes #41 (!)
Previously there were single prefs for

  • % of CPUs
  • % of CPU time
  • suspend if non-BOINC CPU load above X
    These applied whether or not the computer was in use.
    This PR adds separate prefs for when the computer is not in use.
    It includes both web-based and Manager interfaces for editing them.
    Backwards compatible; if the new prefs are not specified, it uses the old ones for both cases.

    niu_cpu_usage_limit: CPU throttling while computer not in use
    niu_max_ncpus_pct: # CPUs to use while computer not in use
    niu_suspend_cpu_usage: suspend if non-BOINC usage while computer not in use

Client: parse these from global_prefs files, and enforce them.
    If not specified, use the corresponding (non-niu) pref all the time.

Web: add them to the computing prefs pages.
    Refactor the prefs into:
    in use / not in use / tasks / disk / network

TODO: update the Manager prefs dialog accordingly
use the values from corresponding in-use pref
Note: the Manager code is written in a clunky way.
I cleaned up one instance. TODO: clean up others.
suspendComputingStaticBox, ID_DEFAULT,
_("Suspend when no mouse/keyboard input in last"),
// CPU throttling
//
Copy link
Contributor

Choose a reason for hiding this comment

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

Precede line 375 with /*xgettext:no-c-format*/

Copy link
Contributor

Choose a reason for hiding this comment

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

@davidpanderson You have not fixed this. For an explanation why this is needed, see here and here.

(Of course the /*xgettext:no-c-format*/ directive must now precede line 376 in the latest version of this file.)

@AenBleidd
Copy link
Member

On Windows it's failing on this:

         sim.cpp
    27>D:\a\boinc\boinc\client\sim.cpp(302,24): error C2065: 'ncpus': undeclared identifier [D:\a\boinc\boinc\win_build\sim_vs2019.vcxproj]
    27>D:\a\boinc\boinc\client\sim.cpp(350,24): warning C4459: declaration of 'delta' hides global declaration [D:\a\boinc\boinc\win_build\sim_vs2019.vcxproj]
       D:\a\boinc\boinc\client\sim.cpp(91,26): message : see declaration of 'delta' [D:\a\boinc\boinc\win_build\sim_vs2019.vcxproj]
    27>D:\a\boinc\boinc\client\sim.cpp(405,26): warning C4458: declaration of 'apps' hides class member [D:\a\boinc\boinc\win_build\sim_vs2019.vcxproj]
       D:\a\boinc\boinc\client\client_state.h(75,18): message : see declaration of 'CLIENT_STATE::apps' [D:\a\boinc\boinc\win_build\sim_vs2019.vcxproj]
    27>D:\a\boinc\boinc\client\sim.cpp(415,36): error C2065: 'ncpus': undeclared identifier [D:\a\boinc\boinc\win_build\sim_vs2019.vcxproj]
    27>D:\a\boinc\boinc\client\sim.cpp(456,32): warning C4477: '_snprintf' : format string '%lu' requires an argument of type 'unsigned long', but variadic argument 1 has type 'unsigned __int64' [D:\a\boinc\boinc\win_build\sim_vs2019.vcxproj]
       D:\a\boinc\boinc\client\sim.cpp(456,32): message : consider using '%zu' in the format string [D:\a\boinc\boinc\win_build\sim_vs2019.vcxproj]
    27>D:\a\boinc\boinc\client\sim.cpp(1498,12): error C2039: 'set_ncpus': is not a member of 'CLIENT_STATE' [D:\a\boinc\boinc\win_build\sim_vs2019.vcxproj]
       D:\a\boinc\boinc\client\client_state.h(71): message : see declaration of 'CLIENT_STATE' [D:\a\boinc\boinc\win_build\sim_vs2019.vcxproj]

On linux it's failing on:

DlgAdvPreferencesBase.cpp: In member function ‘wxPanel* CDlgAdvPreferencesBase::createProcessorTab(wxNotebook*)’:
DlgAdvPreferencesBase.cpp:335:9: error: cannot bind non-const lvalue reference of type ‘wxString&’ to an rvalue of type ‘wxString’
 335 |         wxString (_("Suspend computing when your computer is busy running other programs.")),
     |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
DlgAdvPreferencesBase.cpp:205:45: note:   initializing argument 2 of ‘void CDlgAdvPreferencesBase::addNewRowToSizer(wxSizer*, wxString&, wxWindow*, wxWindow*, wxWindow*, wxWindow*, wxWindow*)’
 205 |                 wxSizer* toSizer, wxString& toolTipText,
     |                                   ~~~~~~~~~~^~~~~~~~~~~
make[2]: *** [Makefile:1158: boincmgr-DlgAdvPreferencesBase.o] Error 1

Copy link
Contributor

@CharlieFenton CharlieFenton left a comment

Choose a reason for hiding this comment

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

@AenBleidd This is a substantial amount of added and changed code. I will try to review this within the next week.

@AenBleidd
Copy link
Member

@CharlieFenton, I completely agree with you. I don't push you to make it just immediately. But this change is quite significant so I believe your review is essential.

@RichardHaselgrove
Copy link
Contributor

It would reassure me while testing if the scheduler at SETI@home (or another project with the provisional web selection tools) could be re-activated so that I can test the automatic propagation of global preferences from project to project (propagation considers the time and project of the most recent update).

@davidpanderson
Copy link
Contributor Author

I fixed the compile error. Oddly, it wasn't an error in VS

@davidpanderson
Copy link
Contributor Author

I fixed the sim build error

@codecov
Copy link

codecov bot commented Aug 7, 2022

Codecov Report

Merging #4871 (f627b09) into master (a036709) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #4871      +/-   ##
============================================
- Coverage     10.43%   10.42%   -0.02%     
  Complexity     1046     1046              
============================================
  Files           278      278              
  Lines         35827    35864      +37     
  Branches       8113     8127      +14     
============================================
  Hits           3739     3739              
- Misses        31717    31754      +37     
  Partials        371      371              
Impacted Files Coverage Δ
lib/prefs.cpp 0.00% <0.00%> (ø)
lib/prefs.h 0.00% <ø> (ø)
lib/cc_config.cpp 0.00% <0.00%> (ø)
sched/db_dump.cpp 0.00% <0.00%> (ø)
sched/validator.cpp 0.00% <0.00%> (ø)
sched/sched_main.cpp 0.00% <0.00%> (ø)
sched/sched_util.cpp 0.00% <0.00%> (ø)
sched/sched_types.cpp 0.00% <0.00%> (ø)
sched/sched_resend.cpp 0.00% <0.00%> (ø)
sched/sched_result.cpp 0.00% <0.00%> (ø)
... and 4 more

@davidpanderson
Copy link
Contributor Author

I changed the web code to fix 1 and (I think) 5.
I changed the way NIU prefs are parsed, fixing 3 and 4.

6 doesn't happen. When the client gets prefs from a project,
it puts them in global_prefs.xml and doesn't modify it.
It only modified global_prefs_override.xml

7: the dialog is now 644 pixels high on Win. That's short enough.
We can't support tiny displays, and they can edit prefs other ways.

9: the web form says "Requires BOINC 7.20.3+"

10: works AFAIK

 - replace code lost from commit 27aff32
 - restore previous values from global_prefs.xml or global_prefs_override.xml
@CharlieFenton
Copy link
Contributor

@davidpanderson I have confirmed you have fixed all the items on my list (except [6] which probably has no solution.

I found a couple of additional problems, but I have fixed them in the code myself. (For some reason, a couple of the changes you made in commit 27aff32 were lost; I replaced them.)

Regarding item 6, you wrote:

6 doesn't happen. When the client gets prefs from a project,
it puts them in global_prefs.xml and doesn't modify it.
It only modified global_prefs_override.xml

I'm sorry if I was not clear. I was referring only to changes on a project web site, so the propagation happens via global_prefs.xml. I agree that updating the client transfers the updated web prefs to global_prefs.xml, which is in effect updating it.

When you then update another project, that project's web prefs receive the updated data from global_prefs.xml. The scenario I asked about was when a user is running BOINC 7.20.2 or earlier, which does not save the not-in-use prefs from the first project in global_prefs.xml. As a result, the second project does not receive the not-in-use prefs and sets them to the in-use prefs (the default.) This scenario does not involve global_prefs_override.xml at all; the customizing I mentioned is done entirely on project websites.

But, as I wrote earlier, I suspect there is no fix for this and it is not critical.

BOINC Client/Manager automation moved this from In Review to Reviewed Sep 6, 2022
@AenBleidd
Copy link
Member

Code looks ok to me. I still would like to check it in a week when I'll have access to the linux laptop with 768 pixels height just to see how new Preferences windows looks like on it, then we could probably close the corresponding ticket as well.
If nothing new appears before the end of the next week, I'll merge this PR and will update Transifex to get new strings localized.
Until then please keep this PR open.
Thanks

@CharlieFenton
Copy link
Contributor

@davidpanderson Please remember to update the documentation, and add the missing description of Suspend when no mouse/keyboard input in last xx minutes .

@davidpanderson
Copy link
Contributor Author

I added the "no input in X min" to the mediawiki page
(which had a number of other errors, some of which I fixed).
I'll update this to the new version when we release it.

@AenBleidd
Copy link
Member

For Ubuntu with the screen resolution 1366x768 the window is too big (the same as for the current version).
Not sure if we need to do smth with it in this particular PR.
Old version:
Screenshot from 2022-09-13 11-19-47
New version:
Screenshot from 2022-09-13 11-20-14

@AenBleidd
Copy link
Member

I suggest to go ahead and merge this, and fix the size issue later in the corresponding ticket instead #1872
@davidpanderson, @CharlieFenton, what do you think?

@CharlieFenton
Copy link
Contributor

I suggest to go ahead and merge this, and fix the size issue later in the corresponding ticket instead #1872

I agree.

@RichardHaselgrove
Copy link
Contributor

Likewise. I agree.

@davidpanderson
Copy link
Contributor Author

On Linux can you drag the dialog to make the bottom visible?

@AenBleidd
Copy link
Member

No. The window is already on its top most position.

@AenBleidd
Copy link
Member

As discussed, I'm merging it now.
I have an idea how to solve this issue #1872, so I'll take a couple of weeks to try it out before pushing new strings for localization to Transifex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

differentiate between number of CPUs to use in idle and busy mode
5 participants