compute queries in on other threads.

[ilyalesokhin-starkware]

This change is 

[ilyalesokhin-starkware]

includes:
https://reviewable.io/reviews/starkware-libs/cairo/6025#-

Reviewable status: 0 of 6 files reviewed, all discussions resolved

[maciektr]

crates/cairo-lang-compiler/src/lib.rs line 201 at r1 (raw file):

///  Checks if there are diagnostics in the database and if there are None, returns
///  the [SierraProgramWithDebug] object of the requested functions
pub fn get_sierra_program_for_functions(

Will this be used by compile_prepared_db?

[orizi]

Reviewed 5 of 8 files at r1, all commit messages.
Reviewable status: 5 of 8 files reviewed, 4 unresolved discussions (waiting on @ilyalesokhin-starkware and @maciektr)

---

-- commits line 3 at r1:
rebase

Code quote:

New commits in r1 on 7/16/2024 at 2:49 PM:

- a2ff8ae: Delay error reporting.

---

crates/cairo-lang-compiler/src/lib.rs line 190 at r1 (raw file):

                            }
                        });
                    }

maybe move out of function for smaller indentation.

Code quote:

                fn handle_func_inner(
                    processed_function_ids: &Mutex<UnorderedHashSet<ConcreteFunctionWithBodyId>>,
                    snapshot: salsa::Snapshot<RootDatabase>,
                    func_id: ConcreteFunctionWithBodyId,
                ) {
                    if processed_function_ids.lock().unwrap().insert(func_id) {
                        rayon::scope(move |s| {
                            let db = &*snapshot;
                            let Ok(function) = db.function_with_body_sierra(func_id) else {
                                return;
                            };
                            for statement in &function.body {
                                let Some(related_function_id) =
                                    try_get_function_with_body_id(db, statement)
                                else {
                                    continue;
                                };

                                let snapshot = salsa::ParallelDatabase::snapshot(&*snapshot);
                                s.spawn(move |_| {
                                    handle_func_inner(
                                        processed_function_ids,
                                        snapshot,
                                        related_function_id,
                                    )
                                })
                            }
                        });
                    }

---

crates/cairo-lang-compiler/src/lib.rs line 209 at r1 (raw file):

    let requested_function_ids_copy = requested_function_ids.clone();
    rayon::spawn(move || compute_queries(snapshot, requested_function_ids_copy));

include the spawn in the function - and call the function warmup_db so it would be easily callable.

Code quote:

    rayon::spawn(move || compute_queries(snapshot, requested_function_ids_copy));

[ilyalesokhin-starkware]

crates/cairo-lang-compiler/src/lib.rs line 209 at r1 (raw file):

Previously, orizi wrote…

include the spawn in the function - and call the function warmup_db so it would be easily callable.

It will add indentation, and it won't be obvious that the warmup happens on a separate thread.

[ilyalesokhin-starkware]

crates/cairo-lang-compiler/src/lib.rs line 201 at r1 (raw file):

Previously, maciektr (maciektr) wrote…

Will this be used by compile_prepared_db?

Ideally, yes, but it is not apparent how to make it work since compile_prepared_db does not have all the requested Ids.

[ilyalesokhin-starkware]

Reviewable status: 5 of 8 files reviewed, 4 unresolved discussions (waiting on @maciektr and @orizi)

---

-- commits line 3 at r1:

Previously, orizi wrote…

rebase

I didn't merge Delay error reporting, as I think it is a regression unless we merge this PR.

[orizi]

Reviewable status: 5 of 8 files reviewed, 4 unresolved discussions (waiting on @ilyalesokhin-starkware and @maciektr)

---

crates/cairo-lang-compiler/src/lib.rs line 209 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

It will add indentation, and it won't be obvious that the warmup happens on a separate thread.

i already supplied a basic solution for the indentation issue - for the other part - HEAVILY DOC.
and maybe spawn_db_warmup or something of that sort.

[ilyalesokhin-starkware]

crates/cairo-lang-compiler/src/lib.rs line 190 at r1 (raw file):

Previously, orizi wrote…

maybe move out of function for smaller indentation.

I need s from the lambda for s.spawn.

[orizi]

Reviewable status: 5 of 8 files reviewed, 4 unresolved discussions (waiting on @ilyalesokhin-starkware and @maciektr)

---

crates/cairo-lang-compiler/src/lib.rs line 190 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

I need s from the lambda for s.spawn.

you can just add it to the function arguments - no?

[ilyalesokhin-starkware]

crates/cairo-lang-compiler/src/lib.rs line 190 at r1 (raw file):

Previously, orizi wrote…

you can just add it to the function arguments - no?

When I try to do that I get:

Code snippet:

error[E0521]: borrowed data escapes outside of closure
   --> crates/cairo-lang-compiler/src/lib.rs:182:33
    |
169 |                           rayon::scope(move |s| {
    |                                              -
    |                                              |
    |                                              `s` declared here, outside of the closure body
    |                                              `s` is a reference that is only valid in the closure body
...
182 | /                                 s.spawn(move |_| {
183 | |                                     handle_func_inner(
184 | |                                         s,
185 | |                                         processed_function_ids,
...   |
188 | |                                     )
189 | |                                 })
    | |__________________________________^ `s` escapes the closure body here

[orizi]

Reviewable status: 5 of 8 files reviewed, 4 unresolved discussions (waiting on @ilyalesokhin-starkware and @maciektr)

---

crates/cairo-lang-compiler/src/lib.rs line 190 at r1 (raw file):

                            }
                        });
                    }

