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

DietPi-Services 2.0 | Fine tuning part 2 #2853

Merged
merged 10 commits into from
May 25, 2019
Merged

DietPi-Services 2.0 | Fine tuning part 2 #2853

merged 10 commits into from
May 25, 2019

Conversation

MichaIng
Copy link
Owner

@MichaIng MichaIng commented May 23, 2019

Status: Ready

ToDo:

  • [ ] Instead of allow editing the /lib/systemd/system/ or /etc/init.d/ service files (thus touching maintainer defaults and breaking file updates by APT), create a copy to /etc/systemd/system/ to edit, which overrides the /lib location file. In case of sysvinit either a wrapper needs to be created or a drop-in config to override the auto-generated systemd unit. The drop-in config is actually the safest way in general, allowing easy reverting. A commented copy of the original file could be used as template, so lines to be changed can be un-commented to edit, which makes the done changes transparent. Move to last part 3
  • Handle $2 input executions exactly the same way, simply by adding the input service name as only array entry.
  • Can be further simplified... but need to sleep over this 😉

Commit list/description:

  • DietPi-Services | Create systemd wrappers and drop-in configs inside /etc/systemd/system/ always, leave /lib/systemd/system/ for APT package installs only
  • DietPi-Services | Do not check /usr/lib/systemd/system/ for service files, since this is never used by systemd: https://manpages.debian.org/stretch/systemd/systemd.unit.5.en.html
  • DietPi-Services | Populate_Available_Array() only once each script execution, which renders $INIT_ALL_ARRAYS obsolete. In case values are changed from menu, re-estimate them per-service. This solves the issue that aSERVICE_RESTART_REQUIRED[] is overwritten for all services, e.g. when one gets reset.
  • DietPi-Services | Instead of looping through all known services and checking aSERVICE_AVAILABLE[], unset aSERVICE_NAME[] for all non-existent and excluded services. This reduces array memory size and loop counts vastly.
  • DietPi-Services | Pass aSERVICE_NAME[] always in double quotes as command argument, to be failsafe if service name contains spaces or magic characters
  • DietPi-Services | Merge single + all service modes, by adding the input service to the arrays only in case
  • DietPi-Services | Further coding simplification and enhancements: E.g. process tool arrays are only loaded in menu mode to further speed up regular input modes.
  • DietPi-Services | Align command usage output with Linux standards
  • DietPi-Services | Reorder: Separate "Service Control" and "Process Tool" functions
  • DietPi-Services | Minor coding and wording
  • DietPi-Services | Fix service restart on exit
  • DietPi-Services | Minor wording

+ DietPi-Services | Create systemd wrappers and drop-in configs inside /etc/systemd/system/ always, leave /lib/systemd/system/ for APT package installs only
+ DietPi-Services | Do not check /usr/lib/systemd/system/ for service files, since this is never used by systemd: https://manpages.debian.org/stretch/systemd/systemd.unit.5.en.html
+ DietPi-Services | Populate_Available_Array() only once each script execution, which renders $INIT_ALL_ARRAYS obsolete. In case values are changed from menu, re-estimate them per-service. This solves the issue that aSERVICE_RESTART_REQUIRED[] is overwritten for all services, e.g. when one gets reset.
+ DietPi-Services | Instead of looping through all known services and checking aSERVICE_AVAILABLE[], unset aSERVICE_NAME[] for all non-existent and excluded services. This reduces array memory size and loop counts vastly.
+ DietPi-Services | Pass aSERVICE_NAME[] always in double quotes as command argument, to be failsafe if service name contains spaces or magic characters
@MichaIng MichaIng added the Coding 📑 Improving code stability, performance and consistancy label May 23, 2019
@MichaIng MichaIng added this to the v6.25 milestone May 23, 2019
@MichaIng MichaIng requested a review from Fourdee May 23, 2019 20:06
@MichaIng MichaIng self-assigned this May 23, 2019
@MichaIng
Copy link
Owner Author

MichaIng commented May 23, 2019

Performance tests

  • NB: All services have been excluded since systemctl calls take much longer than code execution, which makes it uncomparable.

pre-PR

root@VM-Stretch:~# time for i in {1..10}; do dietpi-services restart &> /dev/null; done

real    0m1.971s
user    0m1.624s
sys     0m0.072s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services restart &> /dev/null; done

real    0m1.959s
user    0m1.584s
sys     0m0.100s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services restart &> /dev/null; done

real    0m1.976s
user    0m1.608s
sys     0m0.084s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services restart &> /dev/null; done

real    0m1.954s
user    0m1.592s
sys     0m0.088s

post-PR

root@VM-Stretch:~# time for i in {1..10}; do dietpi-services restart &> /dev/null; done

real    0m1.529s
user    0m1.136s
sys     0m0.128s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services restart &> /dev/null; done

real    0m1.561s
user    0m1.128s
sys     0m0.152s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services restart &> /dev/null; done

real    0m1.550s
user    0m1.156s
sys     0m0.124s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services restart &> /dev/null; done

