Skip to content

Commit 8074cc7

Browse files
committed
perf(core): optimise assigning instances to groups
1 parent 0d1f602 commit 8074cc7

7 files changed

Lines changed: 140 additions & 97 deletions

File tree

src/cli.rs

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use {
2-
crate::{group_selector::GroupSelector, packages::Packages},
2+
crate::{dependency_type::DependencyType, group_selector::GroupSelector, packages::Packages},
33
clap::{builder::ValueParser, crate_description, crate_name, crate_version, Arg, ArgMatches, Command},
44
color_print::cformat,
55
itertools::Itertools,
@@ -42,12 +42,14 @@ pub struct Cli {
4242
pub disable_ansi: bool,
4343
/// Whether to simulate changes without writing them to disk
4444
pub dry_run: bool,
45-
/// A set of filters made up of filter options passed in as CLI arguments:
4645
/// - `--dependencies` to filter by dependency name
46+
pub dependencies: Vec<String>,
4747
/// - `--dependency-types` to filter by dependency type
48-
/// - `--specifier-types` to filter by specifier type
48+
pub dependency_types: Vec<String>,
4949
/// - `--packages` to filter by package name
50-
pub filter: Option<GroupSelector>,
50+
pub packages: Vec<String>,
51+
/// - `--specifier-types` to filter by specifier type
52+
pub specifier_types: Vec<String>,
5153
/// Which severity levels of logging to display
5254
#[allow(dead_code)]
5355
pub log_levels: Vec<LevelFilter>,
@@ -91,20 +93,39 @@ impl Cli {
9193
Self {
9294
check: (matches!(&subcommand, Subcommand::Format | Subcommand::Update)) && matches.get_flag("check"),
9395
cwd: env::current_dir().unwrap(),
94-
filter: get_filters(matches),
96+
dependencies: get_patterns(matches, "dependencies"),
97+
dependency_types: get_patterns(matches, "dependency-types"),
9598
disable_ansi: matches.get_flag("no-ansi"),
9699
dry_run: (matches!(&subcommand, Subcommand::Fix | Subcommand::Format | Subcommand::Update)) && matches.get_flag("dry-run"),
97100
log_levels: get_log_levels(matches),
98-
sort: get_order_by(matches),
99-
show_ignored: should_show(matches, "ignored"),
101+
packages: get_patterns(matches, "packages"),
100102
show_hints: should_show(matches, "hints"),
103+
show_ignored: should_show(matches, "ignored"),
101104
show_instances: should_show(matches, "instances"),
102105
show_status_codes: should_show(matches, "statuses"),
106+
sort: get_order_by(matches),
103107
source_patterns: get_patterns(matches, "source"),
108+
specifier_types: get_patterns(matches, "specifier-types"),
104109
subcommand,
105110
target: get_target(matches),
106111
}
107112
}
113+
114+
pub fn get_filters(&self, packages: &Packages, all_dependency_types: &[DependencyType]) -> Option<GroupSelector> {
115+
if !self.dependencies.is_empty() && !self.dependency_types.is_empty() && !self.packages.is_empty() && !self.specifier_types.is_empty() {
116+
Some(GroupSelector::new(
117+
/* all_packages: */ packages,
118+
/* include_dependencies: */ self.dependencies.clone(),
119+
/* include_dependency_types: */ self.dependency_types.clone(),
120+
/* alias_name: */ "CLI filters".to_string(),
121+
/* include_packages: */ self.packages.clone(),
122+
/* include_specifier_types: */ self.specifier_types.clone(),
123+
/* all_dependency_types: */ all_dependency_types,
124+
))
125+
} else {
126+
None
127+
}
128+
}
108129
}
109130

110131
fn get_target(matches: &ArgMatches) -> UpdateTarget {
@@ -582,24 +603,3 @@ fn get_log_levels(matches: &ArgMatches) -> Vec<LevelFilter> {
582603
.map(|(_, level)| level)
583604
.collect_vec()
584605
}
585-
586-
fn get_filters(matches: &ArgMatches) -> Option<GroupSelector> {
587-
let dependencies = get_patterns(matches, "dependencies");
588-
let dependency_types = get_patterns(matches, "dependency-types");
589-
let label = "CLI filters".to_string();
590-
let all_packages = &Packages::new();
591-
let packages = get_patterns(matches, "packages");
592-
let specifier_types = get_patterns(matches, "specifier-types");
593-
if dependencies.is_empty() && dependency_types.is_empty() && packages.is_empty() && specifier_types.is_empty() {
594-
None
595-
} else {
596-
Some(GroupSelector::new(
597-
all_packages,
598-
dependencies,
599-
dependency_types,
600-
label,
601-
packages,
602-
specifier_types,
603-
))
604-
}
605-
}

src/context.rs

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -47,36 +47,31 @@ impl Context {
4747
let registry_client = registry_client.unwrap_or_else(|| Arc::new(LiveRegistryClient::new()));
4848
let updates_by_internal_name = HashMap::new();
4949
let all_dependency_types = config.rcfile.get_all_dependency_types();
50-
let dependency_groups = config.rcfile.get_dependency_groups(&packages);
51-
let semver_groups = config.rcfile.get_semver_groups(&packages);
52-
let mut version_groups = config.rcfile.get_version_groups(&packages);
50+
let cli_filters = config.cli.get_filters(&packages, &all_dependency_types);
51+
let dependency_groups = config.rcfile.get_dependency_groups(&packages, &all_dependency_types);
52+
let semver_groups = config.rcfile.get_semver_groups(&packages, &all_dependency_types);
53+
let mut version_groups = config.rcfile.get_version_groups(&packages, &all_dependency_types);
5354

5455
let failed_updates = Vec::new();
5556

5657
packages.get_all_instances(&all_dependency_types, |mut descriptor| {
57-
let dependency_group = dependency_groups
58-
.iter()
59-
.find(|alias| alias.can_add(&all_dependency_types, &descriptor));
58+
let dependency_group = dependency_groups.iter().find(|alias| alias.can_add(&descriptor));
6059

6160
if let Some(group) = dependency_group {
6261
descriptor.internal_name = group.label.clone();
6362
}
6463

65-
match &config.cli.filter {
66-
Some(cli_group) => {
67-
descriptor.matches_cli_filter = cli_group.can_add(&all_dependency_types, &descriptor);
68-
}
69-
None => descriptor.matches_cli_filter = true,
70-
}
64+
descriptor.matches_cli_filter = match cli_filters.as_ref() {
65+
Some(filters) => filters.can_add(&descriptor),
66+
None => true,
67+
};
7168

7269
let preferred_semver_range = semver_groups
7370
.iter()
74-
.find(|group| group.selector.can_add(&all_dependency_types, &descriptor))
71+
.find(|group| group.selector.can_add(&descriptor))
7572
.and_then(|group| group.range.clone());
7673

77-
let version_group = version_groups
78-
.iter_mut()
79-
.find(|group| group.selector.can_add(&all_dependency_types, &descriptor));
74+
let version_group = version_groups.iter_mut().find(|group| group.selector.can_add(&descriptor));
8075

8176
let instance = Rc::new(Instance::new(descriptor, preferred_semver_range));
8277

src/group_selector.rs

Lines changed: 73 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ pub struct GroupSelector {
4747
/// - "workspace-protocol" or -!workspace-protocol"
4848
pub include_specifier_types: Vec<String>,
4949
pub exclude_specifier_types: Vec<String>,
50+
// Cache frequently accessed values
51+
has_dependency_type_filters: bool,
52+
has_specifier_type_filters: bool,
53+
has_dependency_filters: bool,
54+
has_package_filters: bool,
5055
}
5156

5257
impl GroupSelector {
@@ -57,44 +62,73 @@ impl GroupSelector {
5762
label: String,
5863
packages: Vec<String>,
5964
specifier_types: Vec<String>,
65+
all_dependency_types: &[DependencyType],
6066
) -> GroupSelector {
6167
let dependencies = with_resolved_keywords(&dependencies, all_packages);
68+
69+
let include_dependencies = create_globs(true, &dependencies);
70+
let exclude_dependencies = create_globs(false, &dependencies);
71+
let include_dependency_types = create_identifiers(true, &dependency_types);
72+
let exclude_dependency_types = create_identifiers(false, &dependency_types);
73+
let include_packages = create_globs(true, &packages);
74+
let exclude_packages = create_globs(false, &packages);
75+
let include_specifier_types = create_identifiers(true, &specifier_types);
76+
let exclude_specifier_types = create_identifiers(false, &specifier_types);
77+
78+
// Validate dependency types during construction
79+
for expected in include_dependency_types.iter().chain(exclude_dependency_types.iter()) {
80+
if !all_dependency_types.iter().any(|actual| actual.name == *expected) {
81+
error!("dependencyType '{expected}' does not match any of syncpack or your customTypes");
82+
error!("check your syncpack config file");
83+
process::exit(1);
84+
}
85+
}
86+
6287
GroupSelector {
63-
include_dependencies: create_globs(true, &dependencies),
64-
exclude_dependencies: create_globs(false, &dependencies),
65-
include_dependency_types: create_identifiers(true, &dependency_types),
66-
exclude_dependency_types: create_identifiers(false, &dependency_types),
88+
// Pre-compute boolean flags to avoid repeated empty checks
89+
has_dependency_type_filters: !include_dependency_types.is_empty() || !exclude_dependency_types.is_empty(),
90+
has_specifier_type_filters: !include_specifier_types.is_empty() || !exclude_specifier_types.is_empty(),
91+
has_dependency_filters: !include_dependencies.is_empty() || !exclude_dependencies.is_empty(),
92+
has_package_filters: !include_packages.is_empty() || !exclude_packages.is_empty(),
93+
94+
include_dependencies,
95+
exclude_dependencies,
96+
include_dependency_types,
97+
exclude_dependency_types,
6798
label,
68-
include_packages: create_globs(true, &packages),
69-
exclude_packages: create_globs(false, &packages),
70-
include_specifier_types: create_identifiers(true, &specifier_types),
71-
exclude_specifier_types: create_identifiers(false, &specifier_types),
99+
include_packages,
100+
exclude_packages,
101+
include_specifier_types,
102+
exclude_specifier_types,
72103
}
73104
}
74105

75-
pub fn can_add(&self, all_dependency_types: &[DependencyType], descriptor: &InstanceDescriptor) -> bool {
76-
self.has_valid_dependency_types(all_dependency_types)
77-
&& self.matches_dependency_types(descriptor)
78-
&& self.matches_packages(descriptor)
79-
&& self.matches_dependencies(descriptor)
80-
&& self.matches_specifier_types(descriptor)
81-
}
106+
pub fn can_add(&self, descriptor: &InstanceDescriptor) -> bool {
107+
// Order checks from cheapest/most-likely-to-fail to most expensive
108+
// 1. Specifier types (often empty, cheap string comparison)
109+
if self.has_specifier_type_filters && !self.matches_specifier_types(descriptor) {
110+
return false;
111+
}
112+
113+
// 2. Dependency types (cheap string comparison)
114+
if self.has_dependency_type_filters && !self.matches_dependency_types(descriptor) {
115+
return false;
116+
}
117+
118+
// 3. Dependencies (glob matching, more expensive)
119+
if self.has_dependency_filters && !self.matches_dependencies(descriptor) {
120+
return false;
121+
}
122+
123+
// 4. Packages (glob matching + borrow, most expensive)
124+
if self.has_package_filters && !self.matches_packages(descriptor) {
125+
return false;
126+
}
82127

83-
fn has_valid_dependency_types(&self, all_dependency_types: &[DependencyType]) -> bool {
84-
self
85-
.include_dependency_types
86-
.iter()
87-
.chain(self.exclude_dependency_types.iter())
88-
.for_each(|expected| {
89-
if !all_dependency_types.iter().any(|actual| actual.name == *expected) {
90-
error!("dependencyType '{expected}' does not match any of syncpack or your customTypes");
91-
error!("check your syncpack config file");
92-
process::exit(1);
93-
}
94-
});
95128
true
96129
}
97130

131+
#[inline]
98132
fn matches_dependency_types(&self, descriptor: &InstanceDescriptor) -> bool {
99133
matches_identifiers(
100134
&descriptor.dependency_type.name,
@@ -103,21 +137,25 @@ impl GroupSelector {
103137
)
104138
}
105139

140+
#[inline]
106141
fn matches_packages(&self, descriptor: &InstanceDescriptor) -> bool {
107-
matches_globs(&descriptor.package.borrow().name, &self.include_packages, &self.exclude_packages)
142+
// Cache the borrow result to avoid repeated borrow checks
143+
let package_name = &descriptor.package.borrow().name;
144+
matches_globs(package_name, &self.include_packages, &self.exclude_packages)
108145
}
109146

147+
#[inline]
110148
fn matches_dependencies(&self, descriptor: &InstanceDescriptor) -> bool {
111149
matches_globs(&descriptor.internal_name, &self.include_dependencies, &self.exclude_dependencies)
112150
}
113151

152+
#[inline]
114153
fn matches_specifier_types(&self, descriptor: &InstanceDescriptor) -> bool {
115-
self.include_specifier_types.is_empty()
116-
|| matches_identifiers(
117-
&descriptor.specifier.get_config_identifier(),
118-
&self.include_specifier_types,
119-
&self.exclude_specifier_types,
120-
)
154+
matches_identifiers(
155+
&descriptor.specifier.get_config_identifier(),
156+
&self.include_specifier_types,
157+
&self.exclude_specifier_types,
158+
)
121159
}
122160
}
123161

@@ -158,7 +196,7 @@ fn matches_identifiers(name: &str, includes: &[String], excludes: &[String]) ->
158196
}
159197

160198
fn matches_any_identifier(value: &str, identifiers: &[String]) -> bool {
161-
identifiers.contains(&value.to_string())
199+
identifiers.iter().any(|id| id == value)
162200
}
163201

164202
/// Resolve keywords such as `$LOCAL` and `!$LOCAL` to their actual values.

src/rcfile.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ impl Default for Rcfile {
143143

144144
impl Rcfile {
145145
/// Create every alias defined in the rcfile.
146-
pub fn get_dependency_groups(&self, packages: &Packages) -> Vec<GroupSelector> {
146+
pub fn get_dependency_groups(&self, packages: &Packages, all_dependency_types: &[DependencyType]) -> Vec<GroupSelector> {
147147
self
148148
.dependency_groups
149149
.iter()
@@ -155,30 +155,31 @@ impl Rcfile {
155155
/* alias_name: */ dependency_group_config.alias_name.clone(),
156156
/* include_packages: */ dependency_group_config.packages.clone(),
157157
/* include_specifier_types: */ dependency_group_config.specifier_types.clone(),
158+
/* all_dependency_types: */ all_dependency_types,
158159
)
159160
})
160161
.collect()
161162
}
162163

163164
/// Create every semver group defined in the rcfile.
164-
pub fn get_semver_groups(&self, packages: &Packages) -> Vec<SemverGroup> {
165+
pub fn get_semver_groups(&self, packages: &Packages, all_dependency_types: &[DependencyType]) -> Vec<SemverGroup> {
165166
let mut all_groups: Vec<SemverGroup> = vec![];
166-
all_groups.push(SemverGroup::get_exact_local_specifiers());
167+
all_groups.push(SemverGroup::get_exact_local_specifiers(all_dependency_types));
167168
self.semver_groups.iter().for_each(|group_config| {
168-
all_groups.push(SemverGroup::from_config(group_config, packages));
169+
all_groups.push(SemverGroup::from_config(group_config, packages, all_dependency_types));
169170
});
170-
all_groups.push(SemverGroup::get_catch_all());
171+
all_groups.push(SemverGroup::get_catch_all(all_dependency_types));
171172
all_groups
172173
}
173174

174175
/// Create every version group defined in the rcfile.
175-
pub fn get_version_groups(&self, packages: &Packages) -> Vec<VersionGroup> {
176+
pub fn get_version_groups(&self, packages: &Packages, all_dependency_types: &[DependencyType]) -> Vec<VersionGroup> {
176177
let mut all_groups: Vec<VersionGroup> = self
177178
.version_groups
178179
.iter()
179-
.map(|group_config| VersionGroup::from_config(group_config, packages))
180+
.map(|group_config| VersionGroup::from_config(group_config, packages, all_dependency_types))
180181
.collect();
181-
all_groups.push(VersionGroup::get_catch_all());
182+
all_groups.push(VersionGroup::get_catch_all(all_dependency_types));
182183
all_groups
183184
}
184185

src/semver_group.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use {
2-
crate::{group_selector::GroupSelector, packages::Packages, specifier::semver_range::SemverRange},
2+
crate::{dependency_type::DependencyType, group_selector::GroupSelector, packages::Packages, specifier::semver_range::SemverRange},
33
serde::Deserialize,
44
};
55

@@ -13,7 +13,7 @@ pub struct SemverGroup {
1313

1414
impl SemverGroup {
1515
/// Create a default group which ensures local packages are an exact version
16-
pub fn get_exact_local_specifiers() -> SemverGroup {
16+
pub fn get_exact_local_specifiers(all_dependency_types: &[DependencyType]) -> SemverGroup {
1717
SemverGroup {
1818
selector: GroupSelector::new(
1919
/* all_packages: */ &Packages::new(),
@@ -22,13 +22,14 @@ impl SemverGroup {
2222
/* label: */ "Local package versions must be exact".to_string(),
2323
/* include_packages: */ vec![],
2424
/* include_specifier_types: */ vec![],
25+
/* all_dependency_types: */ all_dependency_types,
2526
),
2627
range: None,
2728
}
2829
}
2930

3031
/// Create a default/catch-all group which would apply to any instance
31-
pub fn get_catch_all() -> SemverGroup {
32+
pub fn get_catch_all(all_dependency_types: &[DependencyType]) -> SemverGroup {
3233
SemverGroup {
3334
selector: GroupSelector::new(
3435
/* all_packages: */ &Packages::new(),
@@ -37,20 +38,22 @@ impl SemverGroup {
3738
/* label: */ "Default Semver Group".to_string(),
3839
/* include_packages: */ vec![],
3940
/* include_specifier_types: */ vec![],
41+
/* all_dependency_types: */ all_dependency_types,
4042
),
4143
range: None,
4244
}
4345
}
4446

4547
/// Create a single version group from a config item from the rcfile.
46-
pub fn from_config(group: &AnySemverGroup, packages: &Packages) -> SemverGroup {
48+
pub fn from_config(group: &AnySemverGroup, packages: &Packages, all_dependency_types: &[DependencyType]) -> SemverGroup {
4749
let selector = GroupSelector::new(
4850
/* all_packages: */ packages,
4951
/* include_dependencies: */ group.dependencies.clone(),
5052
/* include_dependency_types: */ group.dependency_types.clone(),
5153
/* label: */ group.label.clone(),
5254
/* include_packages: */ group.packages.clone(),
5355
/* include_specifier_types: */ group.specifier_types.clone(),
56+
/* all_dependency_types: */ all_dependency_types,
5457
);
5558

5659
if let Some(true) = group.is_disabled {

0 commit comments

Comments
 (0)