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

Use direct call to sphinx build for faster build #5518

Merged
merged 2 commits into from
Aug 30, 2024

Conversation

khancyr
Copy link
Contributor

@khancyr khancyr commented Oct 31, 2023

remove subprocess call for building (make things a bit faster)
use 2 cpu for inventory parsing and building. We could probably raise this more build it will collide with the parallel build.

On rover wiki building it save 30s.

@khancyr khancyr force-pushed the feature/parallele-build branch from abc3370 to 77d1eb1 Compare October 31, 2023 22:18
@khancyr
Copy link
Contributor Author

khancyr commented Oct 31, 2023

need newer sphinx version to work and updated youtube plugin

@khancyr
Copy link
Contributor Author

khancyr commented Oct 31, 2023

hum ... look like the current CI is just failing silently :

2023-10-31T22:21:23.7942369Z Handler <function download_images at 0x000001C8FBBED3F0> for event 'env-updated' threw an exception (exception: [Errno 22] Invalid argument: 'D:\\a\\ardupilot_wiki\\ardupilot_wiki\\copter\\build\\html\\_video_thumbnail\\-Db4u8LJE5w?t=103.jpg')

https://pipelinesghubeus23.actions.githubusercontent.com/gvVmBJgcaBYml9XHduM82e3qv4adNpnuFd0H59d2aO1VWNxrWB/_apis/pipelines/1/runs/8942/signedlogcontent/2?urlExpires=2023-10-31T22%3A42%3A49.7261743Z&urlSigningMethod=HMACV1&urlSignature=3aXk2tC%2FzkYoRNVwjHh3S8mp0lqPwPsyxaMneu66Myc%3D

Will have to find a solution for this to pass CI

@khancyr
Copy link
Contributor Author

khancyr commented Oct 31, 2023

removing ?t=103 should do the trick

@khancyr khancyr force-pushed the feature/parallele-build branch from 8c9d5d7 to 86628e8 Compare October 31, 2023 23:25
update.py Show resolved Hide resolved
update.py Show resolved Hide resolved
update.py Outdated Show resolved Hide resolved
@khancyr
Copy link
Contributor Author

khancyr commented Nov 1, 2023

@Ryanf55 thx for the review, I didn't do py3 codestyle update as last time it was a blocker for Peter. I will do in a following PR

@khancyr khancyr force-pushed the feature/parallele-build branch 2 times, most recently from 7bc7e9a to 258dcda Compare November 2, 2023 09:57
@khancyr khancyr force-pushed the feature/parallele-build branch from 258dcda to e73b6ba Compare November 13, 2023 08:25
@khancyr khancyr force-pushed the feature/parallele-build branch from e73b6ba to 6cd7cbf Compare August 29, 2024 17:48
@khancyr khancyr force-pushed the feature/parallele-build branch from 6cd7cbf to fcbea20 Compare August 29, 2024 20:22
@khancyr khancyr force-pushed the feature/parallele-build branch from fcbea20 to 0a753ac Compare August 29, 2024 20:27
@peterbarker
Copy link
Contributor

@Ryanf55 are you happy with this now?

@khancyr is the wiki build output identical before/after this change? i.e. with diff -ur

@Ryanf55
Copy link
Contributor

Ryanf55 commented Aug 29, 2024

@Ryanf55 are you happy with this now?

@khancyr is the wiki build output identical before/after this change? i.e. with diff -ur

From a code perspective yes. We still saving time doing this after the latest change? I don't have access to a computer to test till Sunday.

@khancyr
Copy link
Contributor Author

khancyr commented Aug 30, 2024

@peterbarker that show some small diff on the doctrees for the nav bar, but that seems unrelated. An example I saw is terrain navigation for copter. It is linked in two place : First time setup / configuration / failsafes mechnism and Mission Planning / terrain following.
On master terrain following page is hold by failsafes mechanism, so when you reach this page the navbar always show you the First time setup navtree.
image

With the build I did with this PR, it was hold by Mission Planning
image

This is just the navigation tree that change.

@Ryanf55 on copter wiki, I gain about 30-40s per build using this, mostly per the multi cpu use for the parsing.

Notice that using the sphinx-build is what make is using. If you open the makefile it calls sphinx-build. So we were creating a new process to call make to call sphinx-build. This PR save some ressources on this.

I can still remove the part that use multiple cpu and stick with 1 per default as before, I just tried to max out the cpu usage with some ruling that are what they are !

It could be interesting to test on the build server if it is faster to do build sequencially with full cpu usage or continue to try parallele build with lower cpu count.

@peterbarker
Copy link
Contributor

Well, I'm OK with this - Henry?

@khancyr khancyr merged commit 9238723 into ArduPilot:master Aug 30, 2024
4 checks passed
@khancyr khancyr deleted the feature/parallele-build branch August 30, 2024 10:50
@Ryanf55
Copy link
Contributor

Ryanf55 commented Aug 30, 2024

@peterbarker that show some small diff on the doctrees for the nav bar, but that seems unrelated. An example I saw is terrain navigation for copter. It is linked in two place : First time setup / configuration / failsafes mechnism and Mission Planning / terrain following. On master terrain following page is hold by failsafes mechanism, so when you reach this page the navbar always show you the First time setup navtree. image

With the build I did with this PR, it was hold by Mission Planning image

This is just the navigation tree that change.

@Ryanf55 on copter wiki, I gain about 30-40s per build using this, mostly per the multi cpu use for the parsing.

Notice that using the sphinx-build is what make is using. If you open the makefile it calls sphinx-build. So we were creating a new process to call make to call sphinx-build. This PR save some ressources on this.

I can still remove the part that use multiple cpu and stick with 1 per default as before, I just tried to max out the cpu usage with some ruling that are what they are !

It could be interesting to test on the build server if it is faster to do build sequencially with full cpu usage or continue to try parallele build with lower cpu count.

Thanks for the info, great to see!

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.

4 participants