Skip to content
This repository was archived by the owner on Sep 9, 2025. It is now read-only.

Allow path stitcher to work on SQLiteReader #291

Merged
hendrikvanantwerpen merged 4 commits into from
Sep 13, 2023
Merged

Allow path stitcher to work on SQLiteReader #291

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c27f4ef
Rename `Candidates` to `ForwardCandidates`
Jun 30, 2023
b20ac57
Allow `ForwardPartialPathStitcher` to work directly on a `SQLiteReader`
Jun 30, 2023
dc43a64
Thread `ForwardCandidates` errors through `ForwardPartialPathStitcher`
Jun 30, 2023
233e827
Improve comments
Jun 30, 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
Prev Previous commit
Next Next commit
Thread ForwardCandidates errors through ForwardPartialPathStitcher
  • Loading branch information
Hendrik van Antwerpen
Hendrik van Antwerpen committed Sep 13, 2023
commit dc43a646908a28061781a1c3cb9d2a2a5ed0815e
25 changes: 14 additions & 11 deletions stack-graphs/src/stitching.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ where

/// A trait to support finding candidates for partial path extension. Requires an accompanying
/// [`ToAppendable`] implementation to convert the candidate handles into [`Appendable`]s.
pub trait ForwardCandidates<H, A, Db>
pub trait ForwardCandidates<H, A, Db, Err>
where
A: Appendable,
Db: ToAppendable<H, A>,
Expand All @@ -184,7 +184,7 @@ where
&mut self,
_path: &PartialPath,
_cancellation_flag: &dyn CancellationFlag,
) -> Result<(), CancellationError> {
) -> Result<(), Err> {
Ok(())
}

Expand Down Expand Up @@ -224,7 +224,7 @@ impl<'a> GraphEdgeCandidates<'a> {
}
}

impl ForwardCandidates<Edge, Edge, GraphEdges> for GraphEdgeCandidates<'_> {
impl ForwardCandidates<Edge, Edge, GraphEdges, CancellationError> for GraphEdgeCandidates<'_> {
fn get_forward_candidates<R>(&mut self, path: &PartialPath, result: &mut R)
where
R: std::iter::Extend<Edge>,
Expand Down Expand Up @@ -581,7 +581,9 @@ impl<'a> DatabaseCandidates<'a> {
}
}

impl ForwardCandidates<Handle<PartialPath>, PartialPath, Database> for DatabaseCandidates<'_> {
impl ForwardCandidates<Handle<PartialPath>, PartialPath, Database, CancellationError>
for DatabaseCandidates<'_>
{
fn get_forward_candidates<R>(&mut self, path: &PartialPath, result: &mut R)
where
R: std::iter::Extend<Handle<PartialPath>>,
Expand Down Expand Up @@ -824,7 +826,7 @@ impl<H: Clone> ForwardPartialPathStitcher<H> {
/// Attempts to extend one partial path as part of the algorithm. When calling this function,
/// you are responsible for ensuring that `db` already contains all of the possible appendables
/// that we might want to extend `partial_path` with.
fn extend<A, Db, C>(
fn extend<A, Db, C, Err>(
&mut self,
candidates: &mut C,

@dcreager dcreager Sep 13, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only API suggestion I can think of is to consider if this could/should be mut candidates: C instead. I think you're always passing in a mut ref to a temporary, in which case you can just pass in that temporary directly. It probably doesn't matter from a codegen perspective, though, so if you consider this shape to be more understandable I'm also good with keeping it as-is.

@hendrikvanantwerpen hendrikvanantwerpen Sep 13, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this, but the SQLiteReader implements Candidates directly, and would be moved (and lost) by this change. So I'm going to leave it as-is.

partial_path: &PartialPath,
Expand All @@ -833,7 +835,7 @@ impl<H: Clone> ForwardPartialPathStitcher<H> {
where
A: Appendable,
Db: ToAppendable<H, A>,
C: ForwardCandidates<H, A, Db>,
C: ForwardCandidates<H, A, Db, Err>,
{
let (graph, partials, db) = candidates.get_graph_and_partials();
copious_debugging!(" Extend {}", partial_path.display(graph, partials));
Expand Down Expand Up @@ -923,11 +925,11 @@ impl<H: Clone> ForwardPartialPathStitcher<H> {
/// or not. It is not called on the initial paths.
///
/// [`previous_phase_partial_paths`]: #method.previous_phase_partial_paths
pub fn process_next_phase<A, Db, C, E>(&mut self, candidates: &mut C, extend_while: E)
pub fn process_next_phase<A, Db, C, E, Err>(&mut self, candidates: &mut C, extend_while: E)
where
A: Appendable,
Db: ToAppendable<H, A>,
C: ForwardCandidates<H, A, Db>,
C: ForwardCandidates<H, A, Db, Err>,
E: Fn(&StackGraph, &mut PartialPaths, &PartialPath) -> bool,
{
copious_debugging!("==> Start phase {}", self.phase_number);
Expand Down Expand Up @@ -1041,18 +1043,19 @@ impl<H: Clone> ForwardPartialPathStitcher<H> {
/// [`process_next_phase`][] manually.
///
/// [`process_next_phase`]: #method.process_next_phase
pub fn find_all_complete_partial_paths<I, F, A, Db, C>(
pub fn find_all_complete_partial_paths<I, F, A, Db, C, Err>(
candidates: &mut C,
starting_nodes: I,
cancellation_flag: &dyn CancellationFlag,
mut visit: F,
) -> Result<(), CancellationError>
) -> Result<(), Err>
where
I: IntoIterator<Item = Handle<Node>>,
A: Appendable,
Db: ToAppendable<H, A>,
C: ForwardCandidates<H, A, Db>,
C: ForwardCandidates<H, A, Db, Err>,
F: FnMut(&StackGraph, &mut PartialPaths, &PartialPath),
Err: std::convert::From<CancellationError>,
{
let mut stitcher = {
let (graph, partials, _) = candidates.get_graph_and_partials();
Expand Down
13 changes: 4 additions & 9 deletions stack-graphs/src/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ use crate::stitching::Database;
use crate::stitching::ForwardCandidates;
use crate::CancellationError;
use crate::CancellationFlag;
use crate::NoCancellation;

const VERSION: usize = 5;

Expand Down Expand Up @@ -720,17 +719,13 @@ impl PartialSymbolStack {
}
}

impl ForwardCandidates<Handle<PartialPath>, PartialPath, Database> for SQLiteReader {
impl ForwardCandidates<Handle<PartialPath>, PartialPath, Database, StorageError> for SQLiteReader {
fn load_forward_candidates(
&mut self,
path: &PartialPath,
_cancellation_flag: &dyn CancellationFlag,
) -> std::result::Result<(), CancellationError> {
// TODO what about other errors?
match self.load_partial_path_extensions(path, &NoCancellation) {
Err(StorageError::Cancelled(msg)) => Err(CancellationError(msg)),
_ => Ok(()),
}
cancellation_flag: &dyn CancellationFlag,
) -> std::result::Result<(), StorageError> {
self.load_partial_path_extensions(path, cancellation_flag)
}

fn get_forward_candidates<R>(&mut self, path: &PartialPath, result: &mut R)
Expand Down