Skip to content

Commit

Permalink
Some cleanup and performance work
Browse files Browse the repository at this point in the history
  • Loading branch information
@sharkdp
sharkdp committed Dec 9, 2024
1 parent 57b96b8 commit 668fb66
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 81 deletions.
43 changes: 43 additions & 0 deletions crates/red_knot_python_semantic/src/semantic_index/use_def/bitset.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,15 @@ impl<const B: usize> BitSet<B> {
current_block: blocks[0],
}
}

pub(super) fn iter_rev(&self) -> ReverseBitSetIterator<'_, B> {
let blocks = self.blocks();
ReverseBitSetIterator {
blocks,
current_block_index: self.blocks().len() - 1,
current_block: blocks[self.blocks().len() - 1],
}
}
}

/// Iterator over values in a [`BitSet`].
Expand Down Expand Up @@ -159,10 +168,44 @@ impl<const B: usize> Iterator for BitSetIterator<'_, B> {

impl<const B: usize> std::iter::FusedIterator for BitSetIterator<'_, B> {}

/// Iterator over values in a [`BitSet`].
#[derive(Debug, Clone)]
pub(super) struct ReverseBitSetIterator<'a, const B: usize> {
/// The blocks we are iterating over.
blocks: &'a [u64],

/// The index of the block we are currently iterating through.
current_block_index: usize,

/// The block we are currently iterating through (and zeroing as we go.)
current_block: u64,
}

impl<const B: usize> Iterator for ReverseBitSetIterator<'_, B> {
type Item = u32;

fn next(&mut self) -> Option<Self::Item> {
while self.current_block == 0 {
if self.current_block_index == 0 {
return None;
}
self.current_block_index -= 1;
self.current_block = self.blocks[self.current_block_index];
}

let highest_bit_set = 63 - self.current_block.leading_zeros() as u64;
// TODO: efficiency, safety comment, etc.
self.current_block &= !(1u64 << highest_bit_set);
#[allow(clippy::cast_possible_truncation)]
Some(highest_bit_set as u32 + (64 * self.current_block_index) as u32)
}
}

