write-qt-apps-hook.sh: use make-binary-wrapper for significant speedups#225881
write-qt-apps-hook.sh: use make-binary-wrapper for significant speedups#225881RaitoBezarius merged 1 commit intoNixOS:stagingfrom
Conversation
QT apps tend to call makeWrapper with a lot of arguments, which causes noticable slowdowns (+100ms for app startup). The slow down boils down to two reasons: - the required string processing is O(N^2) - bash is slow at doing the processing By using the binary wrapper, we fix the second point, brining the overhead down from 100ms to just 4ms or thereabouts. I tested this change by rebuilding my whole system with it (I use plasma). It booted and it works (that's where I type this commit message), but I am not 100% sure if this won't break anything else. Closes: NixOS#225871
github-actions
bot
added
the
6.topic: qt/kde
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
SuperSandro2000
left a comment
SuperSandro2000
left a comment
There was a problem hiding this comment.
Should be none breaking if all arguments are supported. Probably needs to go to staging.
|
Can I change the base branch to EDIT: ah, that's explained in the manual, retargeted |
|
Should be fine, but I'd probably keep this for after 23.05 as it's the kind of change that has spooky implications. |
ofborg
bot
added
10.rebuild-darwin: 101-500
If all things build, this should just work but it could be that some things use flags not supported by the binary wrapper which would trigger a build failure. binary wrapper is already used by wrapGAppsHook and more things on darwin due to that the shebang must be a binary. |
|
I'm expecting it to be fine, but will it be "last staging cycle before release" fine? I'm less confident. |
RaitoBezarius
left a comment
RaitoBezarius
left a comment
There was a problem hiding this comment.
Let's try, but we should revert in case of large issues.
|
Is there any plan to simplify the wrapper logic? Is it possible? It is not only slow but also as big as ~40KB for one bash wrapper ( |
|
Do you mean the binary or the shell wrapper? The binary wrapper should already be a lot more optimized. The shell wrapper could probably be greatly optimized if we didn't check for every entry if the first and final : are there but since we want to deduplicate the entries it will probably stay longish. Maybe we could try to reduce the amount of entries added or symlinkJoin beforehand. |
|
I think tdesktop is broken now? |
|
I encountered the same |
|
Never mind, tdesktop shoots itself in the foot: #228410 |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
also kills gnuplot-qt: |
|
Fix for |
|
A better fix is already in staging: #229154 |
8 participants
Description of changes
QT apps tend to call makeWrapper with a lot of arguments, which causes noticable slowdowns (+100ms for app startup). The slow down boils down to two reasons:
By using the binary wrapper, we fix the second point, brining the overhead down from 100ms to just 4ms or thereabouts.
I tested this change by rebuilding my whole system with it (I use plasma). It booted and it works (that's where I type this commit message), but I am not 100% sure if this won't break anything else.
Closes: #225871
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)