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.

Sign up for GitHub

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

Jump to bottom

reduce the complexity of hostsctl_export #8

Closed
wants to merge 0 commits into from
Closed

reduce the complexity of hostsctl_export #8

wants to merge 0 commits into from

Conversation

sudoforge
Copy link
Contributor

@sudoforge sudoforge commented Oct 20, 2018 •
edited
Loading

this patch refactors hostsctl_export, reducing the overall complexity of
the function, as well as reducing the excecution time by about 50%.

closes gh-7

before

λ repeat 30 { time sudo hostsctl export } | grep 'total$'
sudo hostsctl export  0.11s user 0.02s system 109% cpu 0.121 total
sudo hostsctl export  0.10s user 0.04s system 109% cpu 0.124 total
sudo hostsctl export  0.11s user 0.03s system 106% cpu 0.129 total
sudo hostsctl export  0.10s user 0.03s system 106% cpu 0.119 total
sudo hostsctl export  0.10s user 0.03s system 107% cpu 0.124 total
sudo hostsctl export  0.10s user 0.03s system 109% cpu 0.122 total
sudo hostsctl export  0.10s user 0.03s system 108% cpu 0.124 total
sudo hostsctl export  0.10s user 0.04s system 109% cpu 0.130 total
sudo hostsctl export  0.11s user 0.03s system 108% cpu 0.122 total
sudo hostsctl export  0.11s user 0.03s system 108% cpu 0.126 total
sudo hostsctl export  0.11s user 0.03s system 108% cpu 0.123 total
sudo hostsctl export  0.10s user 0.02s system 106% cpu 0.119 total
sudo hostsctl export  0.10s user 0.03s system 106% cpu 0.115 total
sudo hostsctl export  0.11s user 0.03s system 107% cpu 0.129 total
sudo hostsctl export  0.10s user 0.02s system 106% cpu 0.121 total
sudo hostsctl export  0.10s user 0.04s system 108% cpu 0.121 total
sudo hostsctl export  0.10s user 0.03s system 106% cpu 0.119 total
sudo hostsctl export  0.11s user 0.03s system 109% cpu 0.125 total
sudo hostsctl export  0.09s user 0.04s system 109% cpu 0.121 total
sudo hostsctl export  0.09s user 0.04s system 106% cpu 0.123 total
sudo hostsctl export  0.11s user 0.03s system 107% cpu 0.124 total
sudo hostsctl export  0.10s user 0.03s system 105% cpu 0.125 total
sudo hostsctl export  0.10s user 0.03s system 107% cpu 0.120 total
sudo hostsctl export  0.10s user 0.03s system 105% cpu 0.121 total
sudo hostsctl export  0.11s user 0.01s system 106% cpu 0.115 total
sudo hostsctl export  0.09s user 0.04s system 107% cpu 0.120 total
sudo hostsctl export  0.10s user 0.03s system 107% cpu 0.116 total
sudo hostsctl export  0.11s user 0.04s system 106% cpu 0.140 total
sudo hostsctl export  0.10s user 0.03s system 107% cpu 0.123 total
sudo hostsctl export  0.10s user 0.03s system 106% cpu 0.126 total

after

