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

Update bar.sh #201

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

prateekshukla1108
Copy link

Solved the sddm crash problem by changing script to bash which also increased the speed
Changed some of the glyphs (some of them were non scalable so looked small on laptops) Slightly changed the pkg_updates to work on arch linux only (as I don't know how to do it on void linux)
Feel free to modify it

Solved the sddm crash problem by changing script to bash
Changed some of the glyphs (some of them were non scalable so looked small on laptops)
Slightly changed the pkg_updates to work on arch linux only (as I don't know how to do it on void linux)
@siduck
Copy link
Owner

siduck commented Dec 22, 2024

am not logged in dwm rn, can someone test this? @MinePro120

@MinePro120
Copy link
Contributor

Will do. Always wondered why chadwm used dash (@siduck any insights?).

@siduck
Copy link
Owner

siduck commented Dec 22, 2024

Will do. Always wondered why chadwm used dash (@siduck any insights?).

dash is the fastest+lightest shell in existence

@MinePro120
Copy link
Contributor

Will do. Always wondered why chadwm used dash (@siduck any insights?).

dash is the fastest+lightest shell in existence

As it is an extra dependency (for most systems), we will have to assess if the speed difference is worth it.

@siduck
Copy link
Owner

siduck commented Dec 22, 2024

Will do. Always wondered why chadwm used dash (@siduck any insights?).

dash is the fastest+lightest shell in existence

As it is an extra dependency (for most systems), we will have to assess if the speed difference is worth it.

its there on most distros by default i think. the speed is worth it and the script runs so frequently so it must be lightweight

@MinePro120
Copy link
Contributor

I do not approve of this PR at all, for the following reasons:

  1. The previous behaviour of bar.sh was to check for updates during the first iteration of the loop, then every hour. Now the updates are checked every second.
  2. Removal of CPU load
  3. amixer is an extra dependency. Apart from that, a systray utility (e.g. volumeicon on arch) would be way more useful imo.
  4. I don't see any reason to change the glyphs. I use a laptop as well and the glyphs seem fine to me. Could you include some screenshots?

These changes may work for you, but I don't see why they should be pushed. You are free to change your own bar.sh to your liking, of course. About the shells, dash is around 4 ms faster in my case (using the vanilla bar.sh), which is a minuscule amount of time. I propose the switch to bash anyway (or, better, sh), with some other minor improvements (i.e. use BAT* instead of BAT0 or BAT1 for the battery capacity). This would be another PR though. I am also curious about your time measurements. Interrupting the script using CTRL+C can't be used for accurate comparisons.

Kept the default glyphs
removed the volume applet
changed update status to update every 1 hour
@prateekshukla1108
Copy link
Author

I updated the script as you said
the main role of PR was to eliminate the crash in SDDM which is resolved

@prateekshukla1108
Copy link
Author

I do not approve of this PR at all, for the following reasons:

1. The previous behaviour of `bar.sh` was to check for updates during the first iteration of the loop, then every hour. Now the updates are checked every second.

2. Removal of CPU load

3. `amixer` is an extra dependency. Apart from that, a systray utility (e.g. `volumeicon` on arch) would be way more useful imo.

4. I don't see any reason to change the glyphs. I use a laptop as well and the glyphs seem fine to me. Could you include some screenshots?

These changes may work for you, but I don't see why they should be pushed. You are free to change your own bar.sh to your liking, of course. About the shells, dash is around 4 ms faster in my case (using the vanilla bar.sh), which is a minuscule amount of time. I propose the switch to bash anyway (or, better, sh), with some other minor improvements (i.e. use BAT* instead of BAT0 or BAT1 for the battery capacity). This would be another PR though. I am also curious about your time measurements. Interrupting the script using CTRL+C can't be used for accurate comparisons.

as for the time measurements, you are right I didn't knew that. I take my words back

@siduck
Copy link
Owner

siduck commented Dec 23, 2024

but i will not move to bash, i want dash to be used

@siduck
Copy link
Owner

siduck commented Dec 23, 2024

pure posix!

@prateekshukla1108
Copy link
Author

pure posix!

I appreciate your emphasis on speed and everything but the problem is that it crashes sddm on XF86XK_AudioMute. I don't even know why I tried everything in script but it didn't work

@MinePro120
Copy link
Contributor

Bash can be made to be posix compliant if that's needed. The performance boost with dash is close to nonexistent. There is absolutely no reason not to use the default shell imho. Changing shells with such a simple script won't increase performance. Optimizing it, on the other hand, will. @prateekshukla1108 can you open a new issue describing your problem in greater detail?

@siduck
Copy link
Owner

siduck commented Dec 24, 2024

close to non existent

There is a difference, dash is the lightest in ram too and its minimal, is the fastest shell

https://codeberg.org/tplasdio/benchmarks/src/branch/main/noop_startup

@MinePro120
Copy link
Contributor

close to non existent

There is a difference, dash is the lightest in ram too and its minimal, is the fastest shell

https://codeberg.org/tplasdio/benchmarks/src/branch/main/noop_startup

An arbitrary benchmark is of little value to the current scenario. You can measure the performance of each shell yourself, using time. It should be minimal. There is no reason not to use the default shell of each distro (sh). I think we should differentiate between the "personal" needs of each individual from the changes that have a meaningful and positive impact for all users.

@siduck
Copy link
Owner

siduck commented Dec 25, 2024

close to non existent

There is a difference, dash is the lightest in ram too and its minimal, is the fastest shell
https://codeberg.org/tplasdio/benchmarks/src/branch/main/noop_startup

An arbitrary benchmark is of little value to the current scenario. You can measure the performance of each shell yourself, using time. It should be minimal. There is no reason not to use the default shell of each distro (sh). I think we should differentiate between the "personal" needs of each individual from the changes that have a meaningful and positive impact for all users.

image

@MinePro120
Copy link
Contributor

The function of bar.sh is to update the bar every second, not run some commands millions of times consecutively, as your benchmark does. Modify bar.sh so that the bar is updated once (i.e. remove the infinite loop), set iterations to 1 and run the benchmark again. You will see what I am talking about.

@siduck
Copy link
Owner

siduck commented Dec 27, 2024

users can change the shebang themselves if they dont like dash

@MinePro120
Copy link
Contributor

Anyway, OP should be more specific so we can make sure dash is the problem indeed (which I doubt).

@prateekshukla1108
Copy link
Author

Anyway, OP should be more specific so we can make sure dash is the problem indeed (which I doubt).

Initially I doubted it too, so I started removing each part of the script one by one and see if the wm was crashing but I got no results after countless hours of trying, then I tried changing the script to bash and it instantly worked out.

You can reproduce it by running it from sddm and then pressing XF86XK_AudioMute repeatedly for 3 or 4 times.

@MinePro120
Copy link
Contributor

I cannot reproduce it on a clean EndeavourOS VM. I suggest the following steps:

  1. Review the command chadwm runs to toggle mute. By default, it's /usr/bin/pactl set-sink-mute 0 toggle. Try running that a couple of times. Does it actually mute the audio? Does this keybind work in another wm or desktop environment?
  2. I doubt sddm crashes, chadwm probably crashes. I also doubt, again, that the shell or the bar.sh is to blame. Remove bar.sh from run.sh completely and try to mute the audio again.
  3. Be sure to try the above on a clean installation. Clone chadwm again and make sure to use the default configs.

@prateekshukla1108
Copy link
Author

I am getting error but it is not conclusive - Dec 27 23:14:23 archlinux sddm[841]: Session started true
Dec 27 23:14:23 archlinux sddm-helper[462855]: [PAM] Closing session
Dec 27 23:14:23 archlinux sddm-helper[462855]: pam_unix(sddm-greeter:session): session closed for user sddm
Dec 27 23:14:23 archlinux sddm-helper[462855]: [PAM] Ended.
Dec 27 23:14:23 archlinux sddm[841]: Auth: sddm-helper exited successfully
Dec 27 23:14:23 archlinux sddm[841]: Greeter stopped. SDDM::Auth::HELPER_SUCCESS
Dec 27 23:14:23 archlinux systemd[1]: session-c8.scope: Deactivated successfully.
Dec 27 23:14:23 archlinux systemd-logind[785]: Session c8 logged out. Waiting for processes to exit.
Dec 27 23:14:23 archlinux systemd-logind[785]: Removed session c8.
Dec 27 23:14:24 archlinux systemd[461932]: Started dbus-:1.9-org.a11y.atspi.Registry@2.service.

Only thing that have worked for me is changing the shell

@MinePro120
Copy link
Contributor

MinePro120 commented Dec 27, 2024

I don't see any error, it seems like chadwm stopped, one way or another. Did you try the steps I mentioned?

@prateekshukla1108
Copy link
Author

I don't see any error, it seems like chadwm stopped, one way or another. Did you try the steps I mentioned?

Yeah, I tried what you said, I reinstalled everything and tried doing it but there was no progress.
Anyways, I think Im good with the bash version

@MinePro120
Copy link
Contributor

Very weird. I would suggest opening an issue so that other users can possibly contribute. And close this PR, since we aren't switching to bash.

@siduck
Copy link
Owner

siduck commented Dec 28, 2024

@prateekshukla1108 can you show a video of the bug and after the fix?

@prateekshukla1108
Copy link
Author

https://drive.google.com/file/d/1LNsLcBMX-lLiIc7myJm8AtRuXZFzcIzV/view?usp=sharing

here's the video. I also included the same activity in another window manager to show that this is not because of some driver issue or something else but issue with dash.

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