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

Add pkg/pwalk, make Chcon() faster #89

Merged
merged 4 commits into from Mar 11, 2020

Conversation

kolyshkin
Copy link
Collaborator

This is a continuation of #85 and containers/common#82

TL;DR: This adds a parallel wrapper on top of filetree.Walk(), enabling multiple callbacks to be used concurrently. This is proven to speed things up. The resulting implementation is used for Chcon(), making it about 1.5x faster.

Please see more in commit messages. For previous discussions, see the abovementioned PRs.

NOTE that this is not as fast as #85. The reason is we use standard filepath.Walk() here, which performs the (unneeded) os.Stat() call on every entry, and also sorts the directory entries (which also does not make sense in many cases). So, yes, we can do better, but that means either another third-party package (such as github.com/karrick/godirwalk) or rewriting the filepath.Walk() from scratch. I'm still on the verge as whether we should do it, and am open to any suggestions.

1. Fix the documentation (we can't use markdown in it).

2. Return earlier if recurse is not set.

3. Inline the callback function.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
On my system, the benchmark shows about 1.7s with the dataset
being Linux kernel source tree. `time chcon` gives me about 1.2s.

How to test:
```
cd go-selinux
mkdir testdata
cp -a ~/git/linux testdata # use Linux kernel source as testdata
go test -c -tags selinux . # compile the test
./go-selinux.test -test.bench . -test.benchtime 10s
```

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@rhatdan
Copy link
Collaborator

rhatdan commented Mar 10, 2020

LGTM
But Lint is not happy.
@giuseppe @vrothberg @mrunalp PTAL

Approved with PullApprove

This is a parallel filetree.Walk() implementation. Please
see README.md and package doc for details.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This gives me 1.5x speedup in comparison with filepath.Walk,
making the code as fast as chcon(3).

I am sure we can do better than that by eliminating os.Stat()
syscall and directory entries sorting done by filepath.Walk(),
but this means either another third-party package (such as
github.com/karrick/godirwalk) or rewriting the filepath.Walk()
from scratch. I'm still on the verge as whether we should do it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Copy link
Collaborator

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM, great work

@rhatdan rhatdan merged commit 6cc0cfd into opencontainers:master Mar 11, 2020
@kolyshkin
Copy link
Collaborator Author

Once a release is made, I can pick this up in containers/storage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants