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
samba: build with profiling enabled #82509
Conversation
@GrahamcOfBorg test samba |
@GrahamcOfBorg test samba |
@Izorkin What is the impact on performance? Is this something that can be enabled by default without degrading performance to much? |
Small check Is there a correct method to check? |
I don't know what the correct methodnto measure this would be, just wanted to check there is not s huge impact. If you don't see one it's fine with me. |
The diff is hard to read. Could you format the derivation so that there is one dependency per line without any other changes (e.g. using nixpkgs-fmt), and then rebase this PR onto that? |
@jtojnar update PR. |
70cac8e
to
182bf0d
Compare
@jtojnar checked PR? |
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.
Looks good to me, though I wanted to wait for someone who actually uses this package.
@jtojnar don`t need to squashed PR? |
No need for squashing. When there are non-trivial changes, it is nice to have them split into multiple commits. Not only is that easier to review but also, it will be easier to figure out why each line was introduced in future debugging. More descriptive commit messages would be nice, thought ( |
That's correct? |
Fixed
|
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.
Cherry-picked this locally and was able to run my samba server with these changes. Note that profiling is off by default and has to be turned on so I am not too worried about the overhead of compiling this support in - didn't notice much difference locally but also didn't get any hard numbers.
Motivation for this change
Add support monitoring with netdata - https://docs.netdata.cloud/collectors/python.d.plugin/samba
Cleanup build configuration and removed unused packages. Checked here - https://wiki.samba.org/index.php/Package_Dependencies_Required_to_Build_Samba
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)