you may consider extracting this into a regular function, and just calling it from the inner function.

Code quote:

                    if processed_function_ids.lock().unwrap().insert(func_id) {
                        rayon::scope(move |s| {
                            let db = &*snapshot;
                            let Ok(function) = db.function_with_body_sierra(func_id) else {
                                return;
                            };
                            for statement in &function.body {
                                let Some(related_function_id) =
                                    try_get_function_with_body_id(db, statement)
                                else {
                                    continue;
                                };

                                let snapshot = salsa::ParallelDatabase::snapshot(&*snapshot);
                                s.spawn(move |_| {
                                    handle_func_inner(
                                        processed_function_ids,
                                        snapshot,
                                        related_function_id,
                                    )
                                })
                            }
                        });
                    }

---

crates/cairo-lang-compiler/src/lib.rs line 209 at r1 (raw file):

Previously, orizi wrote…

i already supplied a basic solution for the indentation issue - for the other part - HEAVILY DOC.
and maybe spawn_db_warmup or something of that sort.

in any case - just add a wrapper function - won't cause indentation.

[ilyalesokhin-starkware]

crates/cairo-lang-compiler/src/lib.rs line 190 at r1 (raw file):

Previously, orizi wrote…

you may consider extracting this into a regular function, and just calling it from the inner function.

I don't follow, where would the rayon scope come from?

[orizi]

Reviewed 2 of 4 files at r2, 1 of 4 files at r3.
Reviewable status: 3 of 8 files reviewed, 4 unresolved discussions (waiting on @ilyalesokhin-starkware and @maciektr)

---

crates/cairo-lang-compiler/src/lib.rs line 190 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

I don't follow, where would the rayon scope come from?

would be a param to the external function.
but now i realise this won't actually work in this case.

---

crates/cairo-lang-compiler/src/lib.rs line 159 at r3 (raw file):

    let processed_function_ids =
        &Mutex::new(UnorderedHashSet::<ConcreteFunctionWithBodyId>::default());
    rayon::scope(move |s| {

how does this compare?

Suggestion:

  rayon::scope_fifo(move |s| {

[ilyalesokhin-starkware]

crates/cairo-lang-compiler/src/lib.rs line 159 at r3 (raw file):

Previously, orizi wrote…

how does this compare?

I didn't see a differnece

[ilyalesokhin-starkware]

Reviewable status: 3 of 8 files reviewed, 4 unresolved discussions (waiting on @maciektr and @orizi)

---

crates/cairo-lang-compiler/src/lib.rs line 190 at r1 (raw file):

Previously, orizi wrote…

would be a param to the external function.
but now i realise this won't actually work in this case.

Done.

---

crates/cairo-lang-compiler/src/lib.rs line 209 at r1 (raw file):

Previously, orizi wrote…

in any case - just add a wrapper function - won't cause indentation.

Done.

[orizi]

Reviewed 1 of 4 files at r3, 6 of 6 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ilyalesokhin-starkware and @maciektr)

---

crates/cairo-lang-compiler/src/lib.rs line 201 at r4 (raw file):

}

// Spawns a task to warm up the db.

Suggestion:

/// Spawns a task to warm up the db.

[ilyalesokhin-starkware]

Reviewable status: 7 of 8 files reviewed, 2 unresolved discussions (waiting on @maciektr and @orizi)

---

crates/cairo-lang-compiler/src/lib.rs line 201 at r4 (raw file):

}

// Spawns a task to warm up the db.

Done.

[orizi]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @maciektr)