Skip to content

[WheelVariants] Implement getting sorted variants aided by variants.json #2

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

Merged
merged 9 commits into from
Mar 19, 2025

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Mar 3, 2025

Add a variants_json argument to get_variant_hashes_by_priority() that enables the function to use variant definitions from variants.json instead of evaluating all possible combinations. This can yield significant speedup and lower resource use if only a handful of variants need to be considered.

The function is implemented without any plugin API changes. After all, the current API should be simpler for plugin authors, and generating separate variant lists should not pose a performance problem.

Implemention-wise, the function first converts all variant descriptions from variants.json into VariantDescription classes. Then, the list of variants is fitlered to include only variants that match configurations obtained from plugins. Finally, they are sorted using the configuration order.

I am not 100% sure about the sorting algorithm, but it seems to yield exactly the same order at least for the dummy-project wheels. The first element of the sort key is the number of metas in the variant, to prioritize variants that matched more items. It is followed by priorities (indices in configuration order) of all metas, ordered from the most preferred to the least preferred.

Add a `variants_json` argument to `get_variant_hashes_by_priority()`
that enables the function to use variant definitions from
`variants.json` instead of evaluating all possible combinations.
This can yield significant speedup and lower resource use if only
a handful of variants need to be considered.

The function is implemented without any plugin API changes.  After all,
the current API should be simpler for plugin authors, and generating
separate variant lists should not pose a performance problem.

Implemention-wise, the function first converts all variant descriptions
from `variants.json` into `VariantDescription` classes.  Then, the list
of variants is fitlered to include only variants that match
configurations obtained from plugins.  Finally, they are sorted using
the configuration order.

I am not 100% sure about the sorting algorithm, but it seems to yield
exactly the same order at least for the `dummy-project` wheels.
The first element of the sort key is the number of metas in the variant,
to prioritize variants that matched more items.  It is followed by
priorities (indices in configuration order) of all metas, ordered
from the most preferred to the least preferred.
@mgorny
Copy link
Contributor Author

mgorny commented Mar 4, 2025

I've also added warnings for missing providers/keys now, e.g.:

VariantLib: No plugins provide the following variant providers: fictional_tech; some variants will be ignored
VariantLib: The provider fictional_hw does not provide the following expected keys: architecturex; some variants will be ignored

This could suggest that either some package is missing, or perhaps it's outdated/in the wrong version.

@DEKHTIARJonathan DEKHTIARJonathan changed the title Implement getting sorted variants aided by variants.json [WheelVariants] Implement getting sorted variants aided by variants.json Mar 4, 2025
@DEKHTIARJonathan DEKHTIARJonathan added tracked Tracked on WheelNext Board and removed tracked Tracked on WheelNext Board labels Mar 4, 2025
@DEKHTIARJonathan DEKHTIARJonathan moved this from In progress to In review in WheelNext - Project Tracker Mar 4, 2025
@mgorny
Copy link
Contributor Author

mgorny commented Mar 4, 2025

I'm going to add a test that verifies that the variants.json-based sorting produces the same order.

@mgorny
Copy link
Contributor Author

mgorny commented Mar 4, 2025

Hmm, so apparently we're seeing a mismatch in two-value variants. If I'm reading the diff correctly, the original algorithm prfers all custom_hw :: driver_version + custom_hw :: hw_architecture variants over custom_hw :: driver_version + networking :: speed, but the new algorithm intersperses them. Need to fix that.

@mgorny
Copy link
Contributor Author

mgorny commented Mar 4, 2025

@DEKHTIARJonathan, do we care about Python 3.10 compatibility in the prototypes, or should I skip testing on 3.10 for now?

@mgorny
Copy link
Contributor Author

mgorny commented Mar 5, 2025

Okay, this should finally be correct. I've added a fuzzing test too, to get better coverage.

@DEKHTIARJonathan DEKHTIARJonathan changed the base branch from main to variant-json March 19, 2025 18:05
@DEKHTIARJonathan DEKHTIARJonathan merged commit dab6b68 into wheelnext:variant-json Mar 19, 2025
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracked Tracked on WheelNext Board
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

2 participants