-
Notifications
You must be signed in to change notification settings - Fork 272
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
Use multiprocessing to speed up bundletool #698
Conversation
These functions use no Bundler object state and can be pure functions.
0f887a1
to
541a4b5
Compare
For more convenient review: https://github.com/bazelbuild/rules_apple/pull/698/files?w=1 |
Disclaimer - I have not yet looked at the the patch, but from experience trying to move things into a parallel step based on the number of CPUs – The danger here is going to be what happens if you need to build multiple things ( In cases like that, bazel can end up running one bundling job per core because it also is scaling things out. If you then scale out to cpu count within the script, you can end up going to cpu count squared. Which might not be too bad on a laptop or mini with say 4-8 core, but as you start to get to machines with 16 or more cores, you can actually end up running out of process entries for the average user, causing the build to fail. I believe there is an open FR on bazel where it would be nice if a rule/action could indicate it need a resource budge (CPU, RAM, etc.) and then as part of of the action being run, it gets told it's assigned budget. |
Thanks for the fast reply. I did think about that, my reasoning is that the file copying is very little cpu and mostly I/O. I wrote a
and I ran this for about ~4000 files and it completed in something like 1s. This isn't an exact apples to apples comparison, but I do believe it's not a problem in practice for file copying since it doesn't seem to be a cpu needy operation. We have a large number of resource bundles in our app, many of which are competing with bazel actions being run concurrently. These are much smaller than the |
Yes that's bazelbuild/bazel#10443. It's marked as P2 and nobody has talked about it so I don't know if or when it will get addressed. |
@thomasvl should this be behind a feature? |
How big are the files in your tests? (ie - the copies may be so fast you don't really get that many copies going in parallel) From experience, it will work until it doesn't and when it doesn't it take a while to figure out because the next build works because of the work was done so there are less jobs and things don't max out. Having an action where bazel will fail and just issuing the command again isn't really living up to the bazel promise. 😃 |
Relatedly I've filed bazelbuild/bazel#10702 to find out about the Bazel overhead that shows in the trace around the action. |
"{Fast, Correct} - Choose two" |
I don't think this would happen with the bundletool because it deletes the output directory at the outset. I think any such performance issues would be repeatable. |
For what it's worth, in a Swift build many or all |
If there were multiple bundles going in parallel, some could succeed while others fail, so the next "try" would have less, and the rest would succeed. (been there, done that) 😄
Yup, and this is why the hardcoded values in there can't be touched even though we might be able to do better. They appear to be mostly working, and we can't really mess with them without risking something going wrong. The things I had done like this before all worked great for in some cases years, it wasn't until they got run on new hardware that suddenly things hit the limits because the numbers/loads/ram changed. Taking a look at the core/thread counts on some of the new MacPros, and we could be in for some hiccups with local builds on those machines (even in the current things being done in parallel). |
Thanks for the discussion @thomasvl. Looks like we're going to use a custom bundletool, so I'll close this out. |
In our experience, the bundling step is a bottleneck in tight edit/compile/{run,test} cycles. On my machine, it can take ~14s which is almost as long as link, and can be longer than incremental compilation.
Part of the problem is Bazel overhead, and part of the problem is that bundletool copies files serially. This change makes use of the
multiprocessing
module to copy files into the bundle in parallel. Of the ~14s mentioned above, about ~5s is Bazel overhead and the remaining 8-9s is the bundletool. With this change, bundletool runs in about 5-6s (most of which is code signing).In order to use
apply_async
, all stateless methods were moved off theBundler
class and converted to free functions. This is because using a method would require supporting picking for theBundler
class. The change from methods to functions is in its own commit.