Fixed O(N^2) performance when handling --bind flags #628
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'll need some help from the bubblewrap maintainers to land this change. Please, review this carefully. I'm not completely sure does it break something or not. But seems like it's okay
I ran the tests that already exist in the project, and they passed. I also launched various software from flatpak. I replaced the system bwrap with the ones I built, with and without algorithm changes. In both, I output the contents of mountinfo at the very end of
setup_newroot()
execution. The list of mount points and their flags seem to be exactly the same on the data I've tried.It improves the complexity to O(N log N). I also have tried to measure performance difference:
flatpak run com.microsoft.Edge
with and without optimizationA test where I pass bwrap 2000 "--bind" flags:
bwrap --dev-bind / / --bind /usr /tmp/1 --bind /usr /tmp/2 ... --bind /usr /tmp/2000 /bin/true
Below I'll describe how algorithm basically works:
First, our problem isn't interactive type, because we have all
bind
operations and their order in arguments. The algorithm use it. Instead of rechecking/proc/self/mountinfo
before every--bind
operation we:After all the mount operations completed, we call
bind_mount_fixup()
function to remount all the mount points with correct flags, that:/proc/self/mountinfo
mountinfo
, requests correct flags from graph and remounts them if neededAlso, when we collect mount operations, we also add to the list all mount's of
proc
,dev
,mqueue
,tmp
. That's because we will anyway collect them from mount info to the graph. And added operation prevents us from change flags on those mount to incorrect.Also, when we bind file we create temp file as before. But before we delete them right after they become unneeded. In my Pull Request we save all paths of those temp files and
unlink()
all of them later, right after we performedbind_mount_fixup()
. That's because if we delete them, their soft links becomes incorrect and this breakretrieve_kernel_case()
function.Read this.
Below is more context about lazy propagation of flags:
If we propagate flags naively with DFS algorithm we'll still have O(N^2) complexity.
That's why the main part of algorithm is lazy propagation on the graph.
After tree of mount points is built (step 2):