Codecov Report

❌ Patch coverage is 61.53846% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.18%. Comparing base (b6c6143) to head (ccdb126).
⚠️ Report is 2 commits behind head on main.

@@            Coverage Diff             @@
##             main    #3429      +/-   ##
==========================================
- Coverage   59.40%   59.18%   -0.23%     
==========================================
  Files         637      633       -4     
  Lines       54089    54079      -10     
==========================================
- Hits        32134    32009     -125     
- Misses      21955    22070     +115     

This seems like something we need to have. module loading is used at scale in several places; we'll need to be certain that your changes are harmless or not used in existing code paths. You seem to have gotten a long way there.

I am thinking that if you are updating this package, the VM tests will exercise the code. WDYT?

Which VM tests are you referring to? I took a look at runvmtest, but at least the default image seems nonmodular, or at the least doesn't bring lib/modules into the image.

I would like to expose a concurrent safe API to allow for several goroutines that concurrently load then follow up with additional configuration that depends on those modules.

My first thought for what that could look like would just be promote depMap -> ModuleLoader and export the current parallelProbe().

Took a crack at this, which also allows for simplifying the interface by removing ParallelProbeOptions - we're just left with Prober.Probe & the backwards compatible Probe[Options].

Not super familiar with the review process here - is there some release window this waiting for, some more LGTMs, or does this PR look good-to-go?

A related discussion to this PR is

• if we say module loading is parallelized, for those modules with dependencies where we want to specify parameters, you can achieve this by just calling in the right order, and blocking where needed, e.g.:

loader := kmodule.NewProber({})
go loadModChain() {
  loader.Probe("driver-common", "opt=b")
  loader.Probe("driver-main", "var=c")
}()

Which involves knowing (and encoding) these dependencies beforehand - which isn't a significant burden, but is a potential footgun. e.g. if you instead write the below, now you've introduced a race where driver-common will sometimes be loaded with no params

go loader.Probe("driver-common", "opt=b")
go loader.Probe("driver-main", "var=c")

One of my colleagues suggested we could instead have some stateful mechanism for desired module parameters, maybe like below to avoid needing to encode the dependencies and simplify the caller code.

for moduleInfo in moduleInfos {
  loader.PrepareProbe(moduleInfo.name, moduleInfo.params)
}

for moduleInfo in moduleInfos {
 go loader.FinishProbe(moduleInfo.name)
}

It does expand the API here a bit, though.