real    0m1.540s
user    0m1.140s
sys     0m0.128s

+ DietPi-Services | Merge single + all service modes, by adding the input service to the arrays only in case
@MichaIng
Copy link
Owner Author

Performance tests: Single service mode (commit 2)

pre-PR

root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start rsync &> /dev/null; done

real    0m1.875s
user    0m1.176s
sys     0m0.196s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start rsync &> /dev/null; done

real    0m1.843s
user    0m1.164s
sys     0m0.200s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start rsync &> /dev/null; done

real    0m1.876s
user    0m1.192s
sys     0m0.192s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start rsync &> /dev/null; done

real    0m1.867s
user    0m1.204s
sys     0m0.164s

post-PR

root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start rsync &> /dev/null; done

real    0m1.325s
user    0m0.780s
sys     0m0.148s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start rsync &> /dev/null; done

real    0m1.330s
user    0m0.800s
sys     0m0.120s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start rsync &> /dev/null; done

real    0m1.313s
user    0m0.800s
sys     0m0.120s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start rsync &> /dev/null; done

real    0m1.335s
user    0m0.800s
sys     0m0.132s

MichaIng added 2 commits May 24, 2019 02:11
+ DietPi-Services | Copy & paste error
+ DietPi-Services | Only print banner when running with input mode, otherwise the menu prompt should be info enough.
+ DietPi-Services | Further coding simplification and enhancements: E.g. process tool arrays are only loaded in menu mode to further speed up regular input modes.
+ DietPi-Services | Align command usage output with Linux standards
@MichaIng
Copy link
Owner Author

MichaIng commented May 25, 2019

a181f57 performance tests

  • NB: Cron handled only. One service is required to compare the process tool array creation time.

pre-commit

root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start &> /dev/null; done

real    0m1.832s
user    0m1.192s
sys     0m0.172s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start &> /dev/null; done

real    0m1.863s
user    0m1.140s
sys     0m0.244s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start &> /dev/null; done

real    0m1.816s
user    0m1.168s
sys     0m0.188s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start &> /dev/null; done

real    0m1.825s
user    0m1.164s
sys     0m0.192s

post-commit

root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start &> /dev/null; done

real    0m1.679s
user    0m1.128s
sys     0m0.164s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start &> /dev/null; done

real    0m1.665s
user    0m1.112s
sys     0m0.164s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start &> /dev/null; done

real    0m1.667s
user    0m1.132s
sys     0m0.156s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start &> /dev/null; done

real    0m1.666s
user    0m1.092s
sys     0m0.192s
  • This was without actual process tool use for the service. When custom settings were applied, reading those contain 6 grep + sed calls for each service. Although there is no measurable difference in case of a single service:
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start &> /dev/null; done

real    0m1.831s
user    0m1.136s
sys     0m0.220s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start &> /dev/null; done

real    0m1.819s
user    0m1.152s
sys     0m0.192s
root@VM-Stretch:~# rm /etc/systemd/system/cron.service.d/dietpi-process_tool.conf
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start &> /dev/null; done

real    0m1.807s
user    0m1.124s
sys     0m0.216s
root@VM-Stretch:~# time for i in {1..10}; do dietpi-services start &> /dev/null; done

real    0m1.861s
user    0m1.152s
sys     0m0.224s

MichaIng added 6 commits May 25, 2019 19:21
+ DietPi-Services | Reorder: Separate "Service Control" and "Process Tool" functions
+ DietPi-Services | Minor coding and wording
+ DietPi-Services | Fix service restart on exit
+ DietPi-Services | Minor wording
+ DietPi-Services | Correct I/O priority default
+ DietPi-Services | Tiny
+ DietPi-Services | Another tiny alignment fix and align very short service names (e.g. cron) correctly as well
@MichaIng MichaIng merged commit aef5ee3 into dev May 25, 2019
@MichaIng MichaIng deleted the dietpi-services branch May 25, 2019 19:19
# - [Service] line throws an effectless error that we can simply hide.
# - All values are single word strings, so bash assigns them correctly.
local Nice CPUAffinity CPUSchedulingPolicy CPUSchedulingPriority IOSchedulingClass IOSchedulingPriority
. $fp &> /dev/null
Copy link
Collaborator

Choose a reason for hiding this comment

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

@MichaIng

Nice, I did think of adding #!/bin/bash to our drop in service, but interesting its not needed 👍

Copy link
Owner Author

Choose a reason for hiding this comment

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

@Fourdee
Hehe nope for sourcing it's not required. Actually it is even ignored. When sourcing scripts, they are always executed with the originating shell, of course, since no new shell is initiated.

Only downside is that custom user changes "might" be a danger. Although all systemd unit syntax is var=value style, so the worst thing is that we source some inerte variables and/or get some more silenced syntax errors.

@Fourdee
Copy link
Collaborator

Fourdee commented May 26, 2019

@MichaIng

Legend 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Coding 📑 Improving code stability, performance and consistancy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants