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

Any way to speed up builds on Windows? #4

Closed
pkgw opened this issue Jan 11, 2021 · 13 comments
Closed

Any way to speed up builds on Windows? #4

pkgw opened this issue Jan 11, 2021 · 13 comments

Comments

@pkgw
Copy link
Collaborator

pkgw commented Jan 11, 2021

For Tectonic, the Windows vcpkg builds are now timing out about 75% of the time because of the 60-minute job time limit for the Azure Pipeline build agents that we use. The lion's share of that time is taken up by the cargo vcpkg build of Tectonic's dependencies. I don't suppose there are any options that we could add/activate that would reduce how long things take? In particular, in the past I've noticed that vcpkg sometime builds both debug and release versions of its outputs ... does that happen to be the case with cargo vcpkg build?

@pkgw
Copy link
Collaborator Author

pkgw commented Jan 12, 2021

As it happens, I just got some evidence that maybe cargo vcpkg build is creating both debug and release artifacts. For Tectonic I've been testing a workaround where I do the cargo vcpkg build operations in a different job than the main build, and I ran into an error while trying to delete the ICU temporaries:

+ rm -rf target/vcpkg/buildtrees target/vcpkg/downloads
rm: cannot remove 'target/vcpkg/buildtrees/icu/x64-windows-static-dbg/common': Directory not empty
rm: cannot remove 'target/vcpkg/buildtrees/icu/x64-windows-static-dbg/data': Directory not empty
[...]
rm: cannot remove 'target/vcpkg/buildtrees/icu/x64-windows-static-rel/common': Directory not empty
rm: cannot remove 'target/vcpkg/buildtrees/icu/x64-windows-static-rel/data': Directory not empty
[...]