#[cfg(test)]
mod tests {
use super::BitSet;

#[track_caller]
fn assert_bitset<const B: usize>(bitset: &BitSet<B>, contents: &[u32]) {
assert_eq!(bitset.iter().collect::<Vec<_>>(), contents);
}
Expand Down
24 changes: 12 additions & 12 deletions crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
//!
//! Tracking live declarations is simpler, since constraints are not involved, but otherwise very
//! similar to tracking live bindings.
use crate::semantic_index::use_def::bitset::ReverseBitSetIterator;

use super::bitset::{BitSet, BitSetIterator};
use ruff_index::newtype_index;
use smallvec::SmallVec;
Expand All @@ -64,14 +66,14 @@ const INLINE_BINDING_BLOCKS: usize = 3;

/// A [`BitSet`] of [`ScopedDefinitionId`], representing live bindings of a symbol in a scope.
type Bindings = BitSet<INLINE_BINDING_BLOCKS>;
type BindingsIterator<'a> = BitSetIterator<'a, INLINE_BINDING_BLOCKS>;
type BindingsIterator<'a> = ReverseBitSetIterator<'a, INLINE_BINDING_BLOCKS>;

/// Can reference this * 64 total declarations inline; more will fall back to the heap.
const INLINE_DECLARATION_BLOCKS: usize = 3;

/// A [`BitSet`] of [`ScopedDefinitionId`], representing live declarations of a symbol in a scope.
type Declarations = BitSet<INLINE_DECLARATION_BLOCKS>;
type DeclarationsIterator<'a> = BitSetIterator<'a, INLINE_DECLARATION_BLOCKS>;
type DeclarationsIterator<'a> = ReverseBitSetIterator<'a, INLINE_DECLARATION_BLOCKS>;

/// Can reference this * 64 total constraints inline; more will fall back to the heap.
const INLINE_CONSTRAINT_BLOCKS: usize = 2;
Expand Down Expand Up @@ -137,8 +139,8 @@ impl SymbolDeclarations {
/// Return an iterator over live declarations for this symbol.
pub(super) fn iter(&self) -> DeclarationIdIterator {
DeclarationIdIterator {
inner: self.live_declarations.iter(),
branching_conditions: self.branching_conditions.iter(),
inner: self.live_declarations.iter_rev(),
branching_conditions: self.branching_conditions.iter().rev(),
}
}

Expand Down Expand Up @@ -215,9 +217,9 @@ impl SymbolBindings {
/// Iterate over currently live bindings for this symbol.
pub(super) fn iter(&self) -> BindingIdWithConstraintsIterator {
BindingIdWithConstraintsIterator {
definitions: self.live_bindings.iter(),
constraints: self.constraints.iter(),
branching_conditions: self.branching_conditions.iter(),
definitions: self.live_bindings.iter_rev(),
constraints: self.constraints.iter().rev(),
branching_conditions: self.branching_conditions.iter().rev(),
}
}

Expand Down Expand Up @@ -507,8 +509,8 @@ pub(super) struct BindingIdWithConstraints<'a> {
#[derive(Debug, Clone)]
pub(super) struct BindingIdWithConstraintsIterator<'a> {
definitions: BindingsIterator<'a>,
constraints: ConstraintsIterator<'a>,
branching_conditions: BranchingConditionsIterator<'a>,
constraints: std::iter::Rev<ConstraintsIterator<'a>>,
branching_conditions: std::iter::Rev<BranchingConditionsIterator<'a>>,
}

impl<'a> Iterator for BindingIdWithConstraintsIterator<'a> {
Expand Down Expand Up @@ -538,8 +540,6 @@ impl<'a> Iterator for BindingIdWithConstraintsIterator<'a> {
}
}

impl std::iter::FusedIterator for BindingIdWithConstraintsIterator<'_> {}

#[derive(Debug, Clone)]
pub(super) struct ConstraintIdIterator<'a> {
wrapped: BitSetIterator<'a, INLINE_CONSTRAINT_BLOCKS>,
Expand Down Expand Up @@ -575,7 +575,7 @@ impl std::iter::FusedIterator for BranchingConditionIdIterator<'_> {}
#[derive(Debug)]
pub(super) struct DeclarationIdIterator<'a> {
inner: DeclarationsIterator<'a>,
branching_conditions: BranchingConditionsIterator<'a>,
branching_conditions: std::iter::Rev<BranchingConditionsIterator<'a>>,
}

impl<'a> Iterator for DeclarationIdIterator<'a> {
Expand Down
123 changes: 54 additions & 69 deletions crates/red_knot_python_semantic/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,63 +257,56 @@ fn bindings_ty<'db>(
db: &'db dyn Db,
bindings_with_constraints: BindingWithConstraintsIterator<'_, 'db>,
) -> Option<Type<'db>> {
let def_types = bindings_with_constraints.map(
|BindingWithConstraints {
binding,
constraints,
branching_conditions,
}| {
let result = branching_conditions.branch_condition_truthiness(db);

if result.any_always_false {
// TODO: do we need to call binding_ty(…) even if we don't need the result?
(None, UnconditionallyVisible::No)
} else {
let unconditionally_visible =
if result.at_least_one_condition && result.all_always_true {
UnconditionallyVisible::Yes
} else {
UnconditionallyVisible::No
};

let mut constraint_tys = constraints
.filter_map(|constraint| narrowing_constraint(db, constraint, binding))
.peekable();

let binding_ty = binding_ty(db, binding);
if constraint_tys.peek().is_some() {
let intersection_ty = constraint_tys
.fold(
IntersectionBuilder::new(db).add_positive(binding_ty),
IntersectionBuilder::add_positive,
)
.build();
(Some(intersection_ty), unconditionally_visible)
let def_types = bindings_with_constraints
.map(
|BindingWithConstraints {
binding,
constraints,
branching_conditions,
}| {
let result = branching_conditions.branch_condition_truthiness(db);

if result.any_always_false {
// TODO: do we need to call binding_ty(…) even if we don't need the result?
(None, UnconditionallyVisible::No)
} else {
(Some(binding_ty), unconditionally_visible)
let unconditionally_visible =
if result.at_least_one_condition && result.all_always_true {
UnconditionallyVisible::Yes
} else {
UnconditionallyVisible::No
};

let mut constraint_tys = constraints
.filter_map(|constraint| narrowing_constraint(db, constraint, binding))
.peekable();

let binding_ty = binding_ty(db, binding);
if constraint_tys.peek().is_some() {
let intersection_ty = constraint_tys
.fold(
IntersectionBuilder::new(db).add_positive(binding_ty),
IntersectionBuilder::add_positive,
)
.build();
(Some(intersection_ty), unconditionally_visible)
} else {
(Some(binding_ty), unconditionally_visible)
}
}
}
},
);
},
)
.take_while_inclusive(|(_, uv)| *uv != UnconditionallyVisible::Yes)
.map(|(ty, _)| ty);

// TODO: get rid of all the collects and clean up, obviously
let def_types: Vec<_> = def_types.collect();

if !def_types.is_empty() && def_types.iter().all(|(ty, _)| ty.is_none()) {
if !def_types.is_empty() && def_types.iter().all(|ty| ty.is_none()) {
return Some(Type::Unknown);
}

// shrink the vector to only include everything from the last unconditionally visible binding
let def_types: Vec<_> = def_types
.iter()
.rev()
.take_while_inclusive(|(_, unconditionally_visible)| {
*unconditionally_visible != UnconditionallyVisible::Yes
})
.map(|(ty, _)| ty.unwrap_or(Type::Never))
.collect();

let mut def_types = def_types.into_iter().rev();
let mut def_types = def_types.iter().map(|ty| ty.unwrap_or(Type::Never)).rev();

if let Some(first) = def_types.next() {
if let Some(second) = def_types.next() {
Expand Down Expand Up @@ -350,33 +343,25 @@ fn declarations_ty<'db>(
declarations: DeclarationsIterator<'_, 'db>,
undeclared_ty: Option<Type<'db>>,
) -> DeclaredTypeResult<'db> {
let decl_types = declarations.map(|(declaration, branching_conditions)| {
let result = branching_conditions.branch_condition_truthiness(db);
let decl_types = declarations
.map(|(declaration, branching_conditions)| {
let result = branching_conditions.branch_condition_truthiness(db);

if result.any_always_false {
(Type::Never, UnconditionallyVisible::No)
} else {
if result.at_least_one_condition && result.all_always_true {
(declaration_ty(db, declaration), UnconditionallyVisible::Yes)
if result.any_always_false {
(Type::Never, UnconditionallyVisible::No)
} else {
(declaration_ty(db, declaration), UnconditionallyVisible::No)
if result.at_least_one_condition && result.all_always_true {
(declaration_ty(db, declaration), UnconditionallyVisible::Yes)
} else {
(declaration_ty(db, declaration), UnconditionallyVisible::No)
}
}
}
});
})
.take_while_inclusive(|(_, uv)| *uv != UnconditionallyVisible::Yes)
.map(|(ty, _)| ty);

// TODO: get rid of all the collects and clean up, obviously
let decl_types: Vec<_> = decl_types.collect();

// shrink the vector to only include everything from the last unconditionally visible binding
let decl_types: Vec<_> = decl_types
.iter()
.rev()
.take_while_inclusive(|(_, unconditionally_visible)| {
*unconditionally_visible != UnconditionallyVisible::Yes
})
.map(|(ty, _)| *ty)
.collect();

let decl_types = decl_types.into_iter().rev();

let mut all_types = undeclared_ty.into_iter().chain(decl_types);
Expand Down

0 comments on commit 668fb66

Please sign in to comment.