Skip Git submodule update when possible

[mkeeter]

Problem

I'm working on a project which calls cargo multiple times during the course of a build. Each call to cargo has overhead, even if it's not doing any work. I'd like to minimize that overhead.

Doing some benchmarking, I notice that GitCheckout::update_submodules is a significant fraction of running time (17%).

Here's the relevant portion of the statistics:

373.00 ms  100.0%	0 s	 	    _RNvCs65SZBtCS5Ks_5cargo4main
373.00 ms  100.0%	0 s	 	     _RNvNtCs65SZBtCS5Ks_5cargo3cli4main
372.00 ms   99.7%	0 s	 	      _RNvNtNtCs65SZBtCS5Ks_5cargo8commands5rustc4exec
339.00 ms   90.8%	0 s	 	       _RNvNtNtCs1Skp9DrlJ3c_5cargo3ops13cargo_compile7compile
339.00 ms   90.8%	0 s	 	        _RNvNtNtCs1Skp9DrlJ3c_5cargo3ops13cargo_compile10compile_ws
306.00 ms   82.0%	0 s	 	         _RNvNtNtCs1Skp9DrlJ3c_5cargo3ops13cargo_compile10create_bcx
304.00 ms   81.5%	0 s	 	          _RNvNtNtCs1Skp9DrlJ3c_5cargo3ops7resolve20resolve_ws_with_opts
289.00 ms   77.4%	0 s	 	           _RNvNtNtCs1Skp9DrlJ3c_5cargo3ops7resolve21resolve_with_registry
278.00 ms   74.5%	0 s	 	            _RNvNtNtCs1Skp9DrlJ3c_5cargo3ops7resolve21resolve_with_previous
269.00 ms   72.1%	1.00 ms	 	             _RNvNtNtCs1Skp9DrlJ3c_5cargo4core8resolver7resolve
263.00 ms   70.5%	0 s	 	              _RNvNtNtCs1Skp9DrlJ3c_5cargo4core8resolver8activate
260.00 ms   69.7%	0 s	 	               _RNvMNtNtNtCs1Skp9DrlJ3c_5cargo4core8resolver9dep_cacheNtB2_15RegistryQueryer10build_deps
260.00 ms   69.7%	0 s	 	                _RINvNtNtCsgvCDDXNC3Ex_4core4iter8adapters11try_processINtNtB2_10filter_map9FilterMapINtNtNtCsgfxZwj4R8NY_5alloc3vec9into_iter8IntoIterTNtNtNtCs1Skp9DrlJ3c_5cargo4core10dependency10DependencyINtNtB1r_2rc2RcINtNtNtNtB1r_11collections5btree3set8BTreeSetNtNtNtB2f_4util9interning14InternedStringEEEENCNvMNtNtB2d_8resolver9dep_cacheNtB4O_15RegistryQueryer10build_deps0ETB29_IB33_INtB1p_3VecNtNtB2d_7summary7SummaryEEB32_EINtNtB6_6result6ResultNtNtB6_7convert10InfallibleNtCsdQXjNFn3J5f_6anyhow5ErrorENCINvXsn_B6J_IB6H_IB61_B5Q_EB7t_EINtNtNtB4_6traits7collect12FromIteratorIB6H_B5Q_B7t_EE9from_iterBQ_E0B8f_EB2f_
260.00 ms   69.7%	0 s	 	                 _RNvXs_NtNtCsgfxZwj4R8NY_5alloc3vec16in_place_collectINtB6_3VecTNtNtNtCs1Skp9DrlJ3c_5cargo4core10dependency10DependencyINtNtB8_2rc2RcIBP_NtNtB13_7summary7SummaryEEIB1T_INtNtNtNtB8_11collections5btree3set8BTreeSetNtNtNtB15_4util9interning14InternedStringEEEEINtNtB6_14spec_from_iter12SpecFromIterBY_INtNtNtCsgvCDDXNC3Ex_4core4iter8adapters12GenericShuntINtNtB4O_10filter_map9FilterMapINtNtB6_9into_iter8IntoIterTBZ_B2A_EENCNvMNtNtB13_8resolver9dep_cacheNtB6O_15RegistryQueryer10build_deps0EINtNtB4S_6result6ResultNtNtB4S_7convert10InfallibleNtCsdQXjNFn3J5f_6anyhow5ErrorEEE9from_iterB15_
260.00 ms   69.7%	0 s	 	                  _RNCNvMNtNtNtCs1Skp9DrlJ3c_5cargo4core8resolver9dep_cacheNtB4_15RegistryQueryer10build_deps0Ba_
260.00 ms   69.7%	0 s	 	                   _RNvMNtNtNtCs1Skp9DrlJ3c_5cargo4core8resolver9dep_cacheNtB2_15RegistryQueryer5query
259.00 ms   69.4%	0 s	 	                    _RNvXs_NtNtCs1Skp9DrlJ3c_5cargo4core8registryNtB4_15PackageRegistryNtB4_8Registry5query
239.00 ms   64.0%	0 s	 	                     _RNvMNtNtCs1Skp9DrlJ3c_5cargo4core8registryNtB2_15PackageRegistry13ensure_loaded
234.00 ms   62.7%	0 s	 	                      _RNvXs0_NtNtNtCs1Skp9DrlJ3c_5cargo7sources3git6sourceNtB5_9GitSourceNtNtNtBb_4core6source6Source17block_until_ready
142.00 ms   38.0%	0 s	 	                       _RNvMNtNtCs1Skp9DrlJ3c_5cargo7sources4pathNtB2_10PathSource6update
142.00 ms   38.0%	0 s	 	                        _RNvNtNtCs1Skp9DrlJ3c_5cargo3ops19cargo_read_manifest13read_packages
72.00 ms   19.3%	0 s	 	                       _RNvMs0_NtNtNtCs1Skp9DrlJ3c_5cargo7sources3git5utilsNtB5_11GitDatabase7copy_to
66.00 ms   17.6%	0 s	 	                        _RNvNvMs2_NtNtNtCs1Skp9DrlJ3c_5cargo7sources3git5utilsNtB7_11GitCheckout17update_submodules17update_submodules