(I'm not sure why I can't rm -rf the ICU build tree, but that's a separate issue. Files might be getting marked read-only or something?)

@pkgw
Copy link
Collaborator Author

pkgw commented Jan 12, 2021

Looks like this is surprisingly poorly supported in vcpkg?

microsoft/vcpkg#143

@mcgoo
Copy link
Owner

mcgoo commented Jan 12, 2021

The vcpkg (and cargo vcpkg) default is to build both debug and release as you say. I don't know how to link Rust code to the debug build, so it has always been extra work for nothing. I missed the moment to make it the default for cargo vcpkg, but I'd like it to be available at the very least.

It's not that hard to request it - cargo-vcpkg would just have to add set(VCPKG_BUILD_TYPE release) to the triplet file on the fly. It will make the vcpkg tree dirty all the time so changing the rev in Cargo.toml will need a stash-pull-pop shuffle I suppose.

Apparently there has been some work done on caching in vcpkg but I have not tried it.

@pkgw
Copy link
Collaborator Author

pkgw commented Jan 12, 2021

Or is there a convenient way to create and use a custom triplet file that's kept outside of the vcpkg repo, maybe?

@mcgoo
Copy link
Owner

mcgoo commented Jan 14, 2021

I'm still thinking about what the best way to add this capability is, but I think it's just a matter of adding a flag to the metadata.

If you want to fix this straight away, you could fork the vcpkg tree and add the set(VCPKG_BUILD_TYPE release) line to the required triplets and change the url= and branch= in Cargo.toml.

That said, I'll try and get some way to do it out shortly.

@pkgw
Copy link
Collaborator Author

pkgw commented Jan 14, 2021

Yeah, things seem to be under control in the Tectonic CI for now so I see no need to fork, but I'm definitely interested to test out any potential fixes.

(That being said, the vcpkg builds seem to be taking longer and longer ... seems like the msys servers are getting slower or something.)

The other thing specific to Tectonic is that several of the deps, including some slow-to-build ones like iconv, are coming from fontconfig, and I'm not sure if we even need that on Windows. Now that Tectonic's multi-crate organization is coming together I'm in a better position to experiment on that front.

@mcgoo mcgoo transferred this issue from mcgoo/vcpkg-rs Feb 2, 2021
@pkgw
Copy link
Collaborator Author

pkgw commented Sep 19, 2022

Resurrecting this issue after a long period of inactivity. Nothing fundamental seems to have changed: vcpkg still defaults to building both debug and release; CI builds for Tectonic/Windows are annoyingly long; and the best way to avoid building debug artifacts seems to be to set VCPKG_BUILD_TYPE.

Oh, and I still agree that it looks like we could fix this in Tectonic by forking the vcpkg tree and customizing the Windows triplet file in the repo. It's just that that feels like a very heavyweight solution to the issue.

@mcgoo have you had any further ideas about the cleanest way to potentially implement this functionality? If there are benefits to building from a clean vcpkg repo it seems like some kind of external triplet file would be the best approach, but I don't know much about the vcpkg design or implementation.

(One thing that has changed is that I'm definitely in a better position to investigate if we can build on Windows without fontconfig, which I am 95% sure would work. But I haven't done that yet.)

(update: Hmm ok yeah we'd definitely need new code to avoid Fontconfig on Windows, which is not looking practical at the moment.)

@waych
Copy link
Collaborator

waych commented Oct 9, 2022

I'm not certain, but sadly, I fear @mcgoo may not be with us any longer. He told me here on github that he was sick before handing me push rights on his repo when I was fixing the CI and link ordering issues last year. The account has been inactive ever since :-(

Regarding approaches to your problem, it seems like having a way to either override triplets easily would make things work well. I've noticed that since this issue was originally opened, vcpkg has added a --overlay-triplets flag. The idea is that this flag is given a directory path with triplet files, and this directory is searched before searching the upstream vcpkg repo's triplet dir. This feels like the right way to approach things as it gives a lot of power to configure things back to the user independent of what is officially supported upstream.

The alternative where vcpkg-rs/cargo-vcpkg try to own the overlay-triplet dir itself seems like a non-starter. For one, vcpkg only seems to support a single dir of overlay. This could be fixed upstream, but even then, maintaining a new set of triplets here, or doing the work required to generate them at build (in turn requiring all the configuration potential to be duplicated) also seems like a lot of unnecessary work.

I haven't worked out yet how both vcpkg-rs and cargo-vcpkg would need to change to properly support "bring your own triplets dir" but it probably isn't too much effort. I've only started playing with the flag myself but my cmake is terrible so I haven't made much progress. Probably only cargo-vcpkg would need to change, as vcpkg-rs I don't recall ever actually invokes upstream vcpkg, only pokes around in the file tree left after the cargo vcpkg build/./vcpkg install step.

Are you dependent on using cargo-vcpkg for the vcpkg build or are you checking out vcpkg installing there and pointing at that tree with the VCPKG_ROOT environment variable? If the latter, you should be able to try out your own triplets at least without the whole forking of vcpkg.

@pkgw
Copy link
Collaborator Author

pkgw commented Oct 11, 2022

Oh, that's sad to hear — fingers crossed that your fears are overblown 😢

The --overlay-triplets option does sound quite promising. For Tectonic, our CI and documentation do rely on cargo-vcpkg, but we could do some prototyping without it. If it looks like we have a solution I think it would definitely be worthwhile to modify cargo-vcpkg to expose the functionality.

@pkgw
Copy link
Collaborator Author

pkgw commented Oct 27, 2022

Oh hey! It looks like there are now "community" triplets including things like x64-linux-release, which ought to be useful in many situations here.

However, there does not appear to be an x64-windows-static-release, so the Tectonic CI builds could still benefit from a custom triple.

@pkgw
Copy link
Collaborator Author

pkgw commented Oct 27, 2022

OK, based on some initial testing in tectonic-typesetting/tectonic#961, it appears that a custom Windows triplet indeed speeds up my build times substantially.

The testing is based on my cargo-vcpkg overlay-triplets branch, which has a prototype implementation for support. I add a key to the toplevel Cargo.toml metadata called overlay-triplets-path, which is fairly directly mapped to the --overlay-triplets argument if present.

There are other ways one could implement this: support for args to vcpkg on the cargo-vcpkg build command line, etc. @waych what do you think of the approach I've taken? If it seems sensible to you I'll tidy it up into a PR.

@pkgw
Copy link
Collaborator Author

pkgw commented Nov 2, 2022

OK, with #13 and the current version of my test case tectonic-typesetting/tectonic#961, I'd say that this issue can be considered closed upon the next release. I've ~doubled the speed of my Windows vcpkg builds and there aren't any obvious inefficiencies induced by cargo-vcpkg anymore.

@waych
Copy link
Collaborator

waych commented Nov 11, 2022

Published as 0.1.7. Sorry for the delay.

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

No branches or pull requests

3 participants