reduce input variants to only _used_ input variants

[minrk]

before recomputing output metadata, which involves a number of expensive $O(\texttt{len(inputvariants)})$ operations.

reduces get_used_vars and Metadata.copy() time dramatically because input_variants can be several thousand items long to produce 10s of variants, since it is the cartesian product of all possible variants across conda-forge, including every single unused combination.

Combining this PR and the following upcoming optimizations to conda-build 24.7:

• reduce cost of large variant matrix conda/conda-build#5392
• perf: only set level if needed conda/conda-build#5384
• Use pickle's loads/dumps as a faster deepcopy conda/conda-build#5281
• (also requires HashableDict is removed in conda_build 24.7 #1967 for compatibility with conda-build 24.7)

render time for the petsc4py feedstock, which has 13,824 input variants to produce 72 linux-64 builds, is reduced from over 30 minutes to 55 seconds to produce the same result:

Checklist

• Added a news entry

[beckermr]

Thanks for this. I don't quite follow it and need to think on it a bit so I understand. I'm concerned the test suite may not catch all of the edge cases.

[minrk]

Looks like it doesn't quite work in every case and tests catch it, despite working in all of the recipes I tested.

I think perhaps conda-smithy needs to do its own detection of used vars and reduce them before passing them to cond-build's render. Because the vast majority of variants are never used in all conda-forge recipes (every recipe computes every variant for every input parameter across all of conda-forge before checking if anything will be used, and then caries this exploded matrix through the whole render process, accounting for ~90% of the remaining render time).

I think the upstream PR is probably not going to work either: conda/conda-build#5392

In general, conda-build seems to assume that every input variant will be used, which doesn't match conda-forge where the input config is used for every package and has numerous unused dimensions which should eventually be dropped.

[beckermr]

Thanks. The detection of used variables is itself expensive and so we'll need to think on this a bit.

[beckermr]

Some questions.

[minrk]

The tests pass now, but reading carefully, I think this reduction must do exactly what _collapse_subpackage_variants does before computing the metadata objects, but _collapse_subpackage_variants indicates the sub-metadata used_vars are necessary to get the right answer every time (I don't know how to concoct a test where the result differs).

But it's not all keys where it matters. The only case where this should be able to get the wrong answer requires:

1. a key that appears in an output get_used_vars, but not the top-level, and
2. has multiple values so it contributes to dimensionality

The consequences of getting it wrong, however, are a missing pin and missing matrix dimension. There is a workaround where recipes can artificially ensure that the key is consumed at the top-level. I've actually had to do exactly this a number of times over the years in multi-output feedstocks, so I know it's doable, but it's not very nice when it is required. I'm not sure it happens anymore, so reintroducing a potential source of it isn't great.

[minrk]

The upstream PR conda/conda-build#5392 no longer reduces the variant list because I don't understand what conda-build promises to do with unused variants. Instead it reduces the cost of a very large variant list, so I think it has a better chance of being acceptable.

[beckermr]

Looking good. Do we have a test that covers a multidimensional key only in an output? This seems like the relevant corner case given the discussion.

[minrk]

Do we have a test that covers a multidimensional key only in an output?

yes, test_conda_build_api_render_for_smithy exercises this with multiple_outputs which has multiple variants for each of two outputs.

I believe I have found a case where this will miss a variant: if a variable is only used in the build script of an output in a multi-output recipe, but nowhere in meta.yaml, it will be missed by get_used_vars(global=True). I'm not sure if/when it makes sense to write a recipe like that and I'm not sure conda-smithy needs to handle it. If we do need to handle it, it may be tricky to do without constructing all of the outputs to get build.script (which is what we want to avoid doing until after we've reduced the variant matrix), but calling find_used_variables_in_shell_script on our own glob of recipe/*.sh (and equivalent for bat) might do it. The difference would be that it would search all scripts, while conda-build's logic only searches the ones that are actually output.script for an output. I'm not sure if the .sh extension can be assumed, even though it's ~always used.

[beckermr]

Looking at this again, I think in order to merge we need to do either one of the following two things

1. Fix the code getting all used vars check so it is reliable
2. Add a test showing exactly what fails and ensure we are ok with ignoring that case

[minrk]

Updated timings rerendering petsc4py (220 variants):

conda-build	conda-smithy	time (s)
24.7.1	3.39.1	79
5392	3.39.1	41
24.7.1	this PR	19
5392	this PR	18

(The savings in conda/conda-build#5392 are mostly redundant with this PR, which applies the same reduction earlier, getting the benefit in more places). So this saves a lot less now that other optimizations have landed, if that enters into the decision for whether this is worth doing.

[beckermr]

Given the latest change, I think we can discard the len(1) > 1 check.

Also, we need to add .bat file to the tests if we don't have one.

[minrk]

I haven't removed the len(variant) > 1 check in this PR, and when I tried it, it removed the mpi provider pins in petsc4py. I.e. having:

host:
- { mpi }

does not result in the pin for the value of mpi (mpich or openmpi) being in the used_vars found by reduce_variants. That does mean if mpich itself had multiple version values to build, it would render incorrectly. The following pattern would render correctly, if we had done that:

host:
- mpich  # { mpi == 'mpich' }

I can try to add a test for that tomorrow, if you think we should, or if that means we should hold off on this. One minute is a lot less dire to optimize than 30.

[beckermr]

Yes. Let's add more tests.