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

Request SCHED_RR using CAP_SYS_NICE and add Python to nix dev shell #2690

Merged
merged 5 commits into from
Jul 23, 2023
Merged

Request SCHED_RR using CAP_SYS_NICE and add Python to nix dev shell #2690

merged 5 commits into from
Jul 23, 2023

Conversation

ry-diffusion
Copy link
Contributor

Describe your PR, what does it fix/add?

  • Add missing python3 dep to nix develop
  • Change Hyprland process scheduling strategy to SCHED_RR using pthread

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

Also require the maintainers to add SYS_CAP_NICE capability to the package

Is it ready for merging, or does it need work?

I don't think that adding python3 directly into devShell is a good solution, but, works.

@vaxerski
Copy link
Member

why

@ry-diffusion
Copy link
Contributor Author

why

It makes Hyprland more responsive when the computer is under CPU stress, and python is required because libudis86 needs it installed

Example:

$ nix develop
$ meson setup build
# ...
| -- Could NOT find Python3 (missing: Python3_EXECUTABLE Interpreter)
| Building itab.c/itab.h...
| -- Configuring done (1.3s)
| CMake Error at libudis86/CMakeLists.txt:30 (add_library):
| Cannot find source file:
| itab.c

| Tried extensions .c .C .c++ .cc .cpp .cxx .cu .mpp .m .M .mm .ixx .cppm .h
| .hh .h++ .hm .hpp .hxx .in .txx .f .F .for .f77 .f90 .f95 .f03 .hip .ispc


| CMake Error at libudis86/CMakeLists.txt:30 (add_library):
| No SOURCES given to target: libudis86
# ...

https://blog.martin-graesslin.com/blog/2017/09/kwinwayland-goes-real-time/
swaywm/sway#6994

src/init/initHelpers.cpp Outdated Show resolved Hide resolved
@jbeich
Copy link
Contributor

jbeich commented Jul 13, 2023

Replacing SCHED_RESET_ON_FORK with pthread_atfork (like Sway) makes it work on FreeBSD with mac_priority(4). For example, cursor in Hyprland no longer decelerates when running poudriere in background.

--- a/src/init/initHelpers.cpp
+++ b/src/init/initHelpers.cpp
@@ -5,11 +5,23 @@ bool Init::isSudo() {
 }
 
 void Init::gainRealTime() {
-#ifdef SCHED_RESET_ON_FORK
     const int   minPrio = sched_get_priority_min(SCHED_RR);
     sched_param param   = {.sched_priority = minPrio};
 
+#ifdef SCHED_RESET_ON_FORK
     if (pthread_setschedparam(pthread_self(), SCHED_RR | SCHED_RESET_ON_FORK, &param))
         Debug::log(WARN, "Failed to change process scheduling strategy");
+#else
+    if (pthread_setschedparam(pthread_self(), SCHED_RR, &param)) {
+        Debug::log(WARN, "Failed to change process scheduling strategy");
+        return;
+    }
+
+    pthread_atfork(NULL, NULL, []() {
+        struct sched_param param = {.sched_priority = 0};
+
+        if (pthread_setschedparam(pthread_self(), SCHED_OTHER, &param))
+            Debug::log(WARN, "Failed to reset process scheduling strategy");
+    });
 #endif
 }
\ No newline at end of file

@ry-diffusion
Copy link
Contributor Author

if (pthread_setschedparam(pthread_self(), SCHED_RR, &param)) {
+        Debug::log(WARN, "Failed to change process scheduling strategy");
+        return;
+    }

hm, I think that if-guard SCHED_RESET_ON_FORK can be removed, because pthread_atfork method also works on Linux, but SCHED_RESET_ON_FORK looks more integrated with Linux, what do you think?

@jbeich
Copy link
Contributor

jbeich commented Jul 15, 2023

Up to @vaxerski. pthread_atfork gives slightly more control, and dropping SCHED_RESET_ON_FORK can reduce maintenance while avoiding build/runtime bugs on non-Linux.

@vaxerski
Copy link
Member

I don't care

@vaxerski vaxerski force-pushed the main branch 2 times, most recently from 09bd49b to ce9c5fd Compare July 18, 2023 22:28
@vaxerski
Copy link
Member

whats the status on this? ready or not

Copy link
Contributor

@jbeich jbeich left a comment

Choose a reason for hiding this comment

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

SCHED_RR part looks OK. I'm not familar with Nix to review.

@vaxerski
Copy link
Member

@fufexan

Copy link
Member

@fufexan fufexan left a comment

Choose a reason for hiding this comment

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

flake lgtm

@vaxerski vaxerski merged commit 9fc5f4c into hyprwm:main Jul 23, 2023
8 checks passed
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.

4 participants