(please excuse the mangled symbols; this is a stock cargo 1.65.0 (4bc8f24d3 2022-10-20))

Proposed Solution

Right now, checking out submodules is done unconditionally in GitDatabase::copy_into.

If we instead add it to GitCheckout::reset, we can ensure that submodules are checked out before the .cargo-ok sentinel is created.

Then, we can skip update_submodules entirely in cases where the sentinel is present.

Notes

I have already implemented this in a branch here; the PR template suggested that I open an issue before a PR, but I'm happy to open a PR once I get approval.

With this patch, I see a 15% speedup in my build, and GitDatabase::copy_to is no longer doing significant work:

298.00 ms   98.3%	0 s	 	     cargo::main::h0febb7da0d452280
298.00 ms   98.3%	0 s	 	      cargo::cli::main::h9c7ecb0b46b1f467
296.00 ms   97.6%	0 s	 	       cargo::commands::rustc::exec::hf5cae7fdc3f0b514
259.00 ms   85.4%	0 s	 	        cargo::ops::cargo_compile::compile::h3b2b664d081d2f73
259.00 ms   85.4%	0 s	 	         cargo::ops::cargo_compile::compile_ws::h2c966109daa8d843
242.00 ms   79.8%	0 s	 	          cargo::ops::cargo_compile::create_bcx::hef4812653f071044
241.00 ms   79.5%	0 s	 	           cargo::ops::resolve::resolve_ws_with_opts::hbe889142b3ce380f
230.00 ms   75.9%	0 s	 	            cargo::ops::resolve::resolve_with_registry::hf1d4c98a9c181397
219.00 ms   72.2%	0 s	 	             cargo::ops::resolve::resolve_with_previous::h3a8e288729a02ab9
210.00 ms   69.3%	1.00 ms	 	              cargo::core::resolver::resolve::ha4e5a156d4c8b284
205.00 ms   67.6%	1.00 ms	 	               cargo::core::resolver::activate_deps_loop::h62bd073de940959e
202.00 ms   66.6%	0 s	 	                cargo::core::resolver::activate::h137ce544843ea7a6
198.00 ms   65.3%	0 s	 	                 cargo::core::resolver::dep_cache::RegistryQueryer::build_deps::hf57628d93379b56c
197.00 ms   65.0%	0 s	 	                  core::iter::adapters::try_process::h2f947a405bdbbc4b
197.00 ms   65.0%	0 s	 	                   alloc::vec::in_place_collect::_$LT$impl$u20$alloc..vec..spec_from_iter..SpecFromIter$LT$T$C$I$GT$$u20$for$u20$alloc..vec..Vec$LT$T$GT$$GT$::from_iter::he16a05ea9f6220b0
197.00 ms   65.0%	0 s	 	                    cargo::core::resolver::dep_cache::RegistryQueryer::build_deps::_$u7b$$u7b$closure$u7d$$u7d$::he6db0565435c52cc
197.00 ms   65.0%	0 s	 	                     cargo::core::resolver::dep_cache::RegistryQueryer::query::h51f2e762f14f0c7c
197.00 ms   65.0%	0 s	 	                      _$LT$cargo..core..registry..PackageRegistry$u20$as$u20$cargo..core..registry..Registry$GT$::query::h286880a1fb2b9b4f
178.00 ms   58.7%	0 s	 	                       cargo::core::registry::PackageRegistry::ensure_loaded::h977163b8aa5a2eb6
176.00 ms   58.0%	0 s	 	                        _$LT$cargo..sources..git..source..GitSource$u20$as$u20$cargo..core..source..Source$GT$::block_until_ready::hcc9ec2030060d43d
157.00 ms   51.8%	0 s	 	                         cargo::sources::path::PathSource::update::h4a2525e03a6caeac
9.00 ms    2.9%	0 s	 	                         cargo::sources::git::utils::GitDatabase::copy_to::h68297a9b17cfefa5

[Eh2406]

What happens if an old cargo checks out and wrights the .cargo-ok, then new cargo sees the .cargo-ok when doing the copy_into, so nether of them check out the submodules?

[mkeeter]

@Eh2406 good question – I suspect this wouldn't work!

There are two obvious ways to work around that:

• Change the sentinel file name entirely
• Use a second sentinel file (with a different name) to indicate that submodule checkout is done

[epage]

I didn't realize we already have an issue for this. As #14603 has more discussion, I'm closing in favor of that.