λ repeat 30 { time sudo ./bin/hostsctl export } | grep 'total$'
sudo ./bin/hostsctl.sh export  0.04s user 0.02s system 108% cpu 0.048 total
sudo ./bin/hostsctl.sh export  0.02s user 0.03s system 120% cpu 0.045 total
sudo ./bin/hostsctl.sh export  0.04s user 0.01s system 119% cpu 0.045 total
sudo ./bin/hostsctl.sh export  0.04s user 0.01s system 120% cpu 0.047 total
sudo ./bin/hostsctl.sh export  0.05s user 0.01s system 123% cpu 0.048 total
sudo ./bin/hostsctl.sh export  0.04s user 0.02s system 127% cpu 0.048 total
sudo ./bin/hostsctl.sh export  0.04s user 0.02s system 121% cpu 0.043 total
sudo ./bin/hostsctl.sh export  0.03s user 0.02s system 126% cpu 0.044 total
sudo ./bin/hostsctl.sh export  0.05s user 0.01s system 123% cpu 0.045 total
sudo ./bin/hostsctl.sh export  0.05s user 0.01s system 126% cpu 0.047 total
sudo ./bin/hostsctl.sh export  0.04s user 0.02s system 129% cpu 0.047 total
sudo ./bin/hostsctl.sh export  0.04s user 0.01s system 125% cpu 0.043 total
sudo ./bin/hostsctl.sh export  0.04s user 0.02s system 126% cpu 0.041 total
sudo ./bin/hostsctl.sh export  0.03s user 0.02s system 120% cpu 0.045 total
sudo ./bin/hostsctl.sh export  0.03s user 0.02s system 124% cpu 0.044 total
sudo ./bin/hostsctl.sh export  0.03s user 0.02s system 121% cpu 0.044 total
sudo ./bin/hostsctl.sh export  0.05s user 0.01s system 124% cpu 0.047 total
sudo ./bin/hostsctl.sh export  0.05s user 0.00s system 122% cpu 0.041 total
sudo ./bin/hostsctl.sh export  0.03s user 0.02s system 118% cpu 0.047 total
sudo ./bin/hostsctl.sh export  0.04s user 0.02s system 123% cpu 0.046 total
sudo ./bin/hostsctl.sh export  0.04s user 0.01s system 122% cpu 0.043 total
sudo ./bin/hostsctl.sh export  0.05s user 0.01s system 124% cpu 0.046 total
sudo ./bin/hostsctl.sh export  0.04s user 0.01s system 121% cpu 0.040 total
sudo ./bin/hostsctl.sh export  0.05s user 0.01s system 123% cpu 0.044 total
sudo ./bin/hostsctl.sh export  0.04s user 0.01s system 125% cpu 0.046 total
sudo ./bin/hostsctl.sh export  0.03s user 0.03s system 129% cpu 0.045 total
sudo ./bin/hostsctl.sh export  0.03s user 0.02s system 124% cpu 0.047 total
sudo ./bin/hostsctl.sh export  0.04s user 0.02s system 123% cpu 0.049 total
sudo ./bin/hostsctl.sh export  0.05s user 0.01s system 124% cpu 0.047 total
sudo ./bin/hostsctl.sh export  0.04s user 0.02s system 122% cpu 0.045 total

@pigmonkey
Copy link
Owner

This is not properly excluding enabled hosts. For instance, I have analytics.google.com enabled.

$ hostsctl list-enabled | grep analytics.google.com
● analytics.google.com

On master, this host is excluded from export.

$ hostsctl export | grep -i analytics.google.com

On this branch, the host is included in the output, which will result in it being disabled.

$ ./hostsctl.sh export | grep -i analytics.google.com
0.0.0.0 analytics.google.com

@sudoforge
Copy link
Contributor Author

sudoforge commented Oct 23, 2018 •
edited
Loading

Hm, I didn't catch that in my initial testing -- good eye. This was due to the --line-regexp option passed to grep, which forces whole-line matches, but is actually unecessary here, because of the combination of --file and --fixed-strings:

-F, --fixed-strings
    Interpret  PATTERN as a list of fixed strings (instead of regular expressions),
    separated by newlines, any of which is to be matched.

-f FILE, --file=FILE
    Obtain patterns from FILE, one per line.  If this option is used multiple times
    or is combined with the -e  (--regexp) option, search for all patterns given. The
    empty file contains zero patterns, and therefore matches nothing.

In summary, each newline in /etc/hostsctl/enabled.hosts will be imported as PATTERN, which in turn is interpreted as a literal string instead of a regex. I amended my commit to remove the unnecessary and error-causing --line-regexp option.

@pigmonkey
Copy link
Owner

I think we do actually need to treat each disabled host as a wholeline match.

The default remote file includes disabling entries for both pro.ip-api.com and ip-api.com. I have enabled ip-api.com. In the current master, the export function properly excludes ip-api.com and includes a blocked entry pro.ip-api.com.

$ hostsctl list-enabled | grep ip-api
● ip-api.com
hostsctl list-disabled | grep ip-api
● pro.ip-api.com
$ hostsctl export | grep ip-api
0.0.0.0 pro.ip-api.com

In your branch, the export function excludes both ip-api.com and pro.ip-api.com, causing both hosts to be enabled.

$ ./hostsctl.sh export | grep ip-api

We need to match only ^ip-api.com$.

@sudoforge
Copy link
Contributor Author

Gah, I didn't think to check a substring like that. I must be misinterpreting the language around --fixed-strings then, specifically that the strings will be "separated by newlines" - I took this to mean that pattern would end up being ^string$, but that's clearly not the case.

I'll take another stab at this tonight.

@sudoforge sudoforge closed this Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The complexity of hosts_export should be reduced
2 participants
@sudoforge @pigmonkey