-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
chore: optimize termux_step_get_dependencies #24567
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
Conversation
45445be
to
fd00a88
Compare
fd00a88
to
3f47560
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it, I can confirm this is faster. I thought that termux_step_get_dependencies()
was pretty fast already, but this change can add up to a significant benefit since it runs a lot.
This change makes it faster mostly for package rebuilding since I modified only this codepath. |
Well, seems like $ time for ((i=0; i<1000000; i++)); do :; done
real 0m1.017s
user 0m1.017s
sys 0m0.000s
$ time for ((i=0; i<1000000; i++)); do [ -f /dev/null ]; done
real 0m1.949s
user 0m1.629s
sys 0m0.319s
$ time for ((i=0; i<1000000; i++)); do [[ -f /dev/null ]]; done
real 0m1.389s
user 0m1.075s
sys 0m0.314s |
3f47560
to
f1d793a
Compare
Also I placed post-timestamp-creation sleep with check if it is needed or 1 second already passed. Should reduce building of each package by 1 second. I checked all packages overriding |
3032780
to
3b2e89a
Compare
I will merge this in 24 hours if nobody minds or after it gets 2+ more approves. |
78b1da8
to
6bb8318
Compare
6bb8318
to
acc1065
Compare
da397e2
to
c2c9a0d
Compare
c2c9a0d
to
4067602
Compare
Ok, I'll merge it in 20 hours if nobody minds or after it gets 2+ more approves. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to approve this earlier.
Go ahead.
Wait, let me test how this all works with termux-pacman repos. |
…imestamp_file Removed external commands invocations where possible. United some conditions under single test invocation. Refactored termux_extract_dep_info to avoid re-sourcing build.sh. Modified package extraction sequence to avoid storage hit and avoid spawning two more commands for checking if in-tar files are prefixed with `./` or not. Modern `tar` compares timestamps with microsecond precision so we do not need to sleep for a whole second to create a correct timestamp, 100 ms should be enough
4067602
to
a5df859
Compare
Removed external commands invocations where possible.
United some conditions under single test invocation
Refactored termux_extract_dep_info to avoid re-sourcing build.sh.
Modified package extraction sequence to avoid storage hit and avoid spawning two more commands for checking if in-tar files are prefixed with
./
or not.With these changes
termux_step_dependencies
execution time drops from12s
to7s
duringkf6-breeze-icons
rebuilding.Also it reduces
~0.1s
-~2.0s
per package during archive extracting step (depends on package size), which is good for packages with lots of dependencies (likekf6-breeze-icons
).