Optimize exclusion computation for markers (#11158)

## Summary

Oddly this showed up in a trace. I think the lack of memoization was
making it fairly expensive.
This commit is contained in:
Charlie Marsh 2025-02-02 08:21:31 -05:00 committed by GitHub
parent 2dfeafbaa4
commit cca1d34432
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 128 additions and 91 deletions

View File

@ -48,16 +48,15 @@
use std::cmp::Ordering; use std::cmp::Ordering;
use std::fmt; use std::fmt;
use std::ops::Bound; use std::ops::Bound;
use std::sync::Mutex; use std::sync::{LazyLock, Mutex, MutexGuard};
use std::sync::MutexGuard;
use arcstr::ArcStr; use arcstr::ArcStr;
use itertools::{Either, Itertools}; use itertools::{Either, Itertools};
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use std::sync::LazyLock;
use uv_pep440::{release_specifier_to_range, Operator, Version, VersionSpecifier};
use version_ranges::Ranges; use version_ranges::Ranges;
use uv_pep440::{release_specifier_to_range, Operator, Version, VersionSpecifier};
use crate::marker::lowering::{ use crate::marker::lowering::{
CanonicalMarkerValueExtra, CanonicalMarkerValueString, CanonicalMarkerValueVersion, CanonicalMarkerValueExtra, CanonicalMarkerValueString, CanonicalMarkerValueVersion,
}; };
@ -833,6 +832,11 @@ impl InternerGuard<'_> {
return NodeId::FALSE; return NodeId::FALSE;
} }
// The operation was memoized.
if let Some(result) = guard.state.cache.get(&(xi, yi)) {
return *result;
}
let (x, y) = (guard.shared.node(xi), guard.shared.node(yi)); let (x, y) = (guard.shared.node(xi), guard.shared.node(yi));
// Perform Shannon Expansion of the higher order variable. // Perform Shannon Expansion of the higher order variable.
@ -857,7 +861,12 @@ impl InternerGuard<'_> {
}; };
// Create the output node. // Create the output node.
guard.create_node(func, children) let node = guard.create_node(func, children);
// Memoize the result of this operation.
guard.state.cache.insert((xi, yi), node);
node
} }
if let Some(exclusions) = self.state.exclusions { if let Some(exclusions) = self.state.exclusions {
@ -865,109 +874,137 @@ impl InternerGuard<'_> {
} }
let mut tree = NodeId::FALSE; let mut tree = NodeId::FALSE;
// Create all nodes upfront.
let os_name_nt = self.expression(MarkerExpression::String {
key: MarkerValueString::OsName,
operator: MarkerOperator::Equal,
value: arcstr::literal!("nt"),
});
let os_name_posix = self.expression(MarkerExpression::String {
key: MarkerValueString::OsName,
operator: MarkerOperator::Equal,
value: arcstr::literal!("posix"),
});
let sys_platform_linux = self.expression(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("linux"),
});
let sys_platform_darwin = self.expression(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("darwin"),
});
let sys_platform_ios = self.expression(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("ios"),
});
let sys_platform_win32 = self.expression(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("win32"),
});
let platform_system_freebsd = self.expression(MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: arcstr::literal!("FreeBSD"),
});
let platform_system_netbsd = self.expression(MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: arcstr::literal!("NetBSD"),
});
let platform_system_openbsd = self.expression(MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: arcstr::literal!("OpenBSD"),
});
let platform_system_sunos = self.expression(MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: arcstr::literal!("SunOS"),
});
let platform_system_ios = self.expression(MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: arcstr::literal!("iOS"),
});
let platform_system_ipados = self.expression(MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: arcstr::literal!("iPadOS"),
});
let sys_platform_aix = self.expression(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("aix"),
});
let sys_platform_android = self.expression(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("android"),
});
let sys_platform_emscripten = self.expression(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("emscripten"),
});
let sys_platform_cygwin = self.expression(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("cygwin"),
});
let sys_platform_wasi = self.expression(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("wasi"),
});
// Pairs of `os_name` and `sys_platform` that are known to be incompatible. // Pairs of `os_name` and `sys_platform` that are known to be incompatible.
// //
// For example: `os_name == 'nt' and sys_platform == 'darwin'` // For example: `os_name == 'nt' and sys_platform == 'darwin'`
let mut pairs = vec![ let mut pairs = vec![
( (os_name_nt, sys_platform_linux),
MarkerExpression::String { (os_name_nt, sys_platform_darwin),
key: MarkerValueString::OsName, (os_name_nt, sys_platform_ios),
operator: MarkerOperator::Equal, (os_name_posix, sys_platform_win32),
value: arcstr::literal!("nt"),
},
MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("linux"),
},
),
(
MarkerExpression::String {
key: MarkerValueString::OsName,
operator: MarkerOperator::Equal,
value: arcstr::literal!("nt"),
},
MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("darwin"),
},
),
(
MarkerExpression::String {
key: MarkerValueString::OsName,
operator: MarkerOperator::Equal,
value: arcstr::literal!("nt"),
},
MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("ios"),
},
),
(
MarkerExpression::String {
key: MarkerValueString::OsName,
operator: MarkerOperator::Equal,
value: arcstr::literal!("posix"),
},
MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: arcstr::literal!("win32"),
},
),
]; ];
// Pairs of `platform_system` and `sys_platform` that are known to be incompatible. // Pairs of `platform_system` and `sys_platform` that are known to be incompatible.
// //
// For example: `platform_system == 'FreeBSD' and sys_platform == 'darwin'` // For example: `platform_system == 'FreeBSD' and sys_platform == 'aix'`
// for platform_system in [
// Any `platform_system` values that we normalize away (like `Windows` to `win32`) are platform_system_freebsd,
// omitted, since we never expect them to be present in the tree. platform_system_netbsd,
// platform_system_openbsd,
// Unfortunately, we can't include Cygwin here, since Cygwin appears to use a platform_system_sunos,
// `platform_system` value with versions encoded (e.g., `CYGWIN_NT-10.0-22631). platform_system_ios,
// platform_system_ipados,
for platform_system in ["FreeBSD", "NetBSD", "OpenBSD", "SunOS", "iOS", "iPadOS"] { ] {
// An enumeration of known values, excluding FreeBSD, SunOS, and other Unix systems,
// which use the lowercased `uname -s`, which typically includes a version (e.g.,
// `freebsd8`).
//
// See: https://docs.python.org/3/library/sys.html#sys.platform
for sys_platform in [ for sys_platform in [
"aix", sys_platform_aix,
"android", sys_platform_android,
"emscripten", sys_platform_emscripten,
"ios", sys_platform_ios,
"linux", sys_platform_linux,
"darwin", sys_platform_darwin,
"win32", sys_platform_win32,
"cygwin", sys_platform_cygwin,
"wasi", sys_platform_wasi,
] { ] {
// Some of the above pairs are actually compatible. // Some of the above pairs are actually compatible.
if matches!((platform_system, sys_platform), ("iOS" | "iPadOS", "ios")) { if sys_platform == sys_platform_ios
&& (platform_system == platform_system_ios
|| platform_system == platform_system_ipados)
{
continue; continue;
} }
pairs.push(( pairs.push((platform_system, sys_platform));
MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: ArcStr::from(platform_system),
},
MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: ArcStr::from(sys_platform),
},
));
} }
} }
for (a, b) in pairs { for (a, b) in pairs {
let a = self.expression(a);
let b = self.expression(b);
let a_and_b = conjunction(self, a, b); let a_and_b = conjunction(self, a, b);
tree = disjunction(self, tree, a_and_b); tree = disjunction(self, tree, a_and_b);
} }