Skip to content
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

add min_fold_count_limit optimization #423

Merged
merged 22 commits into from
Aug 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
2922874
add min_fold_count_limit optimization
u9g Aug 2, 2023
6233217
Merge branch 'main' into min_fold_count_limit
u9g Aug 4, 2023
4d5927a
use usize_from_field_value in get_min_fold_count_limit
u9g Aug 4, 2023
8d8b871
Update trustfall_core/src/interpreter/execution.rs
u9g Aug 4, 2023
a5574af
Update trustfall_core/src/interpreter/execution.rs
u9g Aug 4, 2023
f960c05
Improve comment about the optimization
u9g Aug 4, 2023
d197c10
Merge branch 'main' into min_fold_count_limit
u9g Aug 4, 2023
acd55be
disable optimization when we have a tag on the count of folded elements
u9g Aug 4, 2023
22af47d
rewrite comment again
u9g Aug 4, 2023
735684a
improve comments
u9g Aug 4, 2023
c8a6e23
Merge branch 'main' into min_fold_count_limit
u9g Aug 5, 2023
9b7fe6a
use values() over iter()
u9g Aug 5, 2023
a68060f
improve variable names
u9g Aug 5, 2023
78a49bb
Update trustfall_core/src/interpreter/execution.rs
u9g Aug 5, 2023
62d776e
Merge branch 'min_fold_count_limit' of https://github.com/u9g/trustfa…
u9g Aug 5, 2023
9e6545b
use .any
u9g Aug 5, 2023
625392d
combine booleans to make safe_to_skip_part_of_fold arg
u9g Aug 5, 2023
2318846
Merge branch 'main' into min_fold_count_limit
u9g Aug 5, 2023
3e2caa1
remove comments that point out something obvious and rely on implem
u9g Aug 5, 2023
17ff414
add comment and return None if we try getting the lowerbound of a fil…
u9g Aug 5, 2023
8814461
get rid of safe_to_skip_part_of_fold and move around comments
u9g Aug 5, 2023
7e6354a
improve comment for why we return None
u9g Aug 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 89 additions & 4 deletions trustfall_core/src/interpreter/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,10 +317,53 @@ fn get_max_fold_count_limit(carrier: &mut QueryCarrier, fold: &IRFold) -> Option
result
}

/// If this IRFold has a filter on the folded element count, and that filter imposes
/// a min size that can be statically determined, return that min size so it can
/// be used for further optimizations. Otherwise, return None.
fn get_min_fold_count_limit(carrier: &mut QueryCarrier, fold: &IRFold) -> Option<usize> {
let mut result: Option<usize> = None;

let query_arguments = &carrier.query.as_ref().expect("query was not returned").arguments;
for post_fold_filter in fold.post_filters.iter() {
let next_limit = match post_fold_filter {
Operation::GreaterThanOrEqual(
FoldSpecificFieldKind::Count,
Argument::Variable(var_ref),
) => {
let variable_value =
usize_from_field_value(&query_arguments[&var_ref.variable_name])
.expect("for field value to be coercible to usize");
Some(variable_value)
}
Operation::GreaterThan(FoldSpecificFieldKind::Count, Argument::Variable(var_ref)) => {
let variable_value =
usize_from_field_value(&query_arguments[&var_ref.variable_name])
.expect("for field value to be coercible to usize");
Some(variable_value.saturating_add(1))
}
// If we don't know how many elements of the fold are required
// to satisfy this filter, fail early.
_ => return None,
obi1kenobi marked this conversation as resolved.
Show resolved Hide resolved
};

match (result, next_limit) {
(None, _) => result = next_limit,
(Some(l), Some(r)) if l < r => result = next_limit,
_ => {}
}
}

result
}

fn collect_fold_elements<'query, Vertex: Clone + Debug + 'query>(
mut iterator: ContextIterator<'query, Vertex>,
max_fold_count_limit: &Option<usize>,
min_fold_count_limit: &Option<usize>,
obi1kenobi marked this conversation as resolved.
Show resolved Hide resolved
) -> Option<Vec<DataContext<Vertex>>> {
// If we must collect the fold up to our upperbound of `max_fold_count_limit`,
// then we won't use our lowerbound of `min_fold_count_limit`, as by definition
// the upperbound will be larger than the lowerbound.
if let Some(max_fold_count_limit) = max_fold_count_limit {
// If this fold has more than `max_fold_count_limit` elements,
// it will get filtered out by a post-fold filter.
Expand Down Expand Up @@ -350,9 +393,16 @@ fn collect_fold_elements<'query, Vertex: Clone + Debug + 'query>(

Some(fold_elements)
} else {
// We weren't able to find any early-termination condition for materializing the fold,
// so materialize the whole thing and return it.
Some(iterator.collect())
let collected = match min_fold_count_limit {
// Some queries may be able to be optimized by only partially expanding the fold,
// just enough to check any filters that may be applied to the fold count.
Some(min_fold_count_limit) => iterator.take(*min_fold_count_limit).collect(),
// We weren't able to find any early-termination condition for materializing the fold,
// so materialize the whole thing and return it.
_ => iterator.collect(),
};

Some(collected)
}
}

Expand Down Expand Up @@ -443,6 +493,41 @@ fn compute_fold<'query, AdapterT: Adapter<'query> + 'query>(
let fold_component = fold.component.clone();
let fold_eid = fold.eid;
let max_fold_size = get_max_fold_count_limit(carrier, fold.as_ref());

// Queries that do not observe the fold count nor any fold contents may be able to
// be optimized by only partially expanding the fold, just enough to check any filters
// that may be applied to the fold count.
//
// For example, if `@filter(op: ">", value: ["$ten"])` is our only filter on the count
// of the fold, we can stop computing the rest of the fold after seeing we have 11 elements.
let min_fold_size =
if let Some(min_fold_size) = get_min_fold_count_limit(carrier, fold.as_ref()) {
let no_outputs_in_fold = fold.component.outputs.is_empty();
let has_output_on_fold_count =
fold.fold_specific_outputs.values().any(|x| *x == FoldSpecificFieldKind::Count);
let has_tag_on_fold_count = parent_component.vertices.values().any(|vertex| {
vertex.filters.iter().any(|filter| {
let Some(Argument::Tag(FieldRef::FoldSpecificField(tagged_fold_count))) =
filter.right()
else {
return false;
};

tagged_fold_count.fold_root_vid == fold.to_vid
&& tagged_fold_count.fold_eid == fold.eid
&& tagged_fold_count.kind == FoldSpecificFieldKind::Count
})
});

if no_outputs_in_fold && !has_output_on_fold_count && !has_tag_on_fold_count {
Some(min_fold_size)
} else {
None
}
} else {
None
};

let moved_fold = fold.clone();
let folded_iterator = edge_iterator.filter_map(move |(mut context, neighbors)| {
let imported_tags = context.imported_tags.clone();
Expand All @@ -466,7 +551,7 @@ fn compute_fold<'query, AdapterT: Adapter<'query> + 'query>(
let fold_elements = if fold_exists {
// N.B.: Note the `?` at the end here!
// This lets us early-discard folds that failed a post-processing filter.
Some(collect_fold_elements(computed_iterator, &max_fold_size)?)
Some(collect_fold_elements(computed_iterator, &max_fold_size, &min_fold_size)?)
} else {
None
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,6 @@ TestInterpreterOutputTrace(
),
Opid(15): TraceOp(
opid: Opid(15),
parent_opid: Some(Opid(12)),
content: YieldFrom(ResolveNeighborsInner(2, Prime(PrimeNumber(5)))),
),
Opid(16): TraceOp(
opid: Opid(16),
parent_opid: Some(Opid(12)),
content: OutputIteratorExhausted,
),
Opid(17): TraceOp(
opid: Opid(17),
parent_opid: Some(Opid(4)),
content: YieldInto(SerializableContext(
active_vertex: Some(Composite(CompositeNumber(30, [
Expand Down Expand Up @@ -156,18 +146,12 @@ TestInterpreterOutputTrace(
Vid(2): Some(Prime(PrimeNumber(3))),
},
),
SerializableContext(
active_vertex: Some(Prime(PrimeNumber(5))),
vertices: {
Vid(2): Some(Prime(PrimeNumber(5))),
},
),
]),
},
)),
),
Opid(18): TraceOp(
opid: Opid(18),
Opid(16): TraceOp(
opid: Opid(16),
parent_opid: Some(Opid(4)),
content: YieldFrom(ResolveProperty(SerializableContext(
active_vertex: Some(Composite(CompositeNumber(30, [
Expand Down Expand Up @@ -196,70 +180,64 @@ TestInterpreterOutputTrace(
Vid(2): Some(Prime(PrimeNumber(3))),
},
),
SerializableContext(
active_vertex: Some(Prime(PrimeNumber(5))),
vertices: {
Vid(2): Some(Prime(PrimeNumber(5))),
},
),
]),
},
), Int64(30))),
),
Opid(19): TraceOp(
opid: Opid(19),
Opid(17): TraceOp(
opid: Opid(17),
parent_opid: None,
content: ProduceQueryResult({
"value": Int64(30),
}),
),
Opid(18): TraceOp(
opid: Opid(18),
parent_opid: Some(Opid(4)),
content: AdvanceInputIterator,
),
Opid(19): TraceOp(
opid: Opid(19),
parent_opid: Some(Opid(3)),
content: AdvanceInputIterator,
),
Opid(20): TraceOp(
opid: Opid(20),
parent_opid: Some(Opid(4)),
parent_opid: Some(Opid(2)),
content: AdvanceInputIterator,
),
Opid(21): TraceOp(
opid: Opid(21),
parent_opid: Some(Opid(3)),
content: AdvanceInputIterator,
parent_opid: Some(Opid(1)),
content: OutputIteratorExhausted,
),
Opid(22): TraceOp(
opid: Opid(22),
parent_opid: Some(Opid(2)),
content: AdvanceInputIterator,
content: InputIteratorExhausted,
),
Opid(23): TraceOp(
opid: Opid(23),
parent_opid: Some(Opid(1)),
parent_opid: Some(Opid(2)),
content: OutputIteratorExhausted,
),
Opid(24): TraceOp(
opid: Opid(24),
parent_opid: Some(Opid(2)),
parent_opid: Some(Opid(3)),
content: InputIteratorExhausted,
),
Opid(25): TraceOp(
opid: Opid(25),
parent_opid: Some(Opid(2)),
parent_opid: Some(Opid(3)),
content: OutputIteratorExhausted,
),
Opid(26): TraceOp(
opid: Opid(26),
parent_opid: Some(Opid(3)),
parent_opid: Some(Opid(4)),
content: InputIteratorExhausted,
),
Opid(27): TraceOp(
opid: Opid(27),
parent_opid: Some(Opid(3)),
content: OutputIteratorExhausted,
),
Opid(28): TraceOp(
opid: Opid(28),
parent_opid: Some(Opid(4)),
content: InputIteratorExhausted,
),
Opid(29): TraceOp(
opid: Opid(29),
parent_opid: Some(Opid(4)),
content: OutputIteratorExhausted,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,6 @@ TestInterpreterOutputTrace(
),
Opid(15): TraceOp(
opid: Opid(15),
parent_opid: Some(Opid(12)),
content: YieldFrom(ResolveNeighborsInner(2, Prime(PrimeNumber(5)))),
),
Opid(16): TraceOp(
opid: Opid(16),
parent_opid: Some(Opid(12)),
content: OutputIteratorExhausted,
),
Opid(17): TraceOp(
opid: Opid(17),
parent_opid: Some(Opid(4)),
content: YieldInto(SerializableContext(
active_vertex: Some(Composite(CompositeNumber(30, [
Expand Down Expand Up @@ -156,18 +146,12 @@ TestInterpreterOutputTrace(
Vid(2): Some(Prime(PrimeNumber(3))),
},
),
SerializableContext(
active_vertex: Some(Prime(PrimeNumber(5))),
vertices: {
Vid(2): Some(Prime(PrimeNumber(5))),
},
),
]),
},
)),
),
Opid(18): TraceOp(
opid: Opid(18),
Opid(16): TraceOp(
opid: Opid(16),
parent_opid: Some(Opid(4)),
content: YieldFrom(ResolveProperty(SerializableContext(
active_vertex: Some(Composite(CompositeNumber(30, [
Expand Down Expand Up @@ -196,70 +180,64 @@ TestInterpreterOutputTrace(
Vid(2): Some(Prime(PrimeNumber(3))),
},
),
SerializableContext(
active_vertex: Some(Prime(PrimeNumber(5))),
vertices: {
Vid(2): Some(Prime(PrimeNumber(5))),
},
),
]),
},
), Int64(30))),
),
Opid(19): TraceOp(
opid: Opid(19),
Opid(17): TraceOp(
opid: Opid(17),
parent_opid: None,
content: ProduceQueryResult({
"value": Int64(30),
}),
),
Opid(18): TraceOp(
opid: Opid(18),
parent_opid: Some(Opid(4)),
content: AdvanceInputIterator,
),
Opid(19): TraceOp(
opid: Opid(19),
parent_opid: Some(Opid(3)),
content: AdvanceInputIterator,
),
Opid(20): TraceOp(
opid: Opid(20),
parent_opid: Some(Opid(4)),
parent_opid: Some(Opid(2)),
content: AdvanceInputIterator,
),
Opid(21): TraceOp(
opid: Opid(21),
parent_opid: Some(Opid(3)),
content: AdvanceInputIterator,
parent_opid: Some(Opid(1)),
content: OutputIteratorExhausted,
),
Opid(22): TraceOp(
opid: Opid(22),
parent_opid: Some(Opid(2)),
content: AdvanceInputIterator,
content: InputIteratorExhausted,
),
Opid(23): TraceOp(
opid: Opid(23),
parent_opid: Some(Opid(1)),
parent_opid: Some(Opid(2)),
content: OutputIteratorExhausted,
),
Opid(24): TraceOp(
opid: Opid(24),
parent_opid: Some(Opid(2)),
parent_opid: Some(Opid(3)),
content: InputIteratorExhausted,
),
Opid(25): TraceOp(
opid: Opid(25),
parent_opid: Some(Opid(2)),
parent_opid: Some(Opid(3)),
content: OutputIteratorExhausted,
),
Opid(26): TraceOp(
opid: Opid(26),
parent_opid: Some(Opid(3)),
parent_opid: Some(Opid(4)),
content: InputIteratorExhausted,
),
Opid(27): TraceOp(
opid: Opid(27),
parent_opid: Some(Opid(3)),
content: OutputIteratorExhausted,
),
Opid(28): TraceOp(
opid: Opid(28),
parent_opid: Some(Opid(4)),
content: InputIteratorExhausted,
),
Opid(29): TraceOp(
opid: Opid(29),
parent_opid: Some(Opid(4)),
content: OutputIteratorExhausted,
),
Expand Down